gh-91603: Speed up UnionType instantiation (alt)#91955
gh-91603: Speed up UnionType instantiation (alt)#91955serhiy-storchaka merged 2 commits intopython:mainfrom
Conversation
Co-authored-by: Yurii Karabas <[email protected]>
serhiy-storchaka
added
type-feature
serhiy-storchaka
changed the title
serhiy-storchaka
added
topic-typing
and removed
type-feature
|
The new code is even shorter than the old one! |
|
@serhiy-storchaka Just compare performance with my version. It's almost identical:
Benchmark hidden because not significant (3): (int | float) | list, (int | float) | (list | set), (float | set) | int | int | (float | list) |
|
I am not surprised because it is the same algorithm. "type | type" may be slightly slower because the code is more general, but it is a honest price for simpler code. On other hand, my code avoid creation a new tuple and a new UnionType object if possible, so it is expected that cases like I was skeptical about advisability of such optimization at all, but if it also reduces the size of the code I have no doubts. |
JelleZijlstra
left a comment
JelleZijlstra
left a comment
There was a problem hiding this comment.
Looks mostly good, but I have a question about refcounts.
| if (args == NULL) { | ||
| return NULL; | ||
| if (*obj == Py_None) { | ||
| *obj = (PyObject *)&_PyNone_Type; |
There was a problem hiding this comment.
Don't we need to INCREF here?
There was a problem hiding this comment.
No, it is a borrowed reference.
JelleZijlstra
added
the
🔨 test-with-buildbots
|
🤖 New build scheduled with the buildbot fleet by @JelleZijlstra for commit a4badbc 🤖 If you want to schedule another build, you need to add the ":hammer: test-with-buildbots" label again. |
bedevere-bot
removed
the
🔨 test-with-buildbots
4 participants
Co-authored-by: Yurii Karabas [email protected]
It is a rewriting of #91865.
Closes #91603.