faster-cpython / ideas

1.68k stars 48 forks source link

Remove `RETURN_CONST` to allow static specialization of `RETURN` and `LOAD_CONST` bytecodes #577

Open markshannon opened 1 year ago

markshannon commented 1 year ago

With immortal objects and the desire to shrink code objects, we want to add LOAD_COMMON_CONST for loading None, True, etc.

The RETURN bytecodes do largely different things for generators and normal frames. It would make sense to split this into "normal" and "generator" returns.

The existence of RETURN_CONST prevents this, as we would need 4 versions of it to allow specialization of return for generator/normal frames and for normal/"common" const.

gvanrossum commented 7 months ago

So I think what you are saying is that in Tier 1:

(1) For return in a generator (or coroutine) generate a different opcode, e.g. RETURN_VALUE_FROM_GENERATOR, vs. RETURN_VALUE for return in regular functions (as today).

(2) We'd have two variants for LOAD_CONST: regular LOAD_CONST and LOAD_COMMON_CONST.

(3) RETURN_CONST, which currently is generated for LOAD_CONST + RETURN_VALUE, would have to be replaced by four separate opcodes, e.g. RETURN_CONST, RETURN_COMMON_CONST, RETURN_CONST_FROM_GENERATOR, and RETURN_COMMON_CONST_FROM_GENERATOR.

(4) In addition we'd need instrumented versions (?).

Of course we could choose to only generate the "super" instruction RETURN_CONST for the combination LOAD_CONST + RETURN_VALUE. But then it would be less useful.

And in Tier 2 there would be no difference -- RETURN_CONST is just LOAD_CONST + _POP_FRAME.

So yeah, once we're ready to do this (or part of this, e.g. the LOAD_COMMON_CONST part), we can just get rid of RETURN_CONST.

@markshannon Do you feel that LOAD_COMMON_CONST is worth doing for 3.13? Or is this a 3.14 idea you don't want to forget? Presumably it would become _LOAD_CONST_INLINE_BORROW in Tier 2. The most common case, for loading None, might not actually reduce most code objects' sizes, because functions without docstrings have a None as constant 0 anyway.

Separately, I'm not sure what benefit we'd get from having RETURN_VALUE_FROM_GENERATOR. I don't see anything conditional on generators in _POP_FRAME. But I assume I'm missing something. Could you give me a hint?

markshannon commented 7 months ago

Do you feel that LOAD_COMMON_CONST is worth doing for 3.13?

It might be, but the real benefit of removing RETURN_CONST is that we can split RETURN_VALUE into a generator/coroutine form and a normal function form.

because functions without docstrings have a None as constant 0 anyway.

Is that part of the language spec, or can we change that?

I'm not sure what benefit we'd get from having RETURN_VALUE_FROM_GENERATOR

The code paths for generators and normal functions are quite different. Most of the work in RETURN_VALUE is done by _PyEval_FrameClearAndPop which then calls either clear_thread_frame or clear_gen_frame. Splitting RETURN_VLAUE into two would make for simpler code paths. RETURN_VALUE_FROM_GENERATOR also needs to use return_offset, whereas RETURN_VALUE does not.

gvanrossum commented 7 months ago

Is that part of the language spec, or can we change that?

It's not, but changing it might require some refactoring elsewhere (basically, code objects don't have a field to store the docstring). We played with this in the 3.11 era but never dared do it.

The rest is clear, thank you.