gh-134584: Eliminate redundant refcounting from _CALL_TYPE_1

[tomasr8]

Depends on #135761

• Issue: Eliminate redundant refcounting in the JIT #134584

[Fidget-Spinner]

Looks mostly good, just 2 comments that also explain the CI failure.

[tomasr8]

Thanks for the review! I'll continue working on it tomorrow (Monday) 🙂

[Fidget-Spinner]

I just merged #135761 in. Sorry but you'll have to merge the changes in, as I did not ultimately add _SWAP_FAST.

[Fidget-Spinner]

Yay, thank you!

[Fidget-Spinner]

I will do some microbenchmarking to see if this does anything. Give me a sec.

[Fidget-Spinner]

On this script:

import pyperf

def func():
    x = 1
    for _ in range(120000000):
        type(x)

if __name__ == "__main__":
    runner = pyperf.Runner()
    runner.bench_func('micro', func)

Mean +- std dev: [no-opt] 918 ms +- 17 ms -> [opt] 897 ms +- 21 ms: 1.02x faster

So it's slightly faster. However, note that this optimization won't fully pay off till we get #135465 merged. So let's wait for now till then. Thanks!

Basically right now there's the extra cost of reading and writing to the stack due to the extra _POP_TOP. We're eliminating 1 reference count check in return. This seems to be barely worth it, but I crunched the numbers before and once the stack becomes registers, it becomes very worth it.

If you're interested, I benchmarked my own branch with this (higher is better):

CPython base JIT:
Number of long_calcul per second: 10.23 ± 0.13

CPython TOS caching:
Number of long_calcul per second: 9.60 ± 0.37

CPython decref binary op int elimination:
Number of long_calcul per second: 10.14 ± 0.07

CPython TOS + decref binary op int elimination:
Number of long_calcul per second: 11.45 ± 0.10

So we should expect to see an around ~10% speedup once TOS caching is merged, and we start doing this!

[Fidget-Spinner]

@tomasr8 congrats!!! I decreased the loop count, rebased your PR on top of Mark's TOS caching PR, and did a micro bench with the script above.

TOS Caching vs TOS Caching + This PR
Mean +- std dev: [no-opt] 210 ms +- 3 ms -> [opt] 195 ms +- 1 ms: 1.08x faster

So 8% improvement if we have TOS caching enabled on type operations that load a local variable!

[tomasr8]

Awesome stuff! And a pretty nice speedup for a relatively simple optimization too!
I can try and have a look at Mark's PR so that we can get it merged sooner :)

[brandtbucher]

This shouldn't be here, right? Because we're leaving arg on the stack in a?

[tomasr8]

Veery late response 😅 but I changed and renamed this op to _SHUFFLE_2_LOAD_CONST_INLINE_BORROW since it's analogous to the recently added _SHUFFLE_3_LOAD_CONST_INLINE_BORROW. This also resolves the issue with closing the ref! :)

[tomasr8]

I think it should be good now! Though I haven't yet caught up on the latest changes in the JIT so another pair of eyes would be nice :)

[Fidget-Spinner]

Just one change, otherwise LGTM!

[Fidget-Spinner]

Windows failure is unrelated. Probably due to the other known bug in executor management. I'm merging this, and will be responsible for any breakages/reverts. Thank Tomas, and happy holidays!

[tomasr8]

Thanks for the review, Ken! Happy holidays to you as well! :)