NotificationsYou must be signed in to change notification settings
Fork32.1k
Star67.3k

gh-103295: expose API for writing perf map files#103546

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

Jump to bottom

Merged

pablogsal merged 60 commits intopython:mainfromgsallam:main

May 21, 2023

Merged

gh-103295: expose API for writing perf map files#103546

pablogsal merged 60 commits intopython:mainfromgsallam:main

May 21, 2023

Conversation

Copy link

Contributor

gsallam commentedApr 14, 2023•
edited by bedevere-bot
Loading

#96123 added support for CPython to write /tmp/perf-.map files, associating instruction address ranges with a human-readable frame name for the Linux perf profiler.

Two external Python JIT compilers,Cinder andPyston, both also independently write to perf map files.

Since perf map files are one-per-process, multiple separate libraries trying to write perf map entries independently can lead to file corruption from simultaneous writes.

It's unlikely for both Cinder and Pyston JITs to be used in the same process, but it's quite reasonable to use one of these JITs along with CPython's native perf trampoline support.

This PR add a C-API to write to the perf map file. It also update the perf trampoline to use the new API to write to the perf map file.

Issue:expose API for writing perf map files #103295

Copy link

ghost commentedApr 14, 2023•
edited by ghost
Loading

All commit authors signed the Contributor License Agreement.

Copy link

bedevere-bot commentedApr 14, 2023

Most changes to Pythonrequire a NEWS entry.

Please add it using theblurb_it web app or theblurb command-line tool.

bedevere-bot mentioned this pull request

Apr 14, 2023

expose API for writing perf map files#103295

Closed

bedevere-bot added the awaiting review label

Apr 14, 2023

Copy link

bedevere-bot commentedApr 14, 2023

Most changes to Pythonrequire a NEWS entry.

Please add it using theblurb_it web app or theblurb command-line tool.

4 similar comments

Copy link

bedevere-bot commentedApr 14, 2023

Most changes to Pythonrequire a NEWS entry.

Please add it using theblurb_it web app or theblurb command-line tool.

Copy link

bedevere-bot commentedApr 14, 2023

Most changes to Pythonrequire a NEWS entry.

Please add it using theblurb_it web app or theblurb command-line tool.

Copy link

bedevere-bot commentedApr 14, 2023

Most changes to Pythonrequire a NEWS entry.

Please add it using theblurb_it web app or theblurb command-line tool.

Copy link

bedevere-bot commentedApr 14, 2023

Most changes to Pythonrequire a NEWS entry.

Please add it using theblurb_it web app or theblurb command-line tool.

gsallam marked this pull request as ready for review

April 16, 2023 20:50

arhadthedev added the interpreter-core(Objects, Python, Grammar, and Parser dirs) label

Apr 19, 2023

Copy link

Contributor

czardoz commentedApr 19, 2023

Looks like the CI is having trouble installing dependencies, I will update this branch with rebase and try again in a bit

gsallamand others added18 commits

April 19, 2023 15:27

Update osmodule.h

f76d2c5

Update posixmodule.c

de6b63e

Update pycore_ceval_state.h

4f55b2d

Update perf_trampoline.c

03fccfb

Update pylifecycle.c

78d543a

Update pycore_ceval.h

e8c321d

Update _testinternalcapi.c

3a97933

Create test_perfmaps.py

94441c4

Update posixmodule.c

a221fd8

Update osmodule.h

392e842

Update osmodule.h

4223241

Update osmodule.h

804153a

Update posixmodule.c

ce54c6c

📜🤖 Added by blurb_it.

6c3f233

Update osmodule.h

71db361

Update test_perfmaps.py

020c872

Update ignored.tsv

e33b45d

Fix trampoline APIs on windows, return int instead of void* from the …

04e9716

…init function

bedevere-bot added the awaiting merge label

May 19, 2023

sunmy2019 requested changes

May 19, 2023

View reviewed changes

Python/perf_trampoline.cShow resolvedHide resolved

facebook-github-bot pushed a commit to facebookincubator/cinder that referenced this pull request

May 19, 2023

Backport perf trampoline

a88f497

Summary: Backport the perf-trampoline introduced inpython/cpython#96123.  The perf trampoline doesn't work properly with the JIT, so we have submitted a PR to have a C-API to unify writing to the perf-map filespython/cpython#103546.Reviewed By: czardozDifferential Revision: D45419843fbshipit-source-id: 16bd13d7981e48c9eb7bc0e5eef1c1f4748965f6

facebook-github-bot pushed a commit to facebookincubator/cinder that referenced this pull request

May 19, 2023

Perfmap C-API and JIT integration

c475629

Summary:With the perf trampoline writing to the perf-map files, we want to have a C-API to unify writing to the perf-map files to avoid file corruption from simultaneous writes. We are trying to upstream the API herepython/cpython#103546. More details about the motivation is in the PR.In addition to introducing the C-API, we also change JIT to utilize the new C-API.Reviewed By: czardozDifferential Revision: D45421966fbshipit-source-id: d270cc753a245f93cbfe3d723d0880595fef45f2

gsallamand others added8 commits

May 19, 2023 17:37

Return error codes instead of setting exceptions in PyUnstable_PerfMa…

d245fbf

…pState_Init

Add a note that holding the GIL is not required for these APIs

2027de6

Remove the init_state call since it is always NULL for the perf backend

0383b80

trim extra whitespaces

59f3c15

simplify link to perf map docs from perf profiling

8bc6150

document return values, other doc tweaks

c7a49b4

move headers to sysmodule.h

125513a

pass through init return value from write-entry

10630e8

carljm requested a review fromsunmy2019

May 20, 2023 03:41

carljm added the 🔨 test-with-buildbotsTest PR w/ buildbots; report in status section label

May 20, 2023

Copy link

bedevere-bot commentedMay 20, 2023

🤖 New build scheduled with the buildbot fleet by@carljm for commit10630e8 🤖

If you want to schedule another build, you need to add the🔨 test-with-buildbots label again.

bedevere-bot removed the 🔨 test-with-buildbotsTest PR w/ buildbots; report in status section label

May 20, 2023

gpshead approved these changes

May 20, 2023

View reviewed changes

sunmy2019 approved these changes

May 20, 2023

View reviewed changes

Python/perf_trampoline.c OutdatedShow resolvedHide resolved

carljm added4 commits

May 19, 2023 21:41

apparently :c:data:errno doesn't exist

386dd42

use PyMem_Raw* instead of malloc/free

6585f32

Merge branch 'main' into gsallam/main

ea5965a

* main: (30 commits)pythongh-103987: fix several crashes in mmap module (python#103990)  docs: fix wrong indentation causing rendering error in dis page (python#104661)pythongh-94906: Support multiple steps in math.nextafter (python#103881)pythongh-104472: Skip `test_subprocess.ProcessTestCase.test_empty_env` if ASAN is enabled (python#104667)pythongh-103839: Allow building Tkinter against Tcl 8.7 without external libtommath (pythonGH-103842)pythongh-85984: New additions and improvements to the tty library. (python#101832)pythongh-104659: Consolidate python examples in enum documentation (python#104665)pythongh-92248: Deprecate `type`, `choices`, `metavar` parameters of `argparse.BooleanOptionalAction` (python#103678)pythongh-104645: fix error handling in marshal tests (python#104646)pythongh-104600: Make type.__type_params__ writable (python#104634)pythongh-104602: Add additional test for listcomp with lambda (python#104639)pythongh-104640: Disallow walrus in comprehension within type scopes (python#104641)pythongh-103921: Rename "type" header in argparse docs (python#104654)  Improve readability of `typing._ProtocolMeta.__instancecheck__` (python#104649)pythongh-96522: Fix deadlock in pty.spawn (python#96639)pythonGH-102818: Do not call `PyTraceBack_Here` in sys.settrace trampoline.  (pythonGH-104579)pythonGH-103545: Add macOS specific constants for ``os.setpriority`` to ``os`` (python#104606)pythongh-104623: Update macOS installer to SQLite 3.42.0 (pythonGH-104624)pythongh-104619: never leak comprehension locals to outer locals() (python#104637)pythongh-104602: ensure all cellvars are known up front (python#104603)  ...

fix PyMem_Free to PyMem_RawFree

e6be2a7

carljm added the 🔨 test-with-buildbotsTest PR w/ buildbots; report in status section label

May 20, 2023

Copy link

bedevere-bot commentedMay 20, 2023

🤖 New build scheduled with the buildbot fleet by@carljm for commite6be2a7 🤖

If you want to schedule another build, you need to add the🔨 test-with-buildbots label again.

bedevere-bot removed the 🔨 test-with-buildbotsTest PR w/ buildbots; report in status section label

May 20, 2023

Copy link

Member

carljm commentedMay 20, 2023•
edited
Loading

Refleak buildbot failures are unrelated, they are all failing intest_mmap, which is a known issue on main:#104698. Otherwise buildbots look happy.

I think this is ready for merge;@pablogsal I will plan to merge it tomorrow evening unless you have objections!

pablogsal merged commitbe0c106 intopython:main

May 21, 2023

bedevere-bot removed the awaiting merge label

May 21, 2023

Copy link

Member

pablogsal commentedMay 21, 2023

I think this is ready for merge;@pablogsal I will plan to merge it tomorrow evening unless you have objections!

I went ahead and land it myself after another review pass 😉

Great job everyone!

facebook-github-bot pushed a commit to facebookincubator/cinder that referenced this pull request

May 24, 2023

User error codes and fix 32-bit system issues

0a7cc0b

Summary:Port the latest changes from the [perf-map C-API PR](python/cpython#103546). This includes using error codes instead of exceptions to avoid the need to hold the GIL.Additionally, there was an issue with Fedora on a 32-bit system that was fixed in this [PR](python/cpython#104811), so I also ported the fix.Reviewed By: carljmDifferential Revision: D46041553fbshipit-source-id: 6502189f489ca6cf11949ecd93c044e5ebe4fd2c