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-112354: Initial implementation of warm up on exits and trace-stitching by markshannon · Pull Request #114142 · python/cpython

Conversation

@markshannon

@markshannon markshannon commented Jan 16, 2024 •
edited by gvanrossum
Loading

Uh oh!

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

Copy link
Copy Markdown
Member

Only works for boolean guards, not type guards, for now. And _EXIT_TRACE now.

Needs to be carefully documented, there's some subtlety here.

Exits are implemented as an array of records attached to the executors, and a fixed set of cold exit executors.
We can shrink the _PyExitData further, by moving the counters into a global table, but that's for another PR.

See faster-cpython/ideas#644 for a rough sketch of the design.

  • Issue: Trace stitching #112354

Sorry, something went wrong.

Uh oh!

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

All reactions
markshannon added 8 commits January 10, 2024 14:51
@markshannon
Cold exits: Work in progress.
1975b4c
@markshannon
Merge branch 'main' into cold-exits
9fb97f7
@markshannon
Optimize on side exits
f9aa235
@markshannon
Merge branch 'main' into cold-exits
1288258
@markshannon
Modify internal interfaces
7c6267a
@markshannon
Merge branch 'main' into cold-exits
55c48e8
@markshannon
Jump to next executor without updating current_executor.
8b3c2e0
@markshannon
Support cycle GC for executors.
92a3b61
@bedevere-app bedevere-app Bot mentioned this pull request Jan 16, 2024
Trace stitching #112354
Closed
markshannon added 3 commits January 16, 2024 19:54
@markshannon
Give cold exits their own class, to fix GC handling of exits
e3def48
@markshannon
Generate table of cold exits
87e544b
@markshannon
Treat EXIT_TRACE as a side exit
2172d68
@brandtbucher
brandtbucher self-requested a review January 16, 2024 21:26
markshannon added 5 commits January 16, 2024 21:49
@markshannon
Treat most common guard failures as side exits
4448793
@markshannon
Tweak generated tble to help C analyzer
c70f12f
@markshannon
Add some documentation about the tier 2 engine
d73fe0a
@markshannon
Fix constness and rename hotness
5c8f0bd
@markshannon
Add new static objects to ignored file.
140486b
@markshannon

markshannon commented Jan 17, 2024

Copy link
Copy Markdown
Member Author

Benchmarks and stats: https://github.com/faster-cpython/benchmarking-public/tree/main/results/bm-20240116-3.13.0a2%2B-92a3b61-PYTHON_UOPS

All reactions

Sorry, something went wrong.

Uh oh!

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

@markshannon markshannon added the skip news label Jan 17, 2024
@markshannon
markshannon marked this pull request as ready for review January 17, 2024 15:55
@markshannon
markshannon requested a review from gvanrossum as a code owner January 17, 2024 15:55
@bedevere-app bedevere-app Bot added the awaiting core review label Jan 17, 2024
@markshannon
Address review comments
3362c93
@brandtbucher

brandtbucher commented Jan 18, 2024

Copy link
Copy Markdown
Member

Sorry, didn't get to a proper review of this today. I've been trying (and failing) to merge the JIT branch into this one without crashing.

Maybe we can chat "in person" later?

All reactions

Sorry, something went wrong.

Uh oh!

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

@brandtbucher

brandtbucher commented Jan 18, 2024

Copy link
Copy Markdown
Member

Okay, I was able to get it working-ish... but it's not pretty. We can go over it when we meet.

All reactions

Sorry, something went wrong.

Uh oh!

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

@markshannon
markshannon requested a review from ericsnowcurrently as a code owner January 18, 2024 11:10
@markshannon

markshannon commented Jan 18, 2024

Copy link
Copy Markdown
Member Author

Stats: https://github.com/faster-cpython/benchmarking-public/tree/main/results/bm-20240116-3.13.0a2%2B-c70f12f-PYTHON_UOPS

All reactions

Sorry, something went wrong.

Uh oh!

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

gvanrossum
gvanrossum reviewed Jan 19, 2024
View reviewed changes

@gvanrossum gvanrossum 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.

Here's a first, partial pass. I've tried to focus on things that confused me. I haven't fully reviewed optimizer.c yet, nor the _COLD_EXIT uop definition, so I'll focus on those when I find some time.

Maybe it would be useful to add TODO comments indicating things you're planning to tackle in the near future (subsequent PRs)? Like the counters.

Sorry, something went wrong.

Uh oh!

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

All reactions
Comment thread Python/tier2_engine.md Resolved

Uh oh!

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

Comment thread Python/tier2_engine.md Resolved

Uh oh!

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

Comment thread Python/tier2_engine.md Resolved

Uh oh!

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

Comment thread Python/tier2_engine.md Resolved

Uh oh!

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

Comment thread Python/tier2_engine.md Resolved

Uh oh!

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

Comment thread Python/bytecodes.c Resolved

Uh oh!

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

Comment thread Python/ceval.c Resolved

Uh oh!

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

Comment thread Python/bytecodes.c Resolved

Uh oh!

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

Comment thread Python/optimizer.c Resolved

Uh oh!

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

Comment thread Include/cpython/optimizer.h Resolved

Uh oh!

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

@gvanrossum gvanrossum mentioned this pull request Jan 21, 2024
gh-114058: The Tier2 Optimizer #114059
Closed
@markshannon

markshannon commented Feb 16, 2024

Copy link
Copy Markdown
Member Author

Or better, executor_blah, matching e.g. executor_clear.

I'd prefer to save that for another PR. Maybe the one that removes the CounterExecutor.

All reactions

Sorry, something went wrong.

Uh oh!

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

@markshannon

markshannon commented Feb 16, 2024

Copy link
Copy Markdown
Member Author

FTR, the thresholds are likely to get completely changed soon.

We probably want to change the fixed thresholds to some sort of adaptive thresholds, and consider the T1 and T2 thresholds together so that specialization works correctly.

gvanrossum 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.

gvanrossum
gvanrossum approved these changes Feb 16, 2024
View reviewed changes
@bedevere-app bedevere-app Bot added awaiting merge and removed awaiting core review labels Feb 16, 2024
mdboom
mdboom reviewed Feb 16, 2024
View reviewed changes
Comment thread Include/internal/pycore_uop_metadata.h Resolved

Uh oh!

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

mdboom
mdboom reviewed Feb 16, 2024
View reviewed changes
Comment thread Python/tier2_engine.md Resolved

Uh oh!

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

Comment thread Python/tier2_engine.md Resolved

Uh oh!

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

Comment thread Python/tier2_redundancy_eliminator_cases.c.h Resolved

Uh oh!

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

@gvanrossum

gvanrossum commented Feb 16, 2024

Copy link
Copy Markdown
Member

If you look at GitHub there’s still a missing cast somewhere.

markshannon 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.

brandtbucher
brandtbucher requested changes Feb 16, 2024
View reviewed changes

@brandtbucher brandtbucher 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.

Didn't look too closely at the cases generator or optimizer (I'm assuming there are other eyes on that).

  • I appreciate the writeup!
  • I like that the interpreter/JIT interfaces are a lot more unified on this iteration. I know you're not crazy about conditional macro stuff but I think it makes things a lot easier to follow in this case. Thanks.
  • Suggestions to remove bit of tricky code duplication in template.c.
  • A couple of questions about setting tstate->previous_executor.
  • A few other random questions/suggestions I had while reading through.
  • One thing I find sort of tricky to keep track of is the nuances of the first couple of instructions in the trace (when/where _START_EXECUTOR is added, when/where pointers to optimizers/executors are smuggled in as operands, etc). This might be worth cleaning up now or in the future, or at least writing up. Not sure.

Sorry, something went wrong.

Uh oh!

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

All reactions
Comment thread Tools/jit/template.c Resolved

Uh oh!

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

Comment thread Tools/jit/template.c Resolved

Uh oh!

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

Comment thread Tools/jit/template.c Resolved

Uh oh!

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

Comment thread Tools/jit/template.c Resolved

Uh oh!

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

Comment thread Tools/jit/template.c Resolved

Uh oh!

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

Comment thread Python/optimizer.c Resolved

Uh oh!

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

Comment thread Include/cpython/optimizer.h Resolved

Uh oh!

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

Comment thread Python/tier2_engine.md Resolved

Uh oh!

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

Comment thread Python/bytecodes.c Resolved

Uh oh!

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

Comment thread Tools/jit/template.c Resolved

Uh oh!

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

@bedevere-app bedevere-app Bot added awaiting changes and removed awaiting merge labels Feb 16, 2024
@bedevere-app

bedevere-app Bot commented Feb 16, 2024

Copy link
Copy Markdown

When you're done making the requested changes, leave the comment: I have made the requested changes; please review again.

All reactions

Sorry, something went wrong.

Uh oh!

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

iritkatriel
iritkatriel reviewed Feb 19, 2024
View reviewed changes
Comment thread Python/optimizer.c Resolved

Uh oh!

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

Comment thread Modules/_testinternalcapi.c Resolved

Uh oh!

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

Comment thread Python/tier2_engine.md Resolved

Uh oh!

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

Comment thread Python/tier2_engine.md Resolved

Uh oh!

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

Comment thread Python/tier2_engine.md Resolved

Uh oh!

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

@markshannon
markshannon requested a review from brandtbucher February 19, 2024 17:38
@markshannon markshannon removed the awaiting changes label Feb 19, 2024
iritkatriel
iritkatriel approved these changes Feb 19, 2024
View reviewed changes
@bedevere-app bedevere-app Bot added the awaiting merge label Feb 19, 2024
@markshannon
markshannon merged commit 7b21403 into python:main Feb 20, 2024
@bedevere-app bedevere-app Bot removed the awaiting merge label Feb 20, 2024
@encukou

encukou commented Feb 20, 2024

Copy link
Copy Markdown
Member

The refleaks buildbot failed on test.test_capi.test_opt.TestUops.test_confidence_score. Likely causes: GH-114142, GH-115558, or GH-115688
I'll investigate later if it's not fixed; you probably have more context.

All reactions

Sorry, something went wrong.

Uh oh!

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

This was referenced Feb 20, 2024
GH-115457: Support splitting and replication of micro ops. #115558
Merged
gh-115687: Split up guards from COMPARE_OP #115688
Merged
@markshannon

markshannon commented Feb 20, 2024

Copy link
Copy Markdown
Member Author

I don't think there is a new bug here, but this PR exposes it. #115727

All reactions

Sorry, something went wrong.

Uh oh!

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

@brandtbucher brandtbucher mentioned this pull request Feb 28, 2024
Compiling tiny traces wastes lots of memory #116017
Open
woodruffw pushed a commit to woodruffw-forks/cpython that referenced this pull request Mar 4, 2024
@markshannon @woodruffw
pythonGH-112354: Initial implementation of warm up on exits and trace…
473c3a6
…-stitching (pythonGH-114142)
diegorusso pushed a commit to diegorusso/cpython that referenced this pull request Apr 17, 2024
@markshannon @diegorusso
pythonGH-112354: Initial implementation of warm up on exits and trace…
d097785
…-stitching (pythonGH-114142)
This was referenced Apr 19, 2024
Optimizer/executor crash when using address sanitizer #118113
Closed
global-buffer-overflow in test_opt.py #118074
Closed
@markshannon
markshannon deleted the cold-exits branch August 6, 2024 10:17
LukasWoodtli pushed a commit to LukasWoodtli/cpython that referenced this pull request Jan 22, 2025
@markshannon @LukasWoodtli
pythonGH-112354: Initial implementation of warm up on exits and trace…
68fdb88
…-stitching (pythonGH-114142)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@mdboom mdboom mdboom left review comments
@iritkatriel iritkatriel iritkatriel approved these changes
@gvanrossum gvanrossum gvanrossum approved these changes
@ericsnowcurrently ericsnowcurrently Awaiting requested review from ericsnowcurrently ericsnowcurrently is a code owner
@brandtbucher brandtbucher Awaiting requested review from brandtbucher brandtbucher is a code owner

Uh oh!

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

Assignees

No one assigned

Labels

skip news

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.

6 participants

@markshannon @brandtbucher @gvanrossum @encukou @mdboom @iritkatriel
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.