Repository navigation
gh-64862: Use iter() with stop_exception in the stdlib - #156770
serhiy-storchaka wants to merge 1 commit into
Conversation
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? |
|
Clearer code. |
|
Then this as unnecessary churn. The existing code is already clear. |
|
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. |
ambv
left a comment
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.
| 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.
| 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.
|
When you're done making the requested changes, leave the comment: |
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).