gh-115999: Add free-threaded specialization for STORE_SUBSCR#127169
Uh oh!
There was an error while loading. Please reload this page.
Conversation
The specialization only depends on the type, so no special thread-safety considerations there. STORE_SUBSCR_LIST_INT needs to lock the list before modifying it. `_PyDict_SetItem_Take2` already internally locks the dictionary using a critical section.
Python/ceval_macros.h Outdated
| // avoid any potentially escaping calls (like PyStackRef_CLOSE) while the | ||
| // object is locked. | ||
| #ifdefPy_GIL_DISABLED | ||
| # defineLOCK_OBJECT(op) PyMutex_LockFast(&(_PyObject_CAST(op))->ob_mutex) |
There was a problem hiding this comment.
@mpage, I think this locking pattern will be slightly faster than using critical sections inline in bytecodes.c. The downside is that we will deopt if there's any lock contention, but I think that's an okay tradeoff.
If we decide to go with this pattern, we can update UNPACK_SEQUENCE_LIST to use it.
colesbury commented Nov 22, 2024
@corona10 - I put my name down next to |
colesbury commented Nov 22, 2024
( |
mpage left a comment
mpage left a comment There was a problem hiding this comment.
Nice! I left one comment/question inline and there's a compiler warning that we need to fix up.
Python/ceval_macros.h Outdated
| // avoid any potentially escaping calls (like PyStackRef_CLOSE) while the | ||
| // object is locked. | ||
| #ifdefPy_GIL_DISABLED | ||
| # defineLOCK_OBJECT(op) PyMutex_LockFast(&(_PyObject_CAST(op))->ob_mutex) |
Uh oh!
There was an error while loading. Please reload this page.
corona10 commented Nov 22, 2024
Race condition with the wrong lock :) Fine, thank you for the work, I've updated the list of issues. |
corona10 left a comment
corona10 left a comment There was a problem hiding this comment.
Please add test codes at test_opcache.py
cpython/Lib/test/test_opcache.py
Line 1227 in d24a22e
Uh oh!
There was an error while loading. Please reload this page.
Co-authored-by: Donghee Na <donghee.na92@gmail.com>
colesbury commented Nov 25, 2024
Performance difference is marginal: 1.00x faster (but 99.99% likely to be faster) I think it's worth doing even if the perf is neutral just to minimize divergence with the default (GIL) build. |
pythongh-127518: Fix pystats build after python#127619
…ython#127169) The specialization only depends on the type, so no special thread-safety considerations there. STORE_SUBSCR_LIST_INT needs to lock the list before modifying it. `_PyDict_SetItem_Take2` already internally locks the dictionary using a critical section.
pythongh-127518: Fix pystats build after python#127619
3 participants
The specialization only depends on the type, so no special thread-safety considerations there.
STORE_SUBSCR_LIST_INT needs to lock the list before modifying it.
_PyDict_SetItem_Take2already internally locks the dictionary using a critical section.--disable-gilbuilds #115999