gh-99254: remove all unused consts from code objects

[iritkatriel]

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

• Issue: remove all unused consts in from code objects #99254

[bedevere-bot]

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

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

[sweeneyde]

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

[sweeneyde]

Unfortunately, I can reproduce the Windows failures locally.

I get a segfault when Py_DECREF(0x00000000ffffffff) happens in some POP_TOP instruction. Running .\python.bat -m test with lltrace=1 and commenting out dump_stack() gave output that ended with the following:

Resuming frame for 'Enum.__new__' in module 'enum'
0: RESUME 0
2: LOAD_GLOBAL_BUILTIN 1
14: LOAD_FAST 1
16: CALL_NO_KW_TYPE_1 1
26: LOAD_FAST 0
28: IS_OP 0
30: POP_JUMP_IF_FALSE 2
36: NOP
38: LOAD_FAST 0
40: LOAD_ATTR 2
40: LOAD_ATTR 2
60: LOAD_FAST 1
62: BINARY_SUBSCR_DICT
72: RETURN_VALUE

Resuming frame for 'EnumType.__call__' in module 'enum'
42: RETURN_VALUE

Resuming frame.2: INTERPRETER_EXIT
246: RETURN_VALUE

Resuming frame.2: INTERPRETER_EXIT
522: POP_JUMP_IF_FALSE 2
528: LOAD_GLOBAL_BUILTIN 39
540: LOAD_GLOBAL_MODULE 12
552: CALL_NO_KW_LEN 1
562: LOAD_GLOBAL_MODULE 40
574: COMPARE_OP_INT_JUMP 5
714: LOAD_FAST 3
716: LOAD_GLOBAL_MODULE 12
728: LOAD_FAST 2
730: STORE_SUBSCR_DICT
734: LOAD_GLOBAL_BUILTIN 39
746: LOAD_GLOBAL_MODULE 6
758: CALL_NO_KW_LEN 1
768: LOAD_GLOBAL_MODULE 50
780: COMPARE_OP_INT_JUMP 5
920: LOAD_FAST 3
922: LOAD_GLOBAL_MODULE 6
934: LOAD_FAST 2
936: STORE_SUBSCR_DICT
940: LOAD_FAST 3
942: RETURN_VALUE

Resuming frame for 'compile' in module 're'
28: RETURN_VALUE

Resuming frame for '<module>'
262: STORE_NAME 37
264: EXTENDED_ARG 5
266: LOAD_CONST 174
268: LOAD_CONST 24
270: MAKE_FUNCTION 1
272: STORE_NAME 38
274: EXTENDED_ARG 5
276: LOAD_CONST 174
278: LOAD_CONST 25
280: MAKE_FUNCTION 1
282: STORE_NAME 39
284: EXTENDED_ARG 5
286: LOAD_CONST 174
288: LOAD_CONST 26
290: MAKE_FUNCTION 1
292: STORE_NAME 40
294: EXTENDED_ARG 5
296: LOAD_CONST 175
298: LOAD_CONST 27
300: MAKE_FUNCTION 1
302: STORE_NAME 41
304: LOAD_CONST 28
306: MAKE_FUNCTION 0
308: STORE_NAME 7
310: LOAD_CONST 29
312: MAKE_FUNCTION 0
314: STORE_NAME 42
316: EXTENDED_ARG 5
318: LOAD_CONST 174
320: LOAD_CONST 30
322: MAKE_FUNCTION 1
324: STORE_NAME 43
326: LOAD_NAME 44
328: BUILD_TUPLE 1
330: LOAD_CONST 31
332: MAKE_FUNCTION 1
334: STORE_NAME 45
336: LOAD_CONST 32
338: MAKE_FUNCTION 0
340: STORE_NAME 46
342: LOAD_CONST 33
344: MAKE_FUNCTION 0
346: STORE_NAME 47
348: LOAD_NAME 26
350: STORE_NAME 48
352: LOAD_CONST 34
354: MAKE_FUNCTION 0
356: STORE_NAME 49
358: LOAD_CONST 35
360: MAKE_FUNCTION 0
362: STORE_NAME 50
364: LOAD_CONST 36
366: MAKE_FUNCTION 0
368: STORE_NAME 51
370: LOAD_CONST 37
372: MAKE_FUNCTION 0
374: STORE_NAME 52
376: LOAD_CONST 38
378: MAKE_FUNCTION 0
380: STORE_NAME 53
382: EXTENDED_ARG 5
384: LOAD_CONST 176
386: LOAD_CONST 39
388: MAKE_FUNCTION 1
390: STORE_NAME 54
392: LOAD_NAME 18
394: BUILD_TUPLE 1
396: LOAD_CONST 40
398: MAKE_FUNCTION 1
400: STORE_NAME 55
402: EXTENDED_ARG 5
404: LOAD_CONST 172
406: LOAD_CONST 41
408: MAKE_FUNCTION 1
410: STORE_NAME 26
412: LOAD_NAME 16
414: BUILD_TUPLE 1
416: LOAD_CONST 42
418: MAKE_FUNCTION 1
420: STORE_NAME 56
422: NOP
424: LOAD_CONST 1
426: LOAD_CONST 43
428: IMPORT_NAME 13
430: IMPORT_FROM 57
432: STORE_NAME 57
434: POP_TOP
436: NOP
438: LOAD_NAME 58
8846: POP_JUMP_IF_FALSE 9
8848: POP_TOP

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

[iritkatriel]

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

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

[brandtbucher]

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

But jump distances are computed in assemble_jump_offsets, which happens before this. If any EXTENDED_ARGs are removed from LOAD_CONST instructions, anything that jumps over them will be wrong.

[bedevere-bot]

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

[iritkatriel]

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

But jump distances are computed in assemble_jump_offsets, which happens before this. If any EXTENDED_ARGs are removed from LOAD_CONST instructions, anything that jumps over them will be wrong.

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

[iritkatriel]

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

[brandtbucher]

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]

I have made the requested changes; please review again.

[bedevere-bot]

Thanks for making the requested changes!

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

[bedevere-bot]

🤖 New build scheduled with the buildbot fleet by @iritkatriel for commit 02b68e0 🤖

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

No need for re-review.

[iritkatriel]

Thanks for the reviews and debugging help.