GeezerVison is showing github.com — gh-100762: Switch order of exception match in gen_close by eendebakpt · Pull Request #101011 · python/cpython
Conversation
eendebakpt
commented
Jan 13, 2023
•
edited
Loading
Uh oh!
There was an error while loading. Please reload this page.
This PR optimizes the closing of generators by switching the match statements in the expression PyErr_ExceptionMatches(PyExc_StopIteration) || PyErr_ExceptionMatches(PyExc_GeneratorExit) from the gen_close method. The case of a PyExc_GeneratorExit is more common.
Benchmark (with optimizations including PGO) updated May 9th:
all: Mean +- std dev: [main2] 283 ns +- 11 ns -> [pr] 280 ns +- 6 ns: 1.01x faster
all with comparison: Mean +- std dev: [main2] 570 ns +- 14 ns -> [pr] 563 ns +- 9 ns: 1.01x faster
Benchmark hidden because not significant (3): generator, list comprehension, generator exhaust
Geometric mean: 1.00x faster
Benchmark script:
import pyperf
runner = pyperf.Runner()
setup="""
l=[1,2]
"""
runner.timeit(name=f"generator", stmt=f"(x for x in l)", setup=setup)
runner.timeit(name=f"list comprehension", stmt=f"[x for x in l]", setup=setup)
stmt="""
for x in (_ for _ in ()):
pass
"""
runner.timeit(name=f"generator exhaust", stmt=stmt, setup=setup)
runner.timeit(name=f"all", stmt=f"all( (x for x in l) ) ", setup=setup)
runner.timeit(name=f"all with comparison", stmt=f"all( (x==1 for x in l) ) ", setup=setup)
Notes:
def f():
try:
yield None
print('here')
except GeneratorExit:
print("exiting")
print('done')
g = f()
next(g)
g.close()
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
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.
Seems good. Let's wait for 3.13 in two weeks, since performance improvements are effectively frozen right now.
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
Seems good. Let's wait for 3.13 in two weeks, since performance improvements are effectively frozen right now.
@brandtbucher Thanks for looking into this. The main branch has changed in the meantime, and there is currently only a single match instead of the two before. This makes this PR irrelevant, I will close it.
|
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
Reviewers
markshannon
Awaiting requested review from markshannon
markshannon is a code owner
Uh oh!
There was an error while loading. Please reload this page.
Milestone
No milestone
Development
Successfully merging this pull request may close these issues.
Uh oh!
There was an error while loading. Please reload this page.
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
|