Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

gh-99254: remove all unused consts from code objects#99255

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Merged
iritkatriel merged 19 commits intopython:mainfromiritkatriel:remove_unused_consts
Nov 11, 2022

Conversation

iritkatriel
Copy link
Member

@iritkatrieliritkatriel commentedNov 8, 2022•
edited
Loading

This reduces the total size of unused consts in the top 100 PyPl packages by about 2%:

Before:

Total: 75 errors; 9,946 files; 235,684 code objects; 3,669,436 lines; 31,309,347 opcodes; 31,073,663 opcode pairs; 12,916,440.0 cache_size; 9,198,802.0 cache wasted; 1,858,819 ops quickened; 44,504 prev extended args;1,509,350 total size of co_consts; 189,300 number of co_consts

After:

Total: 75 errors; 9,946 files; 235,684 code objects; 3,669,436 lines; 31,307,877 opcodes; 31,072,193 opcode pairs; 12,915,869.0 cache_size; 9,198,231.0 cache wasted; 1,858,819 ops quickened; 43,034 prev extended args;1,477,889 total size of co_consts; 189,286 number of co_consts

sweeneyde reacted with thumbs up emoji
@iritkatrieliritkatriel added the 🔨 test-with-buildbotsTest PR w/ buildbots; report in status section labelNov 8, 2022
@bedevere-bot
Copy link

🤖 New build scheduled with the buildbot fleet by@iritkatriel for commita9f38fd 🤖

If you want to schedule another build, you need to add the ":hammer: test-with-buildbots" label again.

@bedevere-botbedevere-bot removed the 🔨 test-with-buildbotsTest PR w/ buildbots; report in status section labelNov 8, 2022
iritkatrieland others added2 commitsNovember 9, 2022 18:45
Co-authored-by: Kumar Aditya <59607654+kumaraditya303@users.noreply.github.com>
Co-authored-by: Kumar Aditya <59607654+kumaraditya303@users.noreply.github.com>
Copy link
Member

@sweeneydesweeneyde left a comment

Choose a reason for hiding this comment

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

The code looks good to me. A couple of little things, but they could go either way.

@sweeneyde
Copy link
Member

sweeneyde commentedNov 10, 2022•
edited
Loading

Unfortunately, I can reproduce the Windows failures locally.

I get a segfault whenPy_DECREF(0x00000000ffffffff) happens in somePOP_TOP instruction. Running.\python.bat -m test withlltrace=1 and commenting outdump_stack() gave output that ended with the following:

Resuming frame for 'Enum.__new__' in module 'enum'0: RESUME 02: LOAD_GLOBAL_BUILTIN 114: LOAD_FAST 116: CALL_NO_KW_TYPE_1 126: LOAD_FAST 028: IS_OP 030: POP_JUMP_IF_FALSE 236: NOP38: LOAD_FAST 040: LOAD_ATTR 240: LOAD_ATTR 260: LOAD_FAST 162: BINARY_SUBSCR_DICT72: RETURN_VALUEResuming frame for 'EnumType.__call__' in module 'enum'42: RETURN_VALUEResuming frame.2: INTERPRETER_EXIT246: RETURN_VALUEResuming frame.2: INTERPRETER_EXIT522: POP_JUMP_IF_FALSE 2528: LOAD_GLOBAL_BUILTIN 39540: LOAD_GLOBAL_MODULE 12552: CALL_NO_KW_LEN 1562: LOAD_GLOBAL_MODULE 40574: COMPARE_OP_INT_JUMP 5714: LOAD_FAST 3716: LOAD_GLOBAL_MODULE 12728: LOAD_FAST 2730: STORE_SUBSCR_DICT734: LOAD_GLOBAL_BUILTIN 39746: LOAD_GLOBAL_MODULE 6758: CALL_NO_KW_LEN 1768: LOAD_GLOBAL_MODULE 50780: COMPARE_OP_INT_JUMP 5920: LOAD_FAST 3922: LOAD_GLOBAL_MODULE 6934: LOAD_FAST 2936: STORE_SUBSCR_DICT940: LOAD_FAST 3942: RETURN_VALUEResuming frame for 'compile' in module 're'28: RETURN_VALUEResuming frame for '<module>'262: STORE_NAME 37264: EXTENDED_ARG 5266: LOAD_CONST 174268: LOAD_CONST 24270: MAKE_FUNCTION 1272: STORE_NAME 38274: EXTENDED_ARG 5276: LOAD_CONST 174278: LOAD_CONST 25280: MAKE_FUNCTION 1282: STORE_NAME 39284: EXTENDED_ARG 5286: LOAD_CONST 174288: LOAD_CONST 26290: MAKE_FUNCTION 1292: STORE_NAME 40294: EXTENDED_ARG 5296: LOAD_CONST 175298: LOAD_CONST 27300: MAKE_FUNCTION 1302: STORE_NAME 41304: LOAD_CONST 28306: MAKE_FUNCTION 0308: STORE_NAME 7310: LOAD_CONST 29312: MAKE_FUNCTION 0314: STORE_NAME 42316: EXTENDED_ARG 5318: LOAD_CONST 174320: LOAD_CONST 30322: MAKE_FUNCTION 1324: STORE_NAME 43326: LOAD_NAME 44328: BUILD_TUPLE 1330: LOAD_CONST 31332: MAKE_FUNCTION 1334: STORE_NAME 45336: LOAD_CONST 32338: MAKE_FUNCTION 0340: STORE_NAME 46342: LOAD_CONST 33344: MAKE_FUNCTION 0346: STORE_NAME 47348: LOAD_NAME 26350: STORE_NAME 48352: LOAD_CONST 34354: MAKE_FUNCTION 0356: STORE_NAME 49358: LOAD_CONST 35360: MAKE_FUNCTION 0362: STORE_NAME 50364: LOAD_CONST 36366: MAKE_FUNCTION 0368: STORE_NAME 51370: LOAD_CONST 37372: MAKE_FUNCTION 0374: STORE_NAME 52376: LOAD_CONST 38378: MAKE_FUNCTION 0380: STORE_NAME 53382: EXTENDED_ARG 5384: LOAD_CONST 176386: LOAD_CONST 39388: MAKE_FUNCTION 1390: STORE_NAME 54392: LOAD_NAME 18394: BUILD_TUPLE 1396: LOAD_CONST 40398: MAKE_FUNCTION 1400: STORE_NAME 55402: EXTENDED_ARG 5404: LOAD_CONST 172406: LOAD_CONST 41408: MAKE_FUNCTION 1410: STORE_NAME 26412: LOAD_NAME 16414: BUILD_TUPLE 1416: LOAD_CONST 42418: MAKE_FUNCTION 1420: STORE_NAME 56422: NOP424: LOAD_CONST 1426: LOAD_CONST 43428: IMPORT_NAME 13430: IMPORT_FROM 57432: STORE_NAME 57434: POP_TOP436: NOP438: LOAD_NAME 588846: POP_JUMP_IF_FALSE 98848: POP_TOP

Maybe theremove_unused_consts call needs to be moved earlier, to the/** Optimization **/ section, so thatEXTENDED_ARGs won't have been added yet.

@iritkatriel
Copy link
MemberAuthor

Maybe theremove_unused_consts call needs to be moved earlier, to the/** Optimization **/ section, so thatEXTENDED_ARGs won't have been added yet.

I think it is before - extended args are added in assemble_emit.

@brandtbucher
Copy link
Member

brandtbucher commentedNov 10, 2022•
edited
Loading

I think it is before - extended args are added in assemble_emit.

But jump distances are computed inassemble_jump_offsets, which happens before this. If anyEXTENDED_ARGs are removed fromLOAD_CONST instructions, anything that jumps over them will be wrong.

@bedevere-bot
Copy link

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

@iritkatriel
Copy link
MemberAuthor

I think it is before - extended args are added in assemble_emit.

But jump distances are computed inassemble_jump_offsets, which happens before this. If anyEXTENDED_ARGs are removed fromLOAD_CONST instructions, anything that jumps over them will be wrong.

Ah right, there's a comment about this in the code.

@iritkatriel
Copy link
MemberAuthor

Ok, that fixed it. I wonder why it only showed up on windows.

@brandtbucher
Copy link
Member

I wonder why it only showed up on windows.

I bet the startup sequence contains quite a bit of Windows-specific code. Probably just a really lucky/unlucky code path.

@iritkatriel
Copy link
MemberAuthor

I have made the requested changes; please review again.

@bedevere-bot
Copy link

Thanks for making the requested changes!

@sweeneyde,@brandtbucher: please review the changes made to this pull request.

@iritkatrieliritkatriel added the 🔨 test-with-buildbotsTest PR w/ buildbots; report in status section labelNov 11, 2022
@bedevere-bot
Copy link

🤖 New build scheduled with the buildbot fleet by@iritkatriel for commit02b68e0 🤖

If you want to schedule another build, you need to add the ":hammer: test-with-buildbots" label again.

@bedevere-botbedevere-bot removed the 🔨 test-with-buildbotsTest PR w/ buildbots; report in status section labelNov 11, 2022
@iritkatriel
Copy link
MemberAuthor

Thanks for the reviews and debugging help.

ethanfurman pushed a commit to ethanfurman/cpython that referenced this pull requestNov 12, 2022
@iritkatrieliritkatriel deleted the remove_unused_consts branchDecember 7, 2022 15:49
Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment
Reviewers

@kumaraditya303kumaraditya303kumaraditya303 left review comments

@thatbirdguythatuknownotthatbirdguythatuknownotthatbirdguythatuknownot left review comments

@markshannonmarkshannonAwaiting requested review from markshannonmarkshannon is a code owner

@brandtbucherbrandtbucherAwaiting requested review from brandtbucher

@sweeneydesweeneydeAwaiting requested review from sweeneyde

Assignees
No one assigned
Labels
interpreter-core(Objects, Python, Grammar, and Parser dirs)
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

6 participants
@iritkatriel@bedevere-bot@sweeneyde@brandtbucher@kumaraditya303@thatbirdguythatuknownot
[8]ページ先頭
©2009-2025 Movatter.jp