Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork33.3k
gh-124688: _decimal: Get a module state from ctx objects for performance#124691
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Uh oh!
There was an error while loading.Please reload this page.
Conversation
vstinner commentedSep 27, 2024
Do you have a benchmark showing the effect of this change? |
neonene commentedSep 27, 2024
|
neonene commentedSep 27, 2024 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
My different PGO builds says 3-5% faster on the |
vstinner commentedSep 27, 2024
Would you mind to share your results (before/after, difference)? |
neonene commentedSep 27, 2024
On release builds, the result was as fast as the global state access:#123100 (comment) ( |
neonene commentedSep 27, 2024 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
Picked a few a) main withd69529d restored (hard to evaluate main as-is) b) (a) with some patches c) (b) with 1 diff in |
vstinner commentedSep 28, 2024
Sorry, I don't know how to read your 6 benchmarks. I don't understand what you compared. I don't know what are "some patches" nor why you mention PyType_GetBaseByToken(), it doesn't help me to comparethis PR. So I just compared the main branch to this PR: Nice! 1.09x faster is worth it. |
Uh oh!
There was an error while loading.Please reload this page.
dc12237 intopython:mainUh oh!
There was an error while loading.Please reload this page.
picnixz commentedSep 28, 2024
I added the skip news since Victor enabled auto-merge. If a NEWS entry is really needed, we can add it later in a separate commit though. |
vstinner commentedSep 28, 2024
Merged, thanks@neonene! |
Uh oh!
There was an error while loading.Please reload this page.