Skip to content
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

gh-114203: Optimise simple recursive critical sections #128126

Merged
merged 3 commits into from
Dec 23, 2024

Conversation

Yhg1s
Copy link
Member

@Yhg1s Yhg1s commented Dec 20, 2024

Add a fast path to (single-mutex) critical section locking iff the mutex is already held by the top-most critical section of this thread. This can matter a lot for indirectly recursive critical sections without intervening critical sections.

…mutex

is already held by the currently active, top-most critical section of this
thread. This can matter a lot for indirectly recursive critical sections
without intervening critical sections.
should be relatively rare and we don't want to burden the fastest path.
@Yhg1s Yhg1s marked this pull request as ready for review December 20, 2024 23:03
Copy link
Member

@gpshead gpshead left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I do wonder if there might be additional optimization opportunities along these lines, but that should be future work if ever needed.

Copy link
Contributor

@colesbury colesbury left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@Yhg1s
Copy link
Member Author

Yhg1s commented Dec 23, 2024

I do wonder if there might be additional optimization opportunities along these lines, but that should be future work if ever needed.

That's true for pretty much 100% of the work in free-threaded Python right now 😅 In addition to "I don't know if this fixes all thread-unsafety, but we can look at it later." :P

@Yhg1s
Copy link
Member Author

Yhg1s commented Dec 23, 2024

FWIW, Sam ran benchmarks on the latest version, which seems like a very clear positive ("Geometric mean: 1.024x faster (HPT: reliability of 99.39%, 1.00x faster at 99th %ile)"; no worrying regressions, some loss in some benchmarks but a small win in enough that it's either noise or an overall good thing, massive improvement in a few benchmarks): https://github.com/facebookexperimental/free-threading-benchmarking/blob/main/results/bm-20241220-3.14.0a3%2B-b28153d-NOGIL/bm-20241220-vultr-x86_64-Yhg1s-optimise_recursive_c-3.14.0a3%2B-b28153d-vs-base.svg

@Yhg1s Yhg1s merged commit 180d417 into python:main Dec 23, 2024
43 of 45 checks passed
@Yhg1s Yhg1s deleted the optimise-recursive-critical-section branch December 23, 2024 12:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type-feature A feature request or enhancement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants