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-103650: Fix perf maps address format by art049 · Pull Request #103651 · python/cpython

Conversation

@art049

@art049 art049 commented Apr 20, 2023 •
edited by arhadthedev
Loading

Uh oh!

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

Copy link
Copy Markdown
Contributor
  • Issue: perf maps address format #103650

Sorry, something went wrong.

Uh oh!

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

bedevere-bot commented Apr 20, 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 20, 2023
perf maps address format #103650
Closed
@bedevere-bot bedevere-bot added the awaiting review label Apr 20, 2023
@ghost

ghost commented Apr 20, 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.
@art049
art049 force-pushed the fix/perf-maps-format branch from 6cb2f11 to 8af8b2b Compare April 20, 2023 16:15
@bedevere-bot

bedevere-bot commented Apr 20, 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.
@art049
art049 force-pushed the fix/perf-maps-format branch from ddc2b2d to 5df35f0 Compare April 20, 2023 16:25
sunmy2019
sunmy2019 requested changes Apr 20, 2023
View reviewed changes
Comment thread Python/perf_trampoline.c Resolved

Uh oh!

There was an error while loading. Please reload this page.
@bedevere-bot bedevere-bot added awaiting core review and removed awaiting review labels Apr 20, 2023
@arhadthedev arhadthedev added interpreter-core (Objects, Python, Grammar, and Parser dirs) 3.11 only security fixes 3.12 only security fixes labels Apr 20, 2023
@art049
art049 force-pushed the fix/perf-maps-format branch from db8707f to 4e43459 Compare April 20, 2023 17:14
art049 added 2 commits April 20, 2023 19:55
@art049
Fix perf maps address format
f4d1de8
@art049
fix pointer type
0ba49e2
@art049
art049 force-pushed the fix/perf-maps-format branch from 4e43459 to 0ba49e2 Compare April 20, 2023 17:55
@terryjreedy
terryjreedy requested a review from pablogsal April 20, 2023 18:45
@terryjreedy

terryjreedy commented Apr 20, 2023

Copy link
Copy Markdown
Member

I think that this should wait until Pablo verifies that it is indeed the implementation that should be changed.
art049 reacted with thumbs up emoji
All reactions
  • 👍 1 reaction

Sorry, something went wrong.

Uh oh!

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

carljm commented May 5, 2023

Copy link
Copy Markdown
Member

@art049 did you verify that the new output after this PR works the same with perf?

@terryjreedy since the relevant docs here are the external perf map specification, not the CPython docs, it seems clear that the implementation in CPython should match the specification. There is no option to change the specification, we don't own it.
All reactions

Sorry, something went wrong.

Uh oh!

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

art049 commented May 5, 2023

Copy link
Copy Markdown
Contributor Author

@art049 did you verify that the new output after this PR works the same with perf?

Yes, also there are existing tests checking this:

cpython/Lib/test/test_perf_profiler.py

Lines 304 to 327 in 0ba49e2

def test_python_calls_appear_in_the_stack_if_perf_activated(self):
with temp_dir() as script_dir:
code = """if 1:
def foo(n):
x = 0
for i in range(n):
x += i
def bar(n):
foo(n)
def baz(n):
bar(n)
baz(10000000)
"""
script = make_script(script_dir, "perftest", code)
stdout, stderr = run_perf(script_dir, sys.executable, "-Xperf", script)
self.assertEqual(stderr, "")
self.assertIn(f"py::foo:{script}", stdout)
self.assertIn(f"py::bar:{script}", stdout)
self.assertIn(f"py::baz:{script}", stdout)

carljm reacted with thumbs up emoji
All reactions
  • 👍 1 reaction

Sorry, something went wrong.

Uh oh!

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

bedevere-bot commented May 7, 2023

Copy link
Copy Markdown

🤖 New build scheduled with the buildbot fleet by @pablogsal for commit 0ba49e2 🤖

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 7, 2023
@pablogsal

pablogsal commented May 7, 2023

Copy link
Copy Markdown
Member

Running this PR with the buildbots as the perf stuff is not checked on regular CI.
art049 reacted with thumbs up emoji
All reactions
  • 👍 1 reaction

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
pablogsal
pablogsal approved these changes May 7, 2023
View reviewed changes
@bedevere-bot bedevere-bot added awaiting merge and removed awaiting core review labels May 7, 2023
@pablogsal
pablogsal merged commit 8d95012 into python:main May 7, 2023
@bedevere-bot bedevere-bot removed the awaiting merge label May 7, 2023
jbower-fb pushed a commit to jbower-fb/cpython that referenced this pull request May 8, 2023
@art049 @jbower-fb
pythongh-103650: Fix perf maps address format (python#103651)
44914e0
carljm added a commit to carljm/cpython that referenced this pull request May 9, 2023
@carljm
Merge branch 'main' into inlinecomp2
76077cd 
* main: (47 commits)
  pythongh-97696 Remove unnecessary check for eager_start kwarg (python#104188)
  pythonGH-104308: socket.getnameinfo should release the GIL (python#104307)
  pythongh-104310: Add importlib.util.allowing_all_extensions() (pythongh-104311)
  pythongh-99113: A Per-Interpreter GIL! (pythongh-104210)
  pythonGH-104284: Fix documentation gettext build (python#104296)
  pythongh-89550: Buffer GzipFile.write to reduce execution time by ~15% (python#101251)
  pythongh-104223: Fix issues with inheriting from buffer classes (python#104227)
  pythongh-99108: fix typo in Modules/Setup (python#104293)
  pythonGH-104145: Use fully-qualified cross reference types for the bisect module (python#104172)
  pythongh-103193: Improve `getattr_static` test coverage (python#104286)
  Trim trailing whitespace and test on CI (python#104275)
  pythongh-102500: Remove mention of bytes shorthand (python#104281)
  pythongh-97696: Improve and fix documentation for asyncio eager tasks (python#104256)
  pythongh-99108: Replace SHA3 implementation HACL* version (python#103597)
  pythongh-104273: Remove redundant len() calls in argparse function (python#104274)
  pythongh-64660: Don't hardcode Argument Clinic return converter result variable name (python#104200)
  pythongh-104265 Disallow instantiation of `_csv.Reader` and `_csv.Writer` (python#104266)
  pythonGH-102613: Improve performance of `pathlib.Path.rglob()` (pythonGH-104244)
  pythongh-103650: Fix perf maps address format (python#103651)
  pythonGH-89812: Churn `pathlib.Path` methods (pythonGH-104243)
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@sunmy2019 sunmy2019 sunmy2019 requested changes
@pablogsal pablogsal pablogsal approved these changes

Uh oh!

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

Assignees

No one assigned

Labels

3.11 only security fixes 3.12 only security fixes 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.

7 participants

@art049 @bedevere-bot @terryjreedy @carljm @pablogsal @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.