Skip to content

gh-134637: make PyCFuncPtr_call lock free in ctypes #134702

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

Merged
merged 6 commits into from
May 26, 2025

Conversation

kumaraditya303
Copy link
Contributor

@kumaraditya303 kumaraditya303 commented May 26, 2025

This fixes the performance regression mentioned on the issue, it avoids acquiring the critical section in the general case of no contention.

@ZeroIntensity ZeroIntensity self-requested a review May 26, 2025 13:56
Copy link
Member

@encukou encukou left a comment

Choose a reason for hiding this comment

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

This looks fine for 3.14 backport, but... ideally ctypes should, where possible, look like a “normal” C extension that a third-party could write.
I'm worried about it reaching more and more into rather obscure internal APIs.

@kumaraditya303 kumaraditya303 added the needs backport to 3.14 bugs and security fixes label May 26, 2025
@kumaraditya303
Copy link
Contributor Author

./python -m test -R 3:3 test_ctypes
Using random seed: 4211406380
0:00:00 load avg: 0.24 Run 1 test sequentially in a single process
0:00:00 load avg: 0.24 [1/1] test_ctypes
beginning 6 repetitions. Showing number of leaks (. for 0 or less, X for 10 or more)
123:456
XX. .1.
test_ctypes leaked [0, 1, 0] memory blocks, sum=1 (this is fine)
0:00:15 load avg: 0.41 [1/1] test_ctypes passed

== Tests result: SUCCESS ==

1 test OK.

Total duration: 15.4 sec
Total tests: run=600 skipped=34
Total test files: run=1/1
Result: SUCCESS

No refleaks

@kumaraditya303
Copy link
Contributor Author

Performance on this PR release build without pgo:

 ./python.bat client.py 
Running Release|x64 interpreter...
0
Connected.
207440
203568
203348
200588
204112

Copy link
Member

@ZeroIntensity ZeroIntensity left a comment

Choose a reason for hiding this comment

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

LGTM. Let's run the FT buildbots just in case.

@ZeroIntensity
Copy link
Member

!buildbot nogil

@bedevere-bot
Copy link

🤖 New build scheduled with the buildbot fleet by @ZeroIntensity for commit 095e1cb 🤖

Results will be shown at:

https://buildbot.python.org/all/#/grid?branch=refs%2Fpull%2F134702%2Fmerge

The command will test the builders whose names match following regular expression: nogil

The builders matched are:

  • aarch64 Fedora Rawhide NoGIL refleaks PR
  • PPC64LE Fedora Rawhide NoGIL PR
  • AMD64 Windows PGO NoGIL PR
  • AMD64 Windows Server 2022 NoGIL PR
  • ARM64 MacOS M1 NoGIL PR
  • s390x Fedora Rawhide NoGIL PR
  • AMD64 Fedora Rawhide NoGIL refleaks PR
  • aarch64 Fedora Rawhide NoGIL PR
  • PPC64LE Fedora Rawhide NoGIL refleaks PR
  • AMD64 Fedora Rawhide NoGIL PR
  • ARM64 MacOS M1 Refleaks NoGIL PR
  • x86-64 MacOS Intel NoGIL PR
  • AMD64 CentOS9 NoGIL PR
  • s390x Fedora Rawhide NoGIL refleaks PR
  • x86-64 MacOS Intel ASAN NoGIL PR
  • AMD64 CentOS9 NoGIL Refleaks PR

@kumaraditya303 kumaraditya303 enabled auto-merge (squash) May 26, 2025 17:49
@kumaraditya303 kumaraditya303 linked an issue May 26, 2025 that may be closed by this pull request
@kumaraditya303 kumaraditya303 merged commit 3c05251 into python:main May 26, 2025
39 checks passed
@miss-islington-app
Copy link

Thanks @kumaraditya303 for the PR 🌮🎉.. I'm working now to backport this PR to: 3.14.
🐍🍒⛏🤖

@kumaraditya303 kumaraditya303 deleted the ctypes branch May 26, 2025 18:26
miss-islington pushed a commit to miss-islington/cpython that referenced this pull request May 26, 2025
…ion pointer in `free threading`. (pythonGH-134702)

Fix performance regression in calling `ctypes` function pointer in `free threading`.
(cherry picked from commit 3c05251)

Co-authored-by: Kumar Aditya <[email protected]>
@bedevere-app
Copy link

bedevere-app bot commented May 26, 2025

GH-134742 is a backport of this pull request to the 3.14 branch.

@bedevere-app bedevere-app bot removed the needs backport to 3.14 bugs and security fixes label May 26, 2025
kumaraditya303 added a commit that referenced this pull request May 26, 2025
…tion pointer in `free threading`. (GH-134702) (#134742)

gh-134637: Fix performance regression in calling `ctypes` function pointer in `free threading`. (GH-134702)

Fix performance regression in calling `ctypes` function pointer in `free threading`.
(cherry picked from commit 3c05251)

Co-authored-by: Kumar Aditya <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3.14t vs 3.13t cuts IOCP performance in half
4 participants