Skip to content

gh-90997: bpo-46841: Disassembly of quickened code#32099

Merged
markshannon merged 16 commits into
python:mainfrom
penguin-wwy:dis_quickened_code
Apr 19, 2022
Merged

gh-90997: bpo-46841: Disassembly of quickened code#32099
markshannon merged 16 commits into
python:mainfrom
penguin-wwy:dis_quickened_code

Conversation

@penguin-wwy

@penguin-wwy penguin-wwy commented Mar 24, 2022 •
edited
Loading

Copy link
Copy Markdown
Contributor

@brandtbucher

brandtbucher commented Mar 24, 2022

Copy link
Copy Markdown
Member

Is this ready for review?

If not, perhaps mark it as a "draft" for now.

@brandtbucher brandtbucher self-requested a review March 24, 2022 17:12
@brandtbucher brandtbucher mentioned this pull request Mar 24, 2022
Closed
26 tasks
@penguin-wwy penguin-wwy marked this pull request as draft March 24, 2022 18:03
@penguin-wwy penguin-wwy marked this pull request as ready for review March 25, 2022 14:43
markshannon
markshannon reviewed Apr 1, 2022
View reviewed changes
Comment thread Lib/opcode.py Outdated
@markshannon

markshannon commented Apr 1, 2022

Copy link
Copy Markdown
Member

Looks promising. Will need some tests for the new output format.

@penguin-wwy

penguin-wwy commented Apr 1, 2022

Copy link
Copy Markdown
Contributor Author

One problem is that some instructions are too long(like PRECALL_BUILTIN_FAST_WITH_KEYWORDS), do I need to increase _OPNAME_WIDTH

def adaptive_test(a, b):
    c = a + b
    print(c.__class__)

Exec dis(adaptive_test):

 30           0 RESUME                   0

 31           2 LOAD_FAST                0 (a)
              4 LOAD_FAST                1 (b)
              6 BINARY_OP                0 (+)
             10 STORE_FAST               2 (c)

 32          12 LOAD_GLOBAL              1 (NULL + print)
             24 LOAD_FAST                2 (c)
             26 LOAD_ATTR                1 (__class__)
             36 PRECALL                  1
             40 CALL                     1
             50 POP_TOP
             52 LOAD_CONST               0 (None)
             54 RETURN_VALUE

Exec dis(adaptive_test, adaptive=True):

 30           0 RESUME_QUICK             0

 31           2 LOAD_FAST__LOAD_FAST     0 (a)
              4 LOAD_FAST                1 (b)
              6 BINARY_OP_ADD_INT        0 (+)
             10 STORE_FAST               2 (c)

 32          12 LOAD_GLOBAL_BUILTIN      1 (NULL + print)
             24 LOAD_FAST                2 (c)
             26 LOAD_ATTR_SLOT           1 (__class__)
             36 PRECALL_BUILTIN_FAST_WITH_KEYWORDS     1
             40 CALL_ADAPTIVE            1
             50 POP_TOP
             52 LOAD_CONST               0 (None)
             54 RETURN_VALUE

@penguin-wwy

penguin-wwy commented Apr 2, 2022 •
edited
Loading

Copy link
Copy Markdown
Contributor Author

Looks promising. Will need some tests for the new output format.

Already add some test cases for quicken code in the dis.

@penguin-wwy penguin-wwy requested a review from markshannon April 2, 2022 07:48
markshannon
markshannon reviewed Apr 5, 2022
View reviewed changes

@markshannon markshannon left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Could you remove the changes to _testinternalcapi, so we don't need to export the _PyCode_Quicken function.

Comment thread Lib/test/test_dis.py Outdated
@brandtbucher brandtbucher mentioned this pull request Apr 10, 2022
Closed
@penguin-wwy penguin-wwy changed the title bpo-46841: Disassembly of quickened code gh-90997: bpo-46841: Disassembly of quickened code Apr 14, 2022
@penguin-wwy

penguin-wwy commented Apr 18, 2022

Copy link
Copy Markdown
Contributor Author

I have made the requested changes; please review again :) @markshannon @brandtbucher

@bedevere-bot

bedevere-bot commented Apr 18, 2022

Copy link
Copy Markdown

Thanks for making the requested changes!

: please review the changes made to this pull request.

markshannon
markshannon reviewed Apr 18, 2022
View reviewed changes
Comment thread Lib/test/test_dis.py Outdated
@markshannon

markshannon commented Apr 18, 2022

Copy link
Copy Markdown
Member

On small typo, otherwise looks good.

@penguin-wwy

penguin-wwy commented Apr 19, 2022

Copy link
Copy Markdown
Contributor Author

Thanks. :)
I have made the requested changes; please review again

@bedevere-bot

bedevere-bot commented Apr 19, 2022

Copy link
Copy Markdown

Thanks for making the requested changes!

: please review the changes made to this pull request.

@markshannon

markshannon commented Apr 19, 2022

Copy link
Copy Markdown
Member

Thanks @penguin-wwy for doing this.

@markshannon markshannon merged commit e590379 into python:main Apr 19, 2022
@JelleZijlstra JelleZijlstra mentioned this pull request Oct 5, 2022
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@markshannon markshannon markshannon left review comments

@brandtbucher brandtbucher Awaiting requested review from brandtbucher

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@penguin-wwy @brandtbucher @markshannon @bedevere-bot @the-knights-who-say-ni