@tomasr8 @brandtbucher 7.5% speedup in nbody on the JIT in this PR 👀. I removed some of the optimizations in this PR to make things easier to review, and also to split up work later!

Whether we can skip the refcount operation is a property of the reference, not the object.
I'm a bit puzzled how this works, as is.

You'll need to remove the skip_refcount field from JitOptSymbol and change the type of the reference from JitOptSymbol * to JitOptSymbolRef and put the skip_refcount field there. Much like we did for PyStackRef.

Whether we can skip the refcount operation is a property of the reference, not the object. I'm a bit puzzled how this works, as is.

The optimizer in this PR uses LOAD_FAST_BORROW and LOAD_CONST for a safe indicator that says "something else holds a strong reference to this reference" (in this case, localsplus for LOAD_FAST_BORROW, and co_consts for LOAD_CONST) AND "this reference will be consumed immediately by something, so it's safe to borrow it".

Consider the following operation:

LOAD_FAST_BORROW x
LOAD_FAST_BORROW y
BINARY_OP_ADD_FLOAT

Then it's trivial to see that x and y have strong references elsewhere. Thus they can just become

LOAD_FAST_BORROW x
LOAD_FAST_BORROW y
BINARY_OP_ADD_FLOAT__NO_DECREF_INPUT

Basically, I'm leveraging on the work that Matt did to do cheap and safe reference analysis.

I think it should be possible to just do this for all LOAD_FAST, let me try that.

I think it should be possible to just do this for all LOAD_FAST, let me try that.

Yeah this is not safe. If we assign it back to a locals, it needs to be an owned reference, not borrowed.

You'll need to remove the skip_refcount field from JitOptSymbol and change the type of the reference from JitOptSymbol * to JitOptSymbolRef and put the skip_refcount field there. Much like we did for PyStackRef.

I'm internally deliberating on this one. It's a lot of churn; a few thousand lines of diff. I don't think we need to. We used the tagged pointer representation over PyObject because PyObject has proper lifetimes, reference management, and also to save space. Meanwhile JitOptSymbol has no lifetime management at all (it's effectively immortal for the context of the optimizer), needs no reference management, and if we want to save space, we can just reuse the memory in the thread state. We basically get none of the associated benefits of the _PyStackRef API (sound lifetime semantics), for all the associated negatives like the code churn.

If this needs better naming, I'm all for it. Eventually, I want to expand this so it's more powerful than the stackref API -- skipping reference counts when we know it's truly safe to skip, not just like stackrefs where we skip only because it's on the stack. For example, take the following code:

x = (a, b, c, d)
// Do stuff with a, b, c, d

A smart optimizer would be able to skip all refcounting operations on a,b,c,d as long as x is kept live. As we know x holds an owned reference to a b c d. This is why skipping refcounting is a property of the symbol, not a property of the stackref, and it should be kept as part of the symbol.

I've renamed it to clarify that the main property is observing the strong references are held by someone else, not whether we can skip refcounts or not.

I've renamed it to clarify that the main property is observing the strong references are held by someone else, not whether we can skip refcounts or not.

On second thought, this name is even worse. It's confusing and error-prone.

I'll repeat what I said before "Whether we can skip the refcount operation is a property of the reference, not the object."

def leaks():
    v = 1.0
    while True:
        v = (v,)[0] * v

Will keep leaking as (v,)[0] converts the reference to v to a strong reference which is then leaked if the BINARY_OP is converted to the no-decref version.

Will keep leaking as (v,)[0] converts the reference to v to a strong reference which is then leaked if the BINARY_OP is converted to the no-decref version.

To be pedantic, that won't leak as we don't trace from function entry at the moment. So the optimizer will never see that as a constant symbol. But yes I get your point, let me just go through the churn then.

@markshannon I have done the refactor like you requested.