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 — [3.11] GH-93516: Backport GH-93769 by markshannon · Pull Request #94231 · python/cpython

Conversation

@markshannon

@markshannon markshannon commented Jun 24, 2022 •
edited by bedevere-bot
Loading

Uh oh!

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

Copy link
Copy Markdown
Member

Reduces the overhead of dispatching in DO_TRACING considerably.

My informal measures on the test program in #93516 (comment) show that this reduces execution time by about 10%

@pablogsal Should I regenerate the ABI for this PR?

  • Issue: Severe performance degradation for tracing under 3.11 #93516

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
markshannon added 4 commits June 24, 2022 17:01
@markshannon
Store offset of first traceable instruction to avoid having to recomp…
4a8e2fa 
…ute it all the time when tracing.
@markshannon
Hardcode RESUME opcode in deepfreeze, to break dependency on exact ve…
71dd6c8 
…rsion of Python used.
@markshannon
Handle empty code objects, and other invalid code, gracefully.
588d5fe
@markshannon
Check bounds before data
643a614
@bedevere-bot bedevere-bot added the awaiting core review label Jun 24, 2022
@arhadthedev

arhadthedev commented Jun 24, 2022 •
edited
Loading

Uh oh!

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

Copy link
Copy Markdown
Member

Probably the issue should be renamed to ... Backport GH-93769 for a clickable number in https://github.com/python/cpython/commits/main (though, it's just a nitpick).
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@markshannon markshannon changed the title [3.11] GH-93516: Backport 93769 [3.11] GH-93516: Backport GH-93769 Jun 24, 2022
@Fidget-Spinner Fidget-Spinner mentioned this pull request Jun 25, 2022
Severe performance degradation for tracing under 3.11 #93516
Open
@markshannon

markshannon commented Jun 28, 2022

Copy link
Copy Markdown
Member Author

@pablogsal I'd like to get this merged, as it will be needed for the backport of #94371
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
pablogsal
pablogsal reviewed Jun 28, 2022
View reviewed changes
Comment thread Tools/scripts/deepfreeze.py Resolved

Uh oh!

There was an error while loading. Please reload this page.
pablogsal
pablogsal approved these changes Jun 28, 2022
View reviewed changes

@pablogsal pablogsal left a comment

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

Looks good to me 👍

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
@bedevere-bot bedevere-bot added awaiting merge and removed awaiting core review labels Jun 28, 2022
@pablogsal

pablogsal commented Jun 28, 2022 •
edited
Loading

Uh oh!

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

Copy link
Copy Markdown
Member

@pablogsal Should I regenerate the ABI for this PR?

I can try to do it for you. Otherwise you can follow these instructions:

@pablogsal Should I regenerate the ABI for this PR?
All reactions

Sorry, something went wrong.

Uh oh!

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

pablogsal commented Jun 28, 2022

Copy link
Copy Markdown
Member

I cannot push to the PR, but you can copy this commit into yours if you want:

pablogsal@8380dd0
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
markshannon added 2 commits June 28, 2022 13:52
@markshannon
Assert that _co_firsttraceable is a RESUME
fe6a1d3
@markshannon
Regen ABI doc.
a0ced71
@markshannon

markshannon commented Jun 28, 2022

Copy link
Copy Markdown
Member Author

Hmm, I followed the instructions, yet a0ced71 is clearly different to pablogsal@8380dd0
All reactions

Sorry, something went wrong.

Uh oh!

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

markshannon commented Jun 28, 2022

Copy link
Copy Markdown
Member Author

I see, it is because of the assert changing the line numbers.
Why does an application binary interface include line numbers?
All reactions

Sorry, something went wrong.

Uh oh!

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

pablogsal commented Jun 28, 2022

Copy link
Copy Markdown
Member

Why does an application binary interface include line numbers?

Those are not taken into account when checking it. They just come from the DWARF if I am not mistaken
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
pablogsal
pablogsal approved these changes Jun 28, 2022
View reviewed changes

@pablogsal pablogsal left a comment

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

LGTM !

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
@markshannon
markshannon merged commit 3b4f5ed into python:3.11 Jun 28, 2022
@bedevere-bot bedevere-bot removed the awaiting merge label Jun 28, 2022
@markshannon
markshannon deleted the backport-93769 branch September 26, 2023 12:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@pablogsal pablogsal pablogsal approved these changes

Uh oh!

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

Assignees

No one assigned

Labels

None yet

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.

4 participants

@markshannon @arhadthedev @pablogsal @bedevere-bot
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.