-
-
Notifications
You must be signed in to change notification settings - Fork 33.6k
gh-115178: Add Counts of UOp Pairs to pystats #115181
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 our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Make optimization_stats.opcode an array of pointers to UOpStats objects, instead of a member array. Add a *next_stats[512] field to the UOpStats struct Add _init_pystats function to initialize these new member structs Call _init_pystats inside _PyConfig_Write
Add print_uop_sequence function to specialize.c to output stored chains of uops
Sets the maximum depth of UOP sequences to track. Defaults to 2.
This reverts commit 03db7a5.
|
Thank you both! @erlend-aasland your fix recipe worked a treat - I will be more careful about merging from main/pushing in the future. |
markshannon
left a comment
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.
A couple of minor issues, but looks good in general.
|
A Python core developer has requested some changes be made to your pull request before we can consider merging it. If you could please address their requests along with any other requests in other reviews from core developers that would be appreciated. Once you have made the requested changes, please leave a comment on this pull request containing the phrase |
|
Thank you for the corrections. I have made the requested changes; please review again. |
|
@markshannon I'm going to merge this soon. @JeffersGlass could you please fix the merge conflicts? Thanks! |
|
Thanks @Fidget-Spinner - I've caught up with main and tests are passing. |
|
Actually @Fidget-Spinner before you merge, I’ve seen something I missed in this morning’s merge from main, let me take care of that later today. |
|
Ok just ping me again when you feel its ready, I will take a look then! |
|
@Fidget-Spinner I've made the updates I wanted to, to fix detection of the different kinds of opcode pairs and give them good labels in the output. I've also just rebased on top of main; I think this is ready for review. |
markshannon
left a comment
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.
Thanks @JeffersGlass for doing this.
This is going to be very useful for selecting micro-op pairs for the JIT.
This follows up [115181](python/cpython#115181), and adds a note about the presence of Uop pair counts in pystats.
This follows up [115181](python/cpython#115181), and adds a note about the presence of Uop pair counts in pystats.
Adds the ability to track pairs, triples, and longer sequences of UOps to the
pystatsstatistics.Currently, this only counts these statistics for the non-JIT tier 2 interpreter. I intend to include tracking in the JIT in a follow-up PR.
To try it out, run:
./configure --enable-pystats ./make mkdir /tmp/py_stats PYTHON_UOPS=1 ./python -X pystats -c $'x = 0\nfor i in range(1000):\n x+=i'The output file at
/tmp/py_stats/???.txtwill contain a new section with UOp Sequence counts:Tools/scripts/summarize_stats.pyhas also been expanded to encompass these new stats, with a new section:Counts for top 100 UOp Sequences of Length 2
Finally, a new environment variable,
PYTHONSTATS_UOPDEPTHcan be set to any integer to record sequences of length longer than two. For example:PYTHONSTATS_UOPDEPTH=4 PYTHON_UOPS=1 ./python -X pystats -c 'x = 0; for i in range(1000): x+=iYields:
Counts for top 100 UOp Sequences of Length 2
Counts for top 100 UOp Sequences of Length 3
Counts for top 100 UOp Sequences of Length 4
pystats#115178EDIT: I see this has automatically requested a review from a huge number of people. This is my first time submitting a PR to CPython - please do forgive me if I've goofed this process up somehow.
📚 Documentation preview 📚: https://cpython-previews--115181.org.readthedocs.build/