GeezerVison is showing github.com — GH-116017: Get rid of `_COLD_EXIT` by brandtbucher · Pull Request #120960 · python/cpython
Conversation
brandtbucher
commented
Jun 24, 2024
•
edited by bedevere-app
Bot
Loading
Uh oh!
There was an error while loading. Please reload this page.
The cold exits use quite a bit of memory on JIT builds, and complicate memory allocation improvements (since they are the only immortal, shared-by-all-interpreters, unlinked, un-invalidatable executors). They also introduce a very subtle back-and-forth dance between executors when warming up side exits, and represent a significant proportion of the uops executed.
This:
- Removes them and puts all of their logic into the
_EXIT_TRACE instruction (previously this logic was shared between the exit_to_trace label (only used by _EXIT_TRACE) and _COLD_EXIT...
- ...and also changes
_EXIT_TRACE to use an oparg instead of an exit_index...
- ...which allows us to simplify the tier two instruction format.
Separately, I added some missing stats to _DYNAMIC_EXIT.
This results in a 34.7% reduction in traces executed, and a 1.6% reduction in uops executed (both corresponding to 3.7 billion fewer _COLD_EXITS). Per platform:
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
|
Maybe a question for @mdboom instead of you: why are the figures on the time plot different than the figures given in geometric mean? E.g. darwin arm64 says "Geometric mean: 1.00x faster" but the figure shows 1.027x" or roughly 3%?
|
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
|
I think the number in the README overview of the results is the "HPT" calculation, which (I believe) weights by "noise". You can see that those particular results were pulled upwards by the startup benchmarks, which have a higher standard deviation than most of the others.
But yeah, @mdboom can probably explain better.
|
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
|
The plot takes the mean across all of the distributions of all of the benchmarks. So it takes into account that the distributions of all of the benchmarks aren't necessarily normally distributed.
The geometric mean value (from pyperf) is the nth root of multiplying all of the means of all the benchmarks together.
|
Fidget-Spinner reacted with thumbs up emoji
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.
One minor nit, otherwise it looks good.
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Reviewers
ericsnowcurrently
Awaiting requested review from ericsnowcurrently
ericsnowcurrently 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.
|