-
-
Notifications
You must be signed in to change notification settings - Fork 30.9k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
GH-91432: Implement the FOR_ITER_SET specialization #94104
Conversation
Is iteration over sets common enough to justify this? Our main motivation for specializing |
@markshannon I am okay not applying this change if this is not worth specializing in the operation. |
static PyObject *setiter_iternext(_PySetIterObject *si) { | ||
PyObject *stack[1]; | ||
int err = _PySetIter_GetNext(si, stack); | ||
if (err <= 0) { | ||
return NULL; | ||
} | ||
return stack[0]; | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
static PyObject *setiter_iternext(_PySetIterObject *si) { | |
PyObject *stack[1]; | |
int err = _PySetIter_GetNext(si, stack); | |
if (err <= 0) { | |
return NULL; | |
} | |
return stack[0]; | |
} | |
static PyObject *setiter_iternext(_PySetIterObject *si) { | |
PyObject *key; | |
if (_PySetIter_GetNext(si, &key) <= 0) { | |
return NULL; | |
} | |
return key; | |
} |
might be better
Performance shows a 1% speedup, although I think this just shows that 1% variations can be caused by code layout changes and other build effects. Projecting from stats, I doubt that any further specialization of |
@markshannon agree Let's close the PR! |
Microbenchmark
result: Mean +- std dev: [base] 662 ns +- 5 ns -> [FOR_ITER_SET] 616 ns +- 2 ns: 1.07x faster
Leak test
I just follow @sweeneyde's work.
(Following the faster CPython project is kind of my daily hobby ;) Sorry if you already worked on your local branch.. )
According to his stat, set iteration (3.8%) would be worth optimizing as same as tuple iteration (2.8%)
This PR would be decided to be merged after #94096 is merged.