GV

GeezerVison

The World Wide Web, restored to 800 pixels of glory.

Home|About|Gaming cabinet|Archives|How it works|Starter sites

On this site

  • Sign in
  • ActionsCI/CD and automation
  • CodespacesCloud development environments
  • All Features
  • Documentation
  • Training
  • Certifications
  • Build with Copilot
  • Customize Copilot
  • Integrations
  • Trending repos
  • GitHub Sponsors
  • Developer articles
  • Enterprises

This view

  • Original page
  • New address

GeezerVison is showing github.com — gh-103295: expose API for writing perf map files by gsallam · Pull Request #103546 · python/cpython

Conversation

@gsallam

@gsallam gsallam commented Apr 14, 2023 •
edited by bedevere-bot
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Contributor

#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 and Pyston, 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

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
EdwaRen reacted with hooray emoji
All reactions
  • 🎉 1 reaction
@ghost

ghost commented Apr 14, 2023 •
edited by ghost
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown

All commit authors signed the Contributor License Agreement.
CLA signed
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@bedevere-bot

bedevere-bot commented Apr 14, 2023

Copy link
Copy Markdown

Most changes to Python require a NEWS entry.

Please add it using the blurb_it web app or the blurb command-line tool.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@bedevere-bot bedevere-bot mentioned this pull request Apr 14, 2023
expose API for writing perf map files #103295
Closed
@bedevere-bot bedevere-bot added the awaiting review label Apr 14, 2023
@bedevere-bot

bedevere-bot commented Apr 14, 2023

Copy link
Copy Markdown

Most changes to Python require a NEWS entry.

Please add it using the blurb_it web app or the blurb command-line tool.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
4 similar comments
@bedevere-bot

bedevere-bot commented Apr 14, 2023

Copy link
Copy Markdown

Most changes to Python require a NEWS entry.

Please add it using the blurb_it web app or the blurb command-line tool.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@bedevere-bot

bedevere-bot commented Apr 14, 2023

Copy link
Copy Markdown

Most changes to Python require a NEWS entry.

Please add it using the blurb_it web app or the blurb command-line tool.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@bedevere-bot

bedevere-bot commented Apr 14, 2023

Copy link
Copy Markdown

Most changes to Python require a NEWS entry.

Please add it using the blurb_it web app or the blurb command-line tool.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@bedevere-bot

bedevere-bot commented Apr 14, 2023

Copy link
Copy Markdown

Most changes to Python require a NEWS entry.

Please add it using the blurb_it web app or the blurb command-line tool.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@gsallam
gsallam marked this pull request as ready for review April 16, 2023 20:50
@arhadthedev arhadthedev added the interpreter-core (Objects, Python, Grammar, and Parser dirs) label Apr 19, 2023
@czardoz

czardoz commented Apr 19, 2023

Copy link
Copy Markdown
Contributor

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

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
gsallam and others added 18 commits April 19, 2023 15:27
@gsallam @czardoz
Update osmodule.h
f76d2c5
@gsallam @czardoz
Update posixmodule.c
de6b63e
@gsallam @czardoz
Update pycore_ceval_state.h
4f55b2d
@gsallam @czardoz
Update perf_trampoline.c
03fccfb
@gsallam @czardoz
Update pylifecycle.c
78d543a
@gsallam @czardoz
Update pycore_ceval.h
e8c321d
@gsallam @czardoz
Update _testinternalcapi.c
3a97933
@gsallam @czardoz
Create test_perfmaps.py
94441c4
@gsallam @czardoz
Update posixmodule.c
a221fd8
@gsallam @czardoz
Update osmodule.h
392e842
@gsallam @czardoz
Update osmodule.h
4223241
@gsallam @czardoz
Update osmodule.h
804153a
@gsallam @czardoz
Update posixmodule.c
ce54c6c
@blurb-it @czardoz
📜🤖 Added by blurb_it.
6c3f233
@gsallam @czardoz
Update osmodule.h
71db361
@gsallam @czardoz
Update test_perfmaps.py
020c872
@gsallam @czardoz
Update ignored.tsv
e33b45d
@czardoz
Fix trampoline APIs on windows, return int instead of void* from the …
04e9716 
…init function
@bedevere-bot bedevere-bot added the awaiting merge label May 19, 2023
sunmy2019
sunmy2019 requested changes May 19, 2023
View reviewed changes
Comment thread Python/perf_trampoline.c Resolved

Uh oh!

There was an error while loading. Please reload this page.
facebook-github-bot pushed a commit to facebookincubator/MetaPython that referenced this pull request May 19, 2023
Gamal Sallam @facebook-github-bot
Backport perf trampoline
a88f497 
Summary: Backport the perf-trampoline introduced in python/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 files python/cpython#103546.

Reviewed By: czardoz

Differential Revision: D45419843

fbshipit-source-id: 16bd13d7981e48c9eb7bc0e5eef1c1f4748965f6
facebook-github-bot pushed a commit to facebookincubator/MetaPython that referenced this pull request May 19, 2023
Gamal Sallam @facebook-github-bot
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 here python/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: czardoz

Differential Revision: D45421966

fbshipit-source-id: d270cc753a245f93cbfe3d723d0880595fef45f2
gsallam and others added 8 commits May 19, 2023 17:37
@gsallam
Return error codes instead of setting exceptions in PyUnstable_PerfMa…
d245fbf 
…pState_Init
@gsallam
Add a note that holding the GIL is not required for these APIs
2027de6
@gsallam
Remove the init_state call since it is always NULL for the perf backend
0383b80
@gsallam
trim extra whitespaces
59f3c15
@carljm
simplify link to perf map docs from perf profiling
8bc6150
@carljm
document return values, other doc tweaks
c7a49b4
@carljm
move headers to sysmodule.h
125513a
@carljm
pass through init return value from write-entry
10630e8
@carljm
carljm requested a review from sunmy2019 May 20, 2023 03:41
@carljm carljm added the 🔨 test-with-buildbots Test PR w/ buildbots; report in status section label May 20, 2023
@bedevere-bot

bedevere-bot commented May 20, 2023

Copy link
Copy Markdown

🤖 New build scheduled with the buildbot fleet by @carljm for commit 10630e8 🤖

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

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@bedevere-bot bedevere-bot removed the 🔨 test-with-buildbots Test PR w/ buildbots; report in status section label May 20, 2023
gpshead
gpshead approved these changes May 20, 2023
View reviewed changes
sunmy2019
sunmy2019 approved these changes May 20, 2023
View reviewed changes
Comment thread Python/perf_trampoline.c Resolved

Uh oh!

There was an error while loading. Please reload this page.
Comment thread Python/perf_trampoline.c Resolved

Uh oh!

There was an error while loading. Please reload this page.
carljm added 4 commits May 19, 2023 21:41
@carljm
apparently :c:data:errno doesn't exist
386dd42
@carljm
use PyMem_Raw* instead of malloc/free
6585f32
@carljm
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)
  ...
@carljm
fix PyMem_Free to PyMem_RawFree
e6be2a7
@carljm carljm added the 🔨 test-with-buildbots Test PR w/ buildbots; report in status section label May 20, 2023
@bedevere-bot

bedevere-bot commented May 20, 2023

Copy link
Copy Markdown

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

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

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@bedevere-bot bedevere-bot removed the 🔨 test-with-buildbots Test PR w/ buildbots; report in status section label May 20, 2023
@carljm

carljm commented May 20, 2023 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Member

Refleak buildbot failures are unrelated, they are all failing in test_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!
Eclips4 and pablogsal reacted with thumbs up emoji
All reactions
  • 👍 2 reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@pablogsal
pablogsal merged commit be0c106 into python:main May 21, 2023
@bedevere-bot bedevere-bot removed the awaiting merge label May 21, 2023
@pablogsal

pablogsal commented May 21, 2023

Copy link
Copy Markdown
Member

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!
sunmy2019, gsallam, and carljm reacted with hooray emoji
All reactions
  • 🎉 3 reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
facebook-github-bot pushed a commit to facebookincubator/MetaPython that referenced this pull request May 24, 2023
Gamal Sallam @facebook-github-bot
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: carljm

Differential Revision: D46041553

fbshipit-source-id: 6502189f489ca6cf11949ecd93c044e5ebe4fd2c
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@carljm carljm carljm left review comments
@pablogsal pablogsal pablogsal left review comments
@gpshead gpshead gpshead approved these changes
@sunmy2019 sunmy2019 sunmy2019 approved these changes
+1 more reviewer
@czardoz czardoz czardoz left review comments
Reviewers whose approvals may not affect merge requirements

Uh oh!

There was an error while loading. Please reload this page.

Assignees

No one assigned

Labels

interpreter-core (Objects, Python, Grammar, and Parser dirs)

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Uh oh!

There was an error while loading. Please reload this page.

8 participants

@gsallam @bedevere-bot @czardoz @carljm @pablogsal @gpshead @sunmy2019 @arhadthedev
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.