gh-115999: Enable free-threaded specialization of LOAD_CONST - #129365
Conversation
Sorry, something went wrong.
|
I'm not sure why LOAD_CONST was already checked off on the issue page. The test fails for free-threaded builds without the accompanying bytecode change. |
Sorry, something went wrong.
I think specializing LOAD_CONST during bytecode execution was a recent change: #128708. Previously, it happened when the bytecode was quickened during creation of the code object. |
Sorry, something went wrong.
|
Looks like about 1% gain from this, but given noise levels it could be a wash. Still, considering way more things are currently immortal in the free-threaded build it's probably a bigger win than in the regular build :D |
Sorry, something went wrong.
| uint8_t expected = LOAD_CONST; | ||
| _Py_atomic_compare_exchange_uint8( | ||
| &this_instr->op.code, &expected, | ||
| _Py_IsImmortal(obj) ? LOAD_CONST_IMMORTAL : LOAD_CONST_MORTAL); |
There was a problem hiding this comment.
If you want a bigger win, perhaps you could allow deferred references as well. They behave the same as immortal objects if they're live on the stack.
Considering many LOAD_CONST are deferred objects, that should be a nice win in perf.
That can come in another PR though.
Sorry, something went wrong.
There was a problem hiding this comment.
Yeah, that's probably a good idea. Running some tests on the testsuite, about 75% of all LOAD_CONST invocations involved immortal objects, 25% were deferred, and the remainder is less than 1% (in the free-threaded build. Not nearly as many immortal objects in a regular build). I suspect we'll get fewer immortal objects over time, and most of them will probably be deferred instead, I guess?
Now the question is: do I add PyStackRef_FromPyObjectDeferred(), or do I change PyStackRef_FromPyObjectImmortal() to accept deferred objects as well? :)
Sorry, something went wrong.
mpage
left a comment
There was a problem hiding this comment.
LGTM!
Sorry, something went wrong.
Enable free-threaded specialization of LOAD_CONST, which is an unconditional specialization without stats bookkeeping, so it's a simple opcode replace. (It could even be a relaxed store if we didn't have to worry about instrumentation.)