Skip to content

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
srinivasreddy pushed a commit to srinivasreddy/cpython that referenced this pull request Jan 8, 2025
…128126)

Add a fast path to (single-mutex) critical section locking _iff_ the 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.
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