gh-84436: Add static flag in PyLongObject's lv_tag

[eduardo-elizondo]

This adds a new SIGN_STATIC flag to PyLongObject's lv_tag which is used to determine if a given long object was statically allocated. It also reworks all the functions that use NON_SIZE_BITS to correctly account for this new flag.

• Issue: Fixing Copy on Writes from reference counting and immortal objects #84436

[eduardo-elizondo]

@markshannon here's a couple of changes to the recently introduced lv_flag so that we can use it in: #19474

cc @ericsnowcurrently

[markshannon]

Why do we need a static bit separate from the immortal bit?

[markshannon]

Also, all the checks for sign and compactness need to be as close to single machine instructions as possible.
Adding the extra masking operations is something I would rather avoid for integer operations.

[eduardo-elizondo]

Why do we need a static bit separate from the immortal bit?

The specific place the static check is needed is here: https://github.com/python/cpython/pull/19474/files#r1156466281. It's to be compliant with the added condition of PEP683 to be able to reset the reference count if an immortal object's refcount reaches zero.

For types like None, Bool, this is easy, we just add a new tp_dealloc that resets the value. For interned strings, we check the intern state. For small ints we can either check if it's a small int or use a flag somewhere in the object, like this one.

In my opinion, it's much better to just do the 'expensive' small int check in tp_dealloc of the longobject but if we are already making a bit here available for "immortal longs" we might as well use it. But, I'm open to either option.

Adding the extra masking operations is something I would rather avoid for integer operations.

Assuming that we do want to have the static check, I think we might be able to make this cheaper if we make the immortal bit to be the least significant bit and the sign mask be the second and third least significant bits. I can draft up a change like this.

However, before I do that, I'd like to get some thoughts in perhaps dropping the immortal bit in long's lv_tag and just have the more expensive small int check in tp_dealloc. If that's the case then we can just close this PR (or change NON_SIZE_BITS to 2 and make it clear that we only need the sign mask at the moment).

[markshannon]

I think you misunderstood my (poorly worded) question.

Why do we need a static flag in addition to an immortal flag?
Immortality protects an object from reclamation, so provides the safety guarantees we need.

I worry that having two bits to check will degrade performance too much.

[markshannon]

@eduardo-elizondo?

[eduardo-elizondo]

@markshannon Somehow I missed this message, my bad.

Why do we need a static flag in addition to an immortal flag?

Short answer is: we don't need it! We just need one flag. Having two flags will make things cleaner in code, but harder in reasoning. I tried to explain that above but probably didn't come across correctly. Let me explain a bit more.

The original reason I created this change was to address one of Guido's comment in the Immortal Objects PR. The comment was made on the newly added LongObject's tp_dealloc function. For context, the tp_dealloc was added so that we could restore long objects that would be decref'd to zero due to extension incompatibility. Given that we can't rely on the reference count, the way that we can check if this value has to be restored is by checking if the object is statically allocated which we assume by checking if it's a small int. This leads a rather long check:

static void long_dealloc(PyObject *self) {
    PyLongObject *pylong = (PyLongObject*)self;
    if (pylong && _PyLong_IsCompact(pylong)) {
        stwodigits ival = medium_value(pylong);
        if (IS_SMALL_INT(ival)) {
            PyLongObject *small_pylong = (PyLongObject *)get_small_int((sdigit)ival);
            if (pylong == small_pylong) {
                _Py_SetImmortal(self);
                return;
            }
        }
    }
    Py_TYPE(self)->tp_free(self);
}        

To this, Guido asked:

We now have room for an "immortal" bit in long int objects, since gh-102464. Should use that bit rather than checking the value. @markshannon for more info.

Thus, I created this PR to be able to use this immortal bit. All this PR does is that it exposes: _PyLong_IsStatic which we can then use in long_dealloc to do and make things cleaner:

static void long_dealloc(PyObject *self) {
    PyLongObject *pylong = (PyLongObject*)self;
    if (_PyLong_IsStatic(pylong)) {
      _Py_SetImmortal(self);
      return;
    }
    Py_TYPE(self)->tp_free(self);
}

Going back to your original question:

Why do we need a static flag in addition to an immortal flag?

If we have the static flag we can make things a lot cleaner but it's not needed, we can keep the longer checks and we can still guarantee correctness.

[eduardo-elizondo]

@markshannon Given all of the above, I instead propose to completely drop the "immortality bit" in the long value tag bits since we don't need this signal.

[encukou]

It looks like this PR can be closed, is that right?

[eduardo-elizondo]

@encukou yeah, should be safe to close