From b834dc573e5fcefd0f567931026d55e6afa1a219 Mon Sep 17 00:00:00 2001 From: Donghee Na Date: Tue, 24 Sep 2024 12:14:46 -0700 Subject: [PATCH 01/16] gh-123358: Update LOAD_DEREF to use stackref and atomic incref --- Include/internal/pycore_cell.h | 26 +++++++++++++++++++++++ Include/internal/pycore_opcode_metadata.h | 2 +- Include/internal/pycore_uop_metadata.h | 2 +- Python/bytecodes.c | 5 ++--- Python/executor_cases.c.h | 5 ++--- Python/generated_cases.c.h | 5 ++--- 6 files changed, 34 insertions(+), 11 deletions(-) diff --git a/Include/internal/pycore_cell.h b/Include/internal/pycore_cell.h index 27f67d57b2fb79..ff9ec82d23f39e 100644 --- a/Include/internal/pycore_cell.h +++ b/Include/internal/pycore_cell.h @@ -2,6 +2,7 @@ #define Py_INTERNAL_CELL_H #include "pycore_critical_section.h" +#include "pycore_object.h" #ifdef __cplusplus extern "C" { @@ -42,6 +43,31 @@ PyCell_GetRef(PyCellObject *cell) return res; } +static inline void +_PyCell_GetStackRef(PyCellObject *cell, _PyStackRef *value_addr) +{ + PyObject *value; +#ifdef Py_GIL_DISABLED + value = _Py_atomic_load_ptr(&cell->ob_ref); + if (value != NULL) { + if (_Py_IsImmortal(value) || _PyObject_HasDeferredRefcount(value)) { + *value_addr = (_PyStackRef){ .bits = (uintptr_t)value | Py_TAG_DEFERRED }; + return; + } + if (_Py_TryIncrefCompare(&cell->ob_ref, value)) { + *value_addr = _PyStackRef_FromPyObjectSteal(value); + return; + } + } +#endif + value = PyCell_GetRef(cell); + if (value == NULL) { + *value_addr = PyStackRef_NULL; + return; + } + *value_addr = PyStackRef_FromPyObjectSteal(value); +} + #ifdef __cplusplus } #endif diff --git a/Include/internal/pycore_opcode_metadata.h b/Include/internal/pycore_opcode_metadata.h index 3344ede5e92c07..b5fede10758f2e 100644 --- a/Include/internal/pycore_opcode_metadata.h +++ b/Include/internal/pycore_opcode_metadata.h @@ -1154,7 +1154,7 @@ const struct opcode_metadata _PyOpcode_opcode_metadata[266] = { [LOAD_BUILD_CLASS] = { true, INSTR_FMT_IX, HAS_ERROR_FLAG | HAS_ESCAPES_FLAG }, [LOAD_COMMON_CONSTANT] = { true, INSTR_FMT_IB, HAS_ARG_FLAG }, [LOAD_CONST] = { true, INSTR_FMT_IB, HAS_ARG_FLAG | HAS_CONST_FLAG | HAS_PURE_FLAG }, - [LOAD_DEREF] = { true, INSTR_FMT_IB, HAS_ARG_FLAG | HAS_FREE_FLAG | HAS_ERROR_FLAG | HAS_ESCAPES_FLAG }, + [LOAD_DEREF] = { true, INSTR_FMT_IB, HAS_ARG_FLAG | HAS_LOCAL_FLAG | HAS_ERROR_FLAG | HAS_ESCAPES_FLAG }, [LOAD_FAST] = { true, INSTR_FMT_IB, HAS_ARG_FLAG | HAS_LOCAL_FLAG | HAS_PURE_FLAG }, [LOAD_FAST_AND_CLEAR] = { true, INSTR_FMT_IB, HAS_ARG_FLAG | HAS_LOCAL_FLAG }, [LOAD_FAST_CHECK] = { true, INSTR_FMT_IB, HAS_ARG_FLAG | HAS_LOCAL_FLAG | HAS_ERROR_FLAG | HAS_ESCAPES_FLAG }, diff --git a/Include/internal/pycore_uop_metadata.h b/Include/internal/pycore_uop_metadata.h index 07606135d7a356..6171af241e845d 100644 --- a/Include/internal/pycore_uop_metadata.h +++ b/Include/internal/pycore_uop_metadata.h @@ -123,7 +123,7 @@ const uint16_t _PyUop_Flags[MAX_UOP_ID+1] = { [_MAKE_CELL] = HAS_ARG_FLAG | HAS_FREE_FLAG | HAS_ERROR_FLAG | HAS_ERROR_NO_POP_FLAG, [_DELETE_DEREF] = HAS_ARG_FLAG | HAS_FREE_FLAG | HAS_ERROR_FLAG | HAS_ERROR_NO_POP_FLAG | HAS_ESCAPES_FLAG, [_LOAD_FROM_DICT_OR_DEREF] = HAS_ARG_FLAG | HAS_FREE_FLAG | HAS_ERROR_FLAG | HAS_ERROR_NO_POP_FLAG | HAS_ESCAPES_FLAG, - [_LOAD_DEREF] = HAS_ARG_FLAG | HAS_FREE_FLAG | HAS_ERROR_FLAG | HAS_ESCAPES_FLAG, + [_LOAD_DEREF] = HAS_ARG_FLAG | HAS_LOCAL_FLAG | HAS_ERROR_FLAG | HAS_ESCAPES_FLAG, [_STORE_DEREF] = HAS_ARG_FLAG | HAS_FREE_FLAG | HAS_ESCAPES_FLAG, [_COPY_FREE_VARS] = HAS_ARG_FLAG, [_BUILD_STRING] = HAS_ARG_FLAG | HAS_ERROR_FLAG, diff --git a/Python/bytecodes.c b/Python/bytecodes.c index c712c772201e10..d480244d2f3d51 100644 --- a/Python/bytecodes.c +++ b/Python/bytecodes.c @@ -1649,12 +1649,11 @@ dummy_func( inst(LOAD_DEREF, ( -- value)) { PyCellObject *cell = (PyCellObject *)PyStackRef_AsPyObjectBorrow(GETLOCAL(oparg)); - PyObject *value_o = PyCell_GetRef(cell); - if (value_o == NULL) { + _PyCell_GetStackRef(cell, &value); + if (PyStackRef_IsNull(value)) { _PyEval_FormatExcUnbound(tstate, _PyFrame_GetCode(frame), oparg); ERROR_IF(true, error); } - value = PyStackRef_FromPyObjectSteal(value_o); } inst(STORE_DEREF, (v --)) { diff --git a/Python/executor_cases.c.h b/Python/executor_cases.c.h index fdfec66b73c730..4914a58a640d26 100644 --- a/Python/executor_cases.c.h +++ b/Python/executor_cases.c.h @@ -1824,12 +1824,11 @@ _PyStackRef value; oparg = CURRENT_OPARG(); PyCellObject *cell = (PyCellObject *)PyStackRef_AsPyObjectBorrow(GETLOCAL(oparg)); - PyObject *value_o = PyCell_GetRef(cell); - if (value_o == NULL) { + _PyCell_GetStackRef(cell, &value); + if (PyStackRef_IsNull(value)) { _PyEval_FormatExcUnbound(tstate, _PyFrame_GetCode(frame), oparg); if (true) JUMP_TO_ERROR(); } - value = PyStackRef_FromPyObjectSteal(value_o); stack_pointer[0] = value; stack_pointer += 1; assert(WITHIN_STACK_BOUNDS()); diff --git a/Python/generated_cases.c.h b/Python/generated_cases.c.h index 9de7554d4dfd55..0099454fa91b11 100644 --- a/Python/generated_cases.c.h +++ b/Python/generated_cases.c.h @@ -5502,12 +5502,11 @@ INSTRUCTION_STATS(LOAD_DEREF); _PyStackRef value; PyCellObject *cell = (PyCellObject *)PyStackRef_AsPyObjectBorrow(GETLOCAL(oparg)); - PyObject *value_o = PyCell_GetRef(cell); - if (value_o == NULL) { + _PyCell_GetStackRef(cell, &value); + if (PyStackRef_IsNull(value)) { _PyEval_FormatExcUnbound(tstate, _PyFrame_GetCode(frame), oparg); if (true) goto error; } - value = PyStackRef_FromPyObjectSteal(value_o); stack_pointer[0] = value; stack_pointer += 1; assert(WITHIN_STACK_BOUNDS()); From ab34c927a7659bd66c0f3edb3d9f02b7b7c9fd2e Mon Sep 17 00:00:00 2001 From: Donghee Na Date: Wed, 25 Sep 2024 01:16:26 -0700 Subject: [PATCH 02/16] fix TSAN --- Include/internal/pycore_cell.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Include/internal/pycore_cell.h b/Include/internal/pycore_cell.h index ff9ec82d23f39e..63680f631232c7 100644 --- a/Include/internal/pycore_cell.h +++ b/Include/internal/pycore_cell.h @@ -19,8 +19,8 @@ PyCell_SwapTakeRef(PyCellObject *cell, PyObject *value) { PyObject *old_value; Py_BEGIN_CRITICAL_SECTION(cell); - old_value = cell->ob_ref; - cell->ob_ref = value; + FT_ATOMIC_STORE_PTR_RELEASE(old_value, cell->ob_ref); + FT_ATOMIC_STORE_PTR_RELEASE(cell->ob_ref, value); Py_END_CRITICAL_SECTION(); return old_value; } From 536b6fc0dc416aee89a875b1e9ce5354620cd081 Mon Sep 17 00:00:00 2001 From: Donghee Na Date: Wed, 25 Sep 2024 01:53:04 -0700 Subject: [PATCH 03/16] Update TSAN --- Tools/tsan/suppressions_free_threading.txt | 2 ++ 1 file changed, 2 insertions(+) diff --git a/Tools/tsan/suppressions_free_threading.txt b/Tools/tsan/suppressions_free_threading.txt index e5eb665ae212de..127d710bc58bdc 100644 --- a/Tools/tsan/suppressions_free_threading.txt +++ b/Tools/tsan/suppressions_free_threading.txt @@ -41,6 +41,8 @@ race_top:tstate_is_freed race_top:type_modified_unlocked race_top:write_thread_id race_top:PyThreadState_Clear +# see: https://github.com/python/cpython/issues/117721 +race_top:lock_PyThread_release_lock # Only seen on macOS, sample: https://gist.github.com/aisk/dda53f5d494a4556c35dde1fce03259c race_top:set_default_allocator_unlocked From 05f0cf988fef54319e4811f7c7cf871e441a2a7b Mon Sep 17 00:00:00 2001 From: Donghee Na Date: Wed, 25 Sep 2024 09:44:12 -0700 Subject: [PATCH 04/16] Revert "fix TSAN" This reverts commit c293ba50e1b3ca9e9622f10c4affc406461f7427. --- Include/internal/pycore_cell.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Include/internal/pycore_cell.h b/Include/internal/pycore_cell.h index 63680f631232c7..ff9ec82d23f39e 100644 --- a/Include/internal/pycore_cell.h +++ b/Include/internal/pycore_cell.h @@ -19,8 +19,8 @@ PyCell_SwapTakeRef(PyCellObject *cell, PyObject *value) { PyObject *old_value; Py_BEGIN_CRITICAL_SECTION(cell); - FT_ATOMIC_STORE_PTR_RELEASE(old_value, cell->ob_ref); - FT_ATOMIC_STORE_PTR_RELEASE(cell->ob_ref, value); + old_value = cell->ob_ref; + cell->ob_ref = value; Py_END_CRITICAL_SECTION(); return old_value; } From 617c7590107f7fc9cd7d9ca062d0165343648d40 Mon Sep 17 00:00:00 2001 From: Donghee Na Date: Wed, 25 Sep 2024 09:59:47 -0700 Subject: [PATCH 05/16] Reapply "fix TSAN" This reverts commit 05392cb069f5b748d0d443e148132bab865049e4. --- Include/internal/pycore_cell.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Include/internal/pycore_cell.h b/Include/internal/pycore_cell.h index ff9ec82d23f39e..63680f631232c7 100644 --- a/Include/internal/pycore_cell.h +++ b/Include/internal/pycore_cell.h @@ -19,8 +19,8 @@ PyCell_SwapTakeRef(PyCellObject *cell, PyObject *value) { PyObject *old_value; Py_BEGIN_CRITICAL_SECTION(cell); - old_value = cell->ob_ref; - cell->ob_ref = value; + FT_ATOMIC_STORE_PTR_RELEASE(old_value, cell->ob_ref); + FT_ATOMIC_STORE_PTR_RELEASE(cell->ob_ref, value); Py_END_CRITICAL_SECTION(); return old_value; } From 1b2b618adcb038596c647c0129e34d663c7d1433 Mon Sep 17 00:00:00 2001 From: Donghee Na Date: Wed, 25 Sep 2024 11:43:39 -0700 Subject: [PATCH 06/16] Address code review --- Include/internal/pycore_cell.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Include/internal/pycore_cell.h b/Include/internal/pycore_cell.h index 63680f631232c7..36fd7c1d2c3be4 100644 --- a/Include/internal/pycore_cell.h +++ b/Include/internal/pycore_cell.h @@ -19,7 +19,7 @@ PyCell_SwapTakeRef(PyCellObject *cell, PyObject *value) { PyObject *old_value; Py_BEGIN_CRITICAL_SECTION(cell); - FT_ATOMIC_STORE_PTR_RELEASE(old_value, cell->ob_ref); + old_value = cell->ob_ref; FT_ATOMIC_STORE_PTR_RELEASE(cell->ob_ref, value); Py_END_CRITICAL_SECTION(); return old_value; From cea6813c00e9e2a1e1ef885c0884e757772c9789 Mon Sep 17 00:00:00 2001 From: Donghee Na Date: Wed, 25 Sep 2024 14:25:32 -0700 Subject: [PATCH 07/16] Address code review --- Python/bytecodes.c | 6 +++--- Python/executor_cases.c.h | 8 ++++---- Python/generated_cases.c.h | 8 ++++---- Python/optimizer_cases.c.h | 8 +++++--- 4 files changed, 16 insertions(+), 14 deletions(-) diff --git a/Python/bytecodes.c b/Python/bytecodes.c index d480244d2f3d51..cdd5404cccee61 100644 --- a/Python/bytecodes.c +++ b/Python/bytecodes.c @@ -1647,10 +1647,10 @@ dummy_func( value = PyStackRef_FromPyObjectSteal(value_o); } - inst(LOAD_DEREF, ( -- value)) { + inst(LOAD_DEREF, ( -- value[1])) { PyCellObject *cell = (PyCellObject *)PyStackRef_AsPyObjectBorrow(GETLOCAL(oparg)); - _PyCell_GetStackRef(cell, &value); - if (PyStackRef_IsNull(value)) { + _PyCell_GetStackRef(cell, value); + if (PyStackRef_IsNull(*value)) { _PyEval_FormatExcUnbound(tstate, _PyFrame_GetCode(frame), oparg); ERROR_IF(true, error); } diff --git a/Python/executor_cases.c.h b/Python/executor_cases.c.h index 4914a58a640d26..9180c626bdcb3a 100644 --- a/Python/executor_cases.c.h +++ b/Python/executor_cases.c.h @@ -1821,15 +1821,15 @@ } case _LOAD_DEREF: { - _PyStackRef value; + _PyStackRef *value; oparg = CURRENT_OPARG(); + value = &stack_pointer[0]; PyCellObject *cell = (PyCellObject *)PyStackRef_AsPyObjectBorrow(GETLOCAL(oparg)); - _PyCell_GetStackRef(cell, &value); - if (PyStackRef_IsNull(value)) { + _PyCell_GetStackRef(cell, value); + if (PyStackRef_IsNull(*value)) { _PyEval_FormatExcUnbound(tstate, _PyFrame_GetCode(frame), oparg); if (true) JUMP_TO_ERROR(); } - stack_pointer[0] = value; stack_pointer += 1; assert(WITHIN_STACK_BOUNDS()); break; diff --git a/Python/generated_cases.c.h b/Python/generated_cases.c.h index 0099454fa91b11..778a8b4d58c62b 100644 --- a/Python/generated_cases.c.h +++ b/Python/generated_cases.c.h @@ -5500,14 +5500,14 @@ frame->instr_ptr = next_instr; next_instr += 1; INSTRUCTION_STATS(LOAD_DEREF); - _PyStackRef value; + _PyStackRef *value; + value = &stack_pointer[0]; PyCellObject *cell = (PyCellObject *)PyStackRef_AsPyObjectBorrow(GETLOCAL(oparg)); - _PyCell_GetStackRef(cell, &value); - if (PyStackRef_IsNull(value)) { + _PyCell_GetStackRef(cell, value); + if (PyStackRef_IsNull(*value)) { _PyEval_FormatExcUnbound(tstate, _PyFrame_GetCode(frame), oparg); if (true) goto error; } - stack_pointer[0] = value; stack_pointer += 1; assert(WITHIN_STACK_BOUNDS()); DISPATCH(); diff --git a/Python/optimizer_cases.c.h b/Python/optimizer_cases.c.h index fc0c0eff01d4c1..26695a26dd5c79 100644 --- a/Python/optimizer_cases.c.h +++ b/Python/optimizer_cases.c.h @@ -894,9 +894,11 @@ } case _LOAD_DEREF: { - _Py_UopsSymbol *value; - value = sym_new_not_null(ctx); - stack_pointer[0] = value; + _Py_UopsSymbol **value; + value = &stack_pointer[0]; + for (int _i = 1; --_i >= 0;) { + value[_i] = sym_new_not_null(ctx); + } stack_pointer += 1; assert(WITHIN_STACK_BOUNDS()); break; From a3a89f0f85b983ff51bb1c0d068229b82b73a24e Mon Sep 17 00:00:00 2001 From: Donghee Na Date: Thu, 26 Sep 2024 14:44:39 -0700 Subject: [PATCH 08/16] Remove whole inline --- Include/internal/pycore_cell.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Include/internal/pycore_cell.h b/Include/internal/pycore_cell.h index 36fd7c1d2c3be4..3cc6c2d9f6a4c3 100644 --- a/Include/internal/pycore_cell.h +++ b/Include/internal/pycore_cell.h @@ -33,7 +33,7 @@ PyCell_SetTakeRef(PyCellObject *cell, PyObject *value) } // Gets the cell contents. Returns a new reference. -static inline PyObject * +static PyObject * PyCell_GetRef(PyCellObject *cell) { PyObject *res; @@ -43,7 +43,7 @@ PyCell_GetRef(PyCellObject *cell) return res; } -static inline void +static void _PyCell_GetStackRef(PyCellObject *cell, _PyStackRef *value_addr) { PyObject *value; From 4ad83c1e127d869fcb4545f46efba1d1fcf06c59 Mon Sep 17 00:00:00 2001 From: Donghee Na Date: Thu, 26 Sep 2024 15:08:17 -0700 Subject: [PATCH 09/16] Revert "Remove whole inline" This reverts commit 6dbbaca2a2700c846a4a1c494a8fdead01b5475b. --- Include/internal/pycore_cell.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Include/internal/pycore_cell.h b/Include/internal/pycore_cell.h index 3cc6c2d9f6a4c3..36fd7c1d2c3be4 100644 --- a/Include/internal/pycore_cell.h +++ b/Include/internal/pycore_cell.h @@ -33,7 +33,7 @@ PyCell_SetTakeRef(PyCellObject *cell, PyObject *value) } // Gets the cell contents. Returns a new reference. -static PyObject * +static inline PyObject * PyCell_GetRef(PyCellObject *cell) { PyObject *res; @@ -43,7 +43,7 @@ PyCell_GetRef(PyCellObject *cell) return res; } -static void +static inline void _PyCell_GetStackRef(PyCellObject *cell, _PyStackRef *value_addr) { PyObject *value; From ee3113d6a6457c9b6258c5b67af58b165b519d66 Mon Sep 17 00:00:00 2001 From: Donghee Na Date: Fri, 27 Sep 2024 11:05:02 -0700 Subject: [PATCH 10/16] Address code review from Mark --- Include/internal/pycore_cell.h | 15 ++++++--------- Python/bytecodes.c | 6 +++--- Python/executor_cases.c.h | 8 ++++---- Python/generated_cases.c.h | 8 ++++---- Python/optimizer_cases.c.h | 8 +++----- 5 files changed, 20 insertions(+), 25 deletions(-) diff --git a/Include/internal/pycore_cell.h b/Include/internal/pycore_cell.h index 36fd7c1d2c3be4..1f20429967beb7 100644 --- a/Include/internal/pycore_cell.h +++ b/Include/internal/pycore_cell.h @@ -43,29 +43,26 @@ PyCell_GetRef(PyCellObject *cell) return res; } -static inline void -_PyCell_GetStackRef(PyCellObject *cell, _PyStackRef *value_addr) +static inline +_PyStackRef _PyCell_GetStackRef(PyCellObject *cell) { PyObject *value; #ifdef Py_GIL_DISABLED value = _Py_atomic_load_ptr(&cell->ob_ref); if (value != NULL) { if (_Py_IsImmortal(value) || _PyObject_HasDeferredRefcount(value)) { - *value_addr = (_PyStackRef){ .bits = (uintptr_t)value | Py_TAG_DEFERRED }; - return; + return (_PyStackRef){ .bits = (uintptr_t)value | Py_TAG_DEFERRED }; } if (_Py_TryIncrefCompare(&cell->ob_ref, value)) { - *value_addr = _PyStackRef_FromPyObjectSteal(value); - return; + return _PyStackRef_FromPyObjectSteal(value); } } #endif value = PyCell_GetRef(cell); if (value == NULL) { - *value_addr = PyStackRef_NULL; - return; + return PyStackRef_NULL; } - *value_addr = PyStackRef_FromPyObjectSteal(value); + return PyStackRef_FromPyObjectSteal(value); } #ifdef __cplusplus diff --git a/Python/bytecodes.c b/Python/bytecodes.c index cdd5404cccee61..82ed7b2c099347 100644 --- a/Python/bytecodes.c +++ b/Python/bytecodes.c @@ -1647,10 +1647,10 @@ dummy_func( value = PyStackRef_FromPyObjectSteal(value_o); } - inst(LOAD_DEREF, ( -- value[1])) { + inst(LOAD_DEREF, ( -- value)) { PyCellObject *cell = (PyCellObject *)PyStackRef_AsPyObjectBorrow(GETLOCAL(oparg)); - _PyCell_GetStackRef(cell, value); - if (PyStackRef_IsNull(*value)) { + value = _PyCell_GetStackRef(cell); + if (PyStackRef_IsNull(value)) { _PyEval_FormatExcUnbound(tstate, _PyFrame_GetCode(frame), oparg); ERROR_IF(true, error); } diff --git a/Python/executor_cases.c.h b/Python/executor_cases.c.h index 9180c626bdcb3a..9f36420133b648 100644 --- a/Python/executor_cases.c.h +++ b/Python/executor_cases.c.h @@ -1821,15 +1821,15 @@ } case _LOAD_DEREF: { - _PyStackRef *value; + _PyStackRef value; oparg = CURRENT_OPARG(); - value = &stack_pointer[0]; PyCellObject *cell = (PyCellObject *)PyStackRef_AsPyObjectBorrow(GETLOCAL(oparg)); - _PyCell_GetStackRef(cell, value); - if (PyStackRef_IsNull(*value)) { + value = _PyCell_GetStackRef(cell); + if (PyStackRef_IsNull(value)) { _PyEval_FormatExcUnbound(tstate, _PyFrame_GetCode(frame), oparg); if (true) JUMP_TO_ERROR(); } + stack_pointer[0] = value; stack_pointer += 1; assert(WITHIN_STACK_BOUNDS()); break; diff --git a/Python/generated_cases.c.h b/Python/generated_cases.c.h index 778a8b4d58c62b..753c3c94c88fad 100644 --- a/Python/generated_cases.c.h +++ b/Python/generated_cases.c.h @@ -5500,14 +5500,14 @@ frame->instr_ptr = next_instr; next_instr += 1; INSTRUCTION_STATS(LOAD_DEREF); - _PyStackRef *value; - value = &stack_pointer[0]; + _PyStackRef value; PyCellObject *cell = (PyCellObject *)PyStackRef_AsPyObjectBorrow(GETLOCAL(oparg)); - _PyCell_GetStackRef(cell, value); - if (PyStackRef_IsNull(*value)) { + value = _PyCell_GetStackRef(cell); + if (PyStackRef_IsNull(value)) { _PyEval_FormatExcUnbound(tstate, _PyFrame_GetCode(frame), oparg); if (true) goto error; } + stack_pointer[0] = value; stack_pointer += 1; assert(WITHIN_STACK_BOUNDS()); DISPATCH(); diff --git a/Python/optimizer_cases.c.h b/Python/optimizer_cases.c.h index 26695a26dd5c79..fc0c0eff01d4c1 100644 --- a/Python/optimizer_cases.c.h +++ b/Python/optimizer_cases.c.h @@ -894,11 +894,9 @@ } case _LOAD_DEREF: { - _Py_UopsSymbol **value; - value = &stack_pointer[0]; - for (int _i = 1; --_i >= 0;) { - value[_i] = sym_new_not_null(ctx); - } + _Py_UopsSymbol *value; + value = sym_new_not_null(ctx); + stack_pointer[0] = value; stack_pointer += 1; assert(WITHIN_STACK_BOUNDS()); break; From a7c71816423b1fe4ed4c1ae4338b5edf6fc6c355 Mon Sep 17 00:00:00 2001 From: Donghee Na Date: Fri, 27 Sep 2024 15:19:54 -0700 Subject: [PATCH 11/16] [WIP] Comment out _Py_TryIncrefCompare --- Include/internal/pycore_cell.h | 2 ++ 1 file changed, 2 insertions(+) diff --git a/Include/internal/pycore_cell.h b/Include/internal/pycore_cell.h index 1f20429967beb7..88fe28d7171e3f 100644 --- a/Include/internal/pycore_cell.h +++ b/Include/internal/pycore_cell.h @@ -53,9 +53,11 @@ _PyStackRef _PyCell_GetStackRef(PyCellObject *cell) if (_Py_IsImmortal(value) || _PyObject_HasDeferredRefcount(value)) { return (_PyStackRef){ .bits = (uintptr_t)value | Py_TAG_DEFERRED }; } + /** if (_Py_TryIncrefCompare(&cell->ob_ref, value)) { return _PyStackRef_FromPyObjectSteal(value); } + **/ } #endif value = PyCell_GetRef(cell); From 90ef0d8661ab46d139ac57de2457dc3e66e2fa98 Mon Sep 17 00:00:00 2001 From: Donghee Na Date: Fri, 27 Sep 2024 16:30:34 -0700 Subject: [PATCH 12/16] test SwitchToThread() --- Include/internal/pycore_cell.h | 2 -- Include/internal/pycore_object.h | 7 +++++++ 2 files changed, 7 insertions(+), 2 deletions(-) diff --git a/Include/internal/pycore_cell.h b/Include/internal/pycore_cell.h index 88fe28d7171e3f..1f20429967beb7 100644 --- a/Include/internal/pycore_cell.h +++ b/Include/internal/pycore_cell.h @@ -53,11 +53,9 @@ _PyStackRef _PyCell_GetStackRef(PyCellObject *cell) if (_Py_IsImmortal(value) || _PyObject_HasDeferredRefcount(value)) { return (_PyStackRef){ .bits = (uintptr_t)value | Py_TAG_DEFERRED }; } - /** if (_Py_TryIncrefCompare(&cell->ob_ref, value)) { return _PyStackRef_FromPyObjectSteal(value); } - **/ } #endif value = PyCell_GetRef(cell); diff --git a/Include/internal/pycore_object.h b/Include/internal/pycore_object.h index 0af13b1bcda20b..288ceb95dadee1 100644 --- a/Include/internal/pycore_object.h +++ b/Include/internal/pycore_object.h @@ -16,6 +16,10 @@ extern "C" { #include "pycore_pystate.h" // _PyInterpreterState_GET() #include "pycore_uniqueid.h" // _PyType_IncrefSlow +#ifdef MS_WINDOWS +# define WIN32_LEAN_AND_MEAN +# include // SwitchToThread() +#endif #define _Py_IMMORTAL_REFCNT_LOOSE ((_Py_IMMORTAL_REFCNT >> 1) + 1) @@ -550,6 +554,9 @@ _Py_TryIncRefShared(PyObject *op) _Py_INCREF_STAT_INC(); return 1; } + #ifdef MS_WINDOWS + SwitchToThread(); + #endif } } From f16009c9827d64a14a2d77e0cce0485b7e7fec7a Mon Sep 17 00:00:00 2001 From: Donghee Na Date: Fri, 27 Sep 2024 17:14:34 -0700 Subject: [PATCH 13/16] test fence --- Include/internal/pycore_cell.h | 2 ++ Include/internal/pycore_object.h | 8 -------- 2 files changed, 2 insertions(+), 8 deletions(-) diff --git a/Include/internal/pycore_cell.h b/Include/internal/pycore_cell.h index 1f20429967beb7..07c7008b029db7 100644 --- a/Include/internal/pycore_cell.h +++ b/Include/internal/pycore_cell.h @@ -48,6 +48,7 @@ _PyStackRef _PyCell_GetStackRef(PyCellObject *cell) { PyObject *value; #ifdef Py_GIL_DISABLED + _Py_atomic_fence_acquire(); value = _Py_atomic_load_ptr(&cell->ob_ref); if (value != NULL) { if (_Py_IsImmortal(value) || _PyObject_HasDeferredRefcount(value)) { @@ -57,6 +58,7 @@ _PyStackRef _PyCell_GetStackRef(PyCellObject *cell) return _PyStackRef_FromPyObjectSteal(value); } } + _Py_atomic_fence_release(); #endif value = PyCell_GetRef(cell); if (value == NULL) { diff --git a/Include/internal/pycore_object.h b/Include/internal/pycore_object.h index 288ceb95dadee1..cc6389dc6a4f2a 100644 --- a/Include/internal/pycore_object.h +++ b/Include/internal/pycore_object.h @@ -16,11 +16,6 @@ extern "C" { #include "pycore_pystate.h" // _PyInterpreterState_GET() #include "pycore_uniqueid.h" // _PyType_IncrefSlow -#ifdef MS_WINDOWS -# define WIN32_LEAN_AND_MEAN -# include // SwitchToThread() -#endif - #define _Py_IMMORTAL_REFCNT_LOOSE ((_Py_IMMORTAL_REFCNT >> 1) + 1) // This value is added to `ob_ref_shared` for objects that use deferred @@ -554,9 +549,6 @@ _Py_TryIncRefShared(PyObject *op) _Py_INCREF_STAT_INC(); return 1; } - #ifdef MS_WINDOWS - SwitchToThread(); - #endif } } From 9edf409572c92c0f44b294d221fdf9015aa4923a Mon Sep 17 00:00:00 2001 From: Donghee Na Date: Sat, 28 Sep 2024 00:14:11 -0700 Subject: [PATCH 14/16] Comment out immortal and deferred case --- Include/internal/pycore_cell.h | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/Include/internal/pycore_cell.h b/Include/internal/pycore_cell.h index 07c7008b029db7..12386abd4aabf9 100644 --- a/Include/internal/pycore_cell.h +++ b/Include/internal/pycore_cell.h @@ -48,17 +48,15 @@ _PyStackRef _PyCell_GetStackRef(PyCellObject *cell) { PyObject *value; #ifdef Py_GIL_DISABLED - _Py_atomic_fence_acquire(); value = _Py_atomic_load_ptr(&cell->ob_ref); if (value != NULL) { - if (_Py_IsImmortal(value) || _PyObject_HasDeferredRefcount(value)) { - return (_PyStackRef){ .bits = (uintptr_t)value | Py_TAG_DEFERRED }; - } + // if (_Py_IsImmortal(value) || _PyObject_HasDeferredRefcount(value)) { + // return (_PyStackRef){ .bits = (uintptr_t)value | Py_TAG_DEFERRED }; + // } if (_Py_TryIncrefCompare(&cell->ob_ref, value)) { return _PyStackRef_FromPyObjectSteal(value); } } - _Py_atomic_fence_release(); #endif value = PyCell_GetRef(cell); if (value == NULL) { From 1c8e9230e80ac49672820a6882f0306d1fb9bd05 Mon Sep 17 00:00:00 2001 From: Donghee Na Date: Sat, 28 Sep 2024 00:36:07 -0700 Subject: [PATCH 15/16] test --- Include/internal/pycore_cell.h | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/Include/internal/pycore_cell.h b/Include/internal/pycore_cell.h index 12386abd4aabf9..b7a86ca0abf4a2 100644 --- a/Include/internal/pycore_cell.h +++ b/Include/internal/pycore_cell.h @@ -48,15 +48,16 @@ _PyStackRef _PyCell_GetStackRef(PyCellObject *cell) { PyObject *value; #ifdef Py_GIL_DISABLED + /** value = _Py_atomic_load_ptr(&cell->ob_ref); if (value != NULL) { - // if (_Py_IsImmortal(value) || _PyObject_HasDeferredRefcount(value)) { - // return (_PyStackRef){ .bits = (uintptr_t)value | Py_TAG_DEFERRED }; - // } + if (_Py_IsImmortal(value) || _PyObject_HasDeferredRefcount(value)) { + return (_PyStackRef){ .bits = (uintptr_t)value | Py_TAG_DEFERRED }; + } if (_Py_TryIncrefCompare(&cell->ob_ref, value)) { return _PyStackRef_FromPyObjectSteal(value); } - } + }**/ #endif value = PyCell_GetRef(cell); if (value == NULL) { From 2ec127825254bce03a067d9b37ab3544b7d8a843 Mon Sep 17 00:00:00 2001 From: Donghee Na Date: Thu, 3 Oct 2024 11:39:28 +0900 Subject: [PATCH 16/16] Test _Py_TryIncrefFast only --- Include/internal/pycore_cell.h | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/Include/internal/pycore_cell.h b/Include/internal/pycore_cell.h index b7a86ca0abf4a2..5a12a2f14571af 100644 --- a/Include/internal/pycore_cell.h +++ b/Include/internal/pycore_cell.h @@ -48,16 +48,15 @@ _PyStackRef _PyCell_GetStackRef(PyCellObject *cell) { PyObject *value; #ifdef Py_GIL_DISABLED - /** value = _Py_atomic_load_ptr(&cell->ob_ref); if (value != NULL) { if (_Py_IsImmortal(value) || _PyObject_HasDeferredRefcount(value)) { return (_PyStackRef){ .bits = (uintptr_t)value | Py_TAG_DEFERRED }; } - if (_Py_TryIncrefCompare(&cell->ob_ref, value)) { + if (_Py_TryIncrefFast(value)) { return _PyStackRef_FromPyObjectSteal(value); } - }**/ + } #endif value = PyCell_GetRef(cell); if (value == NULL) {