bpo-47120: Replace the JUMP_ABSOLUTE opcode by the relative JUMP_BACKWARD

[iritkatriel]

https://bugs.python.org/issue47120

[iritkatriel]

Stats before:

15,128 files
333,926 code objects
5,336,282 lines
44,619,617 opcodes
2,169,508 total size of co_consts
270,630 number of co_consts
447,685 number of absolute jumps
136,753 number of relative jumps
116,057 number of absolute jumps with extended args
4,781 number of relative jumps with extended args
74,772 number of absolute jumps backwards
10,897 number of absolute jumps backwards with extended args
440,798 number of absolute jumps with delta < 256

After:

15,128 files
333,926 code objects
5,336,282 lines
44,613,954 opcodes
2,169,508 total size of co_consts
270,630 number of co_consts
383,386 number of absolute jumps
201,052 number of relative jumps
106,288 number of absolute jumps with extended args
8,887 number of relative jumps with extended args
10,473 number of absolute jumps backwards
1,128 number of absolute jumps backwards with extended args
380,589 number of absolute jumps with delta < 256

[markshannon]

If I understand the numbers correctly:

• currently about 20% of jumps need an EXTENDED_ARG prefix
• once all jumps are converted, only about 5% of jumps will need an EXTENDED_ARG prefix

Is that correct?

[iritkatriel]

My reading is that currently there are 584K jumps, of which 114K (~20%) have extended args. Once we convert all absolute jumps, the 380K which have small delta will not have extended args, so there will be about 3K that do. Plus the 9K of relative jumps with extended args gives 12K which is about 2%.

Does that sound right?

[iritkatriel]

Ah, I'm looking at the stats post this PR. You were probably looking at the pre-PR set.

[markshannon]

I agree with your figure of 12k jumps that will need extended arg, both before and after stats give that number.
I don't know where I got 5% from though 🤷‍♂️ 12/k/584k == 2% not 5%.

I was quite happy with 5%, 2% is excellent.

[iritkatriel]

@markshannon This is now merged with main and the tests ran. Did you want to review it again or shall I merge?

[markshannon]

I see that you've dropped the quickening on backward edges. Was this deliberate?

It should be OK, provided we switch to early quickening discussed in faster-cpython/ideas#338 (comment). We will need to track this to make sure we don't lose the ability to quicken code in loops.

Other than that, LGTM.

[iritkatriel]

I see that you've dropped the quickening on backward edges. Was this deliberate?

Let me put that back, I don't know why I removed it.

Is it ok that other opcodes have PREDICT(JUMP_ABSOLUTE) rather than PREDICT(JUMP_ABSOLUTE_QUICK)? Should I change it to PREDICT(JUMP_BACKWARD_QUICK) rather than PREDICT(JUMP_BACKWARD)?

[markshannon]

It should be OK to remove the quickening, provided that we implement faster-cpython/ideas#338 (comment). In fact it might be better to do so, as it avoids have to quicken all the other back jumps.

@brandtbucher would it make it easier to remove the quickening of back jumps now?

[brandtbucher]

Let's leave the backedge quickening on, since we figured out the issue and we're still not 100% settled on the approach to take for faster-cpython/ideas#338.

(This also seems more relevant to a "quicken everything" decision than the "quicken in-place" one, though there is some overlap between the two.)

[brandtbucher]

We might also want to consider adding a PREDICT(JUMP_BACKWARD_QUICK) before / instead of each PREDICT(JUMP_BACKWARD).

[iritkatriel]

@markshannon @brandtbucher I'll let you decide in which order to merge PRs impacting compile.c/ceval.c because you have other PRs on the go. I'll merge as necessary.

[markshannon]

This is the oldest, so I'm merging this first.
Most of the other changes are to the interpreter, so the conflicts should be small.