-
-
Notifications
You must be signed in to change notification settings - Fork 31.9k
gh-128563: Add correction note to tail call in whats new #130908
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Merging ASAP so we don't further unintentionally mislead anyone. |
|
|
@nelhage fyi i gave you credit for finding this as well in the whats new, please check it out :)! |
@@ -295,6 +295,19 @@ For further information on how to build Python, see | |||
|
|||
__ https://en.wikipedia.org/wiki/Tail_call | |||
|
|||
.. attention:: | |||
|
|||
This section previously reported a 9-15% geomean speedup. This number has since been |
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.
Can we say "geometric mean" here? I think geomean is too jargonny.
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.
🤦 thanks, but I automerged too quickly.
cautiously revised down to 3-5%. While we expect performance results to be better | ||
than what we report, our estimates are more conservative due to a | ||
`compiler bug <https://github.com/llvm/llvm-project/issues/106846>`_ found in | ||
Clang/LLVM 19. We were unaware of this bug, and it artifically boosted |
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.
Instead of saying "artificially boosted our numbers" (which seems to imply it magically made tail calling even better), maybe say "which negatively impacted the performance of the traditional non-tail calling interpreter loop"
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.
Yeah Stefan Marr pointed out that the artificially boosting comment makes no sense too. So I addressed that here #130911
…n#130908) * Add correction note to tail call in whats new * Update 3.14.rst
@nelhage
📚 Documentation preview 📚: https://cpython-previews--130908.org.readthedocs.build/