Replies: 4 comments 8 replies
-
|
The docs on iterator types say this:
So that implies the type shouldn't be relied on. I think the only functionality that might be relevant to compatibility is pickling, but actually they just produce an |
Beta Was this translation helpful? Give feedback.
-
|
It seems innocent enough. I just wonder if there's enough of a benefit, given that such an iterator will only be used for small ranges? |
Beta Was this translation helpful? Give feedback.
-
|
I added this to specialize.c: if (t == &PyRangeIter_Type) {
+ rangeiterobject *r = (rangeiterobject *)iter;
+ if (r->step == 1 && r->start >= -5 && r->start + r->len <= 257) {
+ return SPEC_FAIL_FOR_ITER_SMALLRANGE;
+ }
return SPEC_FAIL_FOR_ITER_RANGE;
}Then took stats of some of pyperformance, broken down by benchmark: https://gist.github.com/sweeneyde/26a67de173e6dfbe9f715e2457ce753d It seems some benchmarks primarily use small-int ranges. Patch is here: https://github.com/python/cpython/compare/main...sweeneyde:smallrangeiter?expand=1 TODO: Benchmarks |
Beta Was this translation helpful? Give feedback.
-
|
Microbenchmark (with pgo and lto) is pretty decent: |
Beta Was this translation helpful? Give feedback.
-
|
Pyperformance results are mixed: https://gist.github.com/sweeneyde/b7a8814e788b59475a30f7ee35fd456f |
Beta Was this translation helpful? Give feedback.
-
|
How much of those results are just noise? Why would this slow anything down other than random cache effects that would be different if you just move two functions around or delete an unused fprintf() call? |
Beta Was this translation helpful? Give feedback.
-
|
Interesting that bm_chaos has a 93.6% rate ("anticipated hit rate"?) but is slowed down 2%. And tornado has a 14.7% rate while being 11% faster. There doesn't seem to be much logic to these results. |
Beta Was this translation helpful? Give feedback.
-
|
Yeah, there is something fishy going on. I'll try catching up with main and try again. |
Beta Was this translation helpful? Give feedback.
-
|
I wonder if we could somehow instrument how often this applies in e.g. the PyPerformance benchmark suite. |
Beta Was this translation helpful? Give feedback.
-
|
@sweeneyde's gist suggests that it occurs pretty frequently (at least in hot |
Beta Was this translation helpful? Give feedback.
-
|
@sweeneyde I tested your branch and a variation of it (branch smallrangeiter_step) that also constructs a Pyperformance results for main, your branch and my variation show little differences. The step version is reported a bit faster, but I think this is mainly noise on my system. I also used valgrind to profile the code. With the patch, most of the execution time (about 60%) is in Test resultsAll tests with `--enable-optimizations`Microbenchmark: Results from pyperformance: |
Beta Was this translation helpful? Give feedback.
-
|
Thanks for those benchmarks! I filtered out to focus on the ones that use smallrangeiter the most and compiled them into a table:
|
Beta Was this translation helpful? Give feedback.
Uh oh!
There was an error while loading. Please reload this page.
-
Current implementation of rangeiterator
Proposal to add another type:
Then on
range.__iter__, instead of just two branches (rangeiter and longrangeiter), this would be a third.Are there backward-compatibility concerns about what the type of an iterator is? It's already standard that range iterators use different types for big numbers, so maybe it wouldn't be too bad to have a different type for this common small case?
Beta Was this translation helpful? Give feedback.
All reactions