Repository navigation
gh-64862: Use iter() with stop_exception in the stdlib - #156770
serhiy-storchaka wants to merge 1 commit into
Conversation
Sorry, something went wrong.
Replace hand-written loops which drain a container or a queue. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
What's the benefit of this change again? |
Sorry, something went wrong.
|
Clearer code. |
Sorry, something went wrong.
|
Then this as unnecessary churn. The existing code is already clear. |
Sorry, something went wrong.
|
It replaces 5 lines with 2 or 1. Other reason -- dogfooding. By using the new idiom in our code we add additional testing surface and show users how to use it. |
Sorry, something went wrong.
ambv
left a comment
There was a problem hiding this comment.
I like the change if only to introduce reader to the new idiom. But in two cases it's detrimental to performance at runtime so I would avoid changing those.
Sorry, something went wrong.
| itemref = self.data.pop() | ||
| except KeyError: | ||
| raise KeyError('pop from empty WeakSet') from None | ||
| for itemref in iter(self.data.pop, stop_exception=KeyError): |
There was a problem hiding this comment.
Not worth putting here. Most WeakSet.pop() calls return at the first live ref, which is almost always the first call. The old code raised and allocated nothing on that path. The new code sets up the iter(), binds the method, checks for the exception type, etc. etc.
I measured and that's 40% slower.
Sorry, something went wrong.
| events = self._events_queue | ||
| while events: | ||
| event = events.popleft() | ||
| for event in iter(self._events_queue.popleft, |
There was a problem hiding this comment.
This is also slower because it raises an exception on every call and the old loop never raised.
Sorry, something went wrong.
|
When you're done making the requested changes, leave the comment: I have made the requested changes; please review again. |
Sorry, something went wrong.
Replace hand-written loops which drain a container or a queue with iter(callable, stop_exception=...), added in GH-156298.
All of them except XMLPullParser.read_events() already caught the exception raised by the exhausted container; read_events() tested the queue for emptiness before popping from it, which is also one step less atomic.
Loops which drain a small container are left as they are: raising and catching the exception costs about 100 ns, which only pays off above ~35 items (measured on a PGO build; the per-item cost of the exception form is ~19% lower, because it does not test the container in every iteration).