Movatterモバイル変換


[0]ホーム

URL:


Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

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

Merged
vstinner merged 6 commits intopython:mainfromneonene:decstate
Sep 28, 2024
Merged

Conversation

@neonene
Copy link
Contributor

@neoneneneonene commentedSep 27, 2024
edited by bedevere-appbot
Loading

@vstinner
Copy link
Member

Do you have a benchmark showing the effect of this change?

@neonene
Copy link
ContributorAuthor

_dec_mpd_radix() is added (818c24f) so as not to be passed a different object, not for performance.

@rhettingerrhettinger removed their request for reviewSeptember 27, 2024 18:15
@neonene
Copy link
ContributorAuthor

neonene commentedSep 27, 2024
edited
Loading

My different PGO builds says 3-5% faster on thetelco benchmark.

@vstinner
Copy link
Member

My different PGO builds says 3-5% faster on the telco benchmark.

Would you mind to share your results (before/after, difference)?

@neonene
Copy link
ContributorAuthor

On release builds, the result was as fast as the global state access:#123100 (comment) (PR with alternatives: 1.08x )

@neonene
Copy link
ContributorAuthor

neonene commentedSep 27, 2024
edited
Loading

Picked a fewtelco results on Windows PGO builds:

a) main withd69529d restored (hard to evaluate main as-is)

9.68 ms +- 0.17 ms -> 8.94 ms +- 0.15 ms: 1.08x faster  # PR (ctx object)9.68 ms +- 0.17 ms -> 8.76 ms +- 0.11 ms: 1.10x faster  # global state access

b) (a) with some patches

8.99 ms +- 0.11 ms -> 8.49 ms +- 0.16 ms: 1.06x faster  # PR (ctx object)8.99 ms +- 0.11 ms -> 8.44 ms +- 0.20 ms: 1.07x faster  # global state access

c) (b) with 1 diff inPyType_GetBaseByToken()

8.91 ms +- 0.21 ms -> 8.78 ms +- 0.16 ms: 1.02x faster  # PR (ctx object)8.91 ms +- 0.21 ms -> 8.25 ms +- 0.17 ms: 1.08x faster  # global state access

@vstinner
Copy link
Member

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:

Mean +- std dev: [ref] 8.18 ms +- 0.28 ms -> [change] 7.53 ms +- 0.18 ms: 1.09x faster

Nice! 1.09x faster is worth it.

neonene reacted with thumbs up emoji

@vstinnervstinnerenabled auto-merge (squash)September 28, 2024 15:03
@vstinnervstinner merged commitdc12237 intopython:mainSep 28, 2024
36 checks passed
@picnixz
Copy link
Member

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
Copy link
Member

Merged, thanks@neonene!

@neoneneneonene deleted the decstate branchSeptember 28, 2024 18:52
Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment

Reviewers

@vstinnervstinnervstinner left review comments

Assignees

No one assigned

Labels

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@neonene@vstinner@picnixz

[8]ページ先頭

©2009-2025 Movatter.jp