bpo-46527: allow calling enumerate(iterable=...) again

[JelleZijlstra]

As mentioned in the issue, this fixes a regression in 3.11.

The regression was introduced in GH-25154 (bpo-43706). There were already
comments there about how this was too much code for a simple change. This
makes it even worse. I'd be open to removing the vectorcall support instead.

https://bugs.python.org/issue46527

[corona10]

lgtm Thanks for the fix

[corona10]

@JelleZijlstra
Can you please compare the benchmark result for this PR?
https://bugs.python.org/file49929/bench_enumerate.py

[corona10]

Memory leak check done

Raised RLIMIT_NOFILE: 256 -> 1024
0:00:00 load avg: 4.75 Run tests sequentially
0:00:00 load avg: 4.75 [1/1] test_enumerate
beginning 6 repetitions
123456
......

== Tests result: SUCCESS ==

1 test OK.

Total duration: 2.5 sec
Tests result: SUCCESS

[JelleZijlstra]

I don't think my machine is great for benchmarking, but here's what I got.

Branch:

% ./python.exe bench_enumerate.py 
.....................
WARNING: the benchmark result may be unstable
* the maximum (5.09 us) is 55% greater than the mean (3.29 us)

Try to rerun the benchmark with more runs, values and/or loops.
Run 'python.exe -m pyperf system tune' command to reduce the system jitter.
Use pyperf stats, pyperf dump and pyperf hist to analyze results.
Use --quiet option to hide these warnings.

bench enumerate: Mean +- std dev: 3.29 us +- 0.30 us

Main:

% ./python.exe bench_enumerate.py   
.....................
WARNING: the benchmark result may be unstable
* the standard deviation (528 ns) is 14% of the mean (3.72 us)

Try to rerun the benchmark with more runs, values and/or loops.
Run 'python.exe -m pyperf system tune' command to reduce the system jitter.
Use pyperf stats, pyperf dump and pyperf hist to analyze results.
Use --quiet option to hide these warnings.

bench enumerate: Mean +- std dev: 3.72 us +- 0.53 us

So I guess I made it faster.

[rhettinger]

Consider just reverting to the previous stable code. Vectorcall has little value in functions like enumerate() where the time loop of the input dominates the time for the initial call.

[JelleZijlstra]

Consider just reverting to the previous stable code.

Happy to do that if that's the consensus here. The code I added is rather complicated and probably not worth the maintenance overhead.

[corona10]

@rhettinger @JelleZijlstra
The root of regression and unstable was due to the absence of unit tests cover not vectorcall itself.
I think that after merging this PR the function will be stable, since unittest will cover all use cases of enumerate.
The only problem is might be code complexity it can be undervalued to maintain it.

The reason I applied the vector call was that even if the enumerate complexity is increased, the implementation itself has not been changed a long time, so it has a low possibility of implementation changes in the future too.
And also after we decided to make CPyhton faster, I feel that the bar raiser of complexity becomes lower than past.

[corona10]

But I will agree with reverting the change if we thought that enumerate has the possibility of changes in the future, but adding a unit test itself looks valuable too.

[corona10]

@rhettinger @JelleZijlstra So since no more regression is expected, I would like to propose merging the PR and once we need to change the implementation, we can revert Vectorcall anytime, Rollbacking Vectorcall will not raise any behavior regression so anytime we can rollback it.

[bedevere-bot]

A Python core developer has requested some changes be made to your pull request before we can consider merging it. If you could please address their requests along with any other requests in other reviews from core developers that would be appreciated.

Once you have made the requested changes, please leave a comment on this pull request containing the phrase I have made the requested changes; please review again. I will then notify any core developers who have left a review that you're ready for them to take another look at this pull request.

[JelleZijlstra]

I have made the requested changes; please review again

[bedevere-bot]

Thanks for making the requested changes!

@corona10: please review the changes made to this pull request.

[corona10]

ditto?

[corona10]

lgtm

[corona10]

Please reflect nit change :)

[bedevere-bot]

A Python core developer has requested some changes be made to your pull request before we can consider merging it. If you could please address their requests along with any other requests in other reviews from core developers that would be appreciated.

Once you have made the requested changes, please leave a comment on this pull request containing the phrase I have made the requested changes; please review again. I will then notify any core developers who have left a review that you're ready for them to take another look at this pull request.