From b8a2a0823884e944281f860fc0495b185c666c26 Mon Sep 17 00:00:00 2001 From: Manjusaka Date: Tue, 16 Dec 2025 01:17:29 +0800 Subject: [PATCH 1/2] gh-134584: Eliminate redundant refcounting from _STORE_ATTR_INSTANCE_VALUE Signed-off-by: Manjusaka --- Include/internal/pycore_opcode_metadata.h | 2 +- Include/internal/pycore_uop_ids.h | 2 +- Include/internal/pycore_uop_metadata.h | 6 +++--- Lib/test/test_capi/test_opt.py | 17 +++++++++++++++++ ...25-12-16-01-17-21.gh-issue-134584.tsxYYw.rst | 1 + Python/bytecodes.c | 8 +++++--- Python/executor_cases.c.h | 14 ++++++++++---- Python/generated_cases.c.h | 15 +++++++++++++-- Python/optimizer_bytecodes.c | 5 +++++ Python/optimizer_cases.c.h | 13 +++++++++++-- 10 files changed, 67 insertions(+), 16 deletions(-) create mode 100644 Misc/NEWS.d/next/Core_and_Builtins/2025-12-16-01-17-21.gh-issue-134584.tsxYYw.rst diff --git a/Include/internal/pycore_opcode_metadata.h b/Include/internal/pycore_opcode_metadata.h index fda52806561430..ae038cd716c55c 100644 --- a/Include/internal/pycore_opcode_metadata.h +++ b/Include/internal/pycore_opcode_metadata.h @@ -1482,7 +1482,7 @@ _PyOpcode_macro_expansion[256] = { [SET_FUNCTION_ATTRIBUTE] = { .nuops = 1, .uops = { { _SET_FUNCTION_ATTRIBUTE, OPARG_SIMPLE, 0 } } }, [SET_UPDATE] = { .nuops = 1, .uops = { { _SET_UPDATE, OPARG_SIMPLE, 0 } } }, [STORE_ATTR] = { .nuops = 1, .uops = { { _STORE_ATTR, OPARG_SIMPLE, 3 } } }, - [STORE_ATTR_INSTANCE_VALUE] = { .nuops = 3, .uops = { { _GUARD_TYPE_VERSION_AND_LOCK, 2, 1 }, { _GUARD_DORV_NO_DICT, OPARG_SIMPLE, 3 }, { _STORE_ATTR_INSTANCE_VALUE, 1, 3 } } }, + [STORE_ATTR_INSTANCE_VALUE] = { .nuops = 4, .uops = { { _GUARD_TYPE_VERSION_AND_LOCK, 2, 1 }, { _GUARD_DORV_NO_DICT, OPARG_SIMPLE, 3 }, { _STORE_ATTR_INSTANCE_VALUE, 1, 3 }, { _POP_TOP, OPARG_SIMPLE, 4 } } }, [STORE_ATTR_SLOT] = { .nuops = 2, .uops = { { _GUARD_TYPE_VERSION, 2, 1 }, { _STORE_ATTR_SLOT, 1, 3 } } }, [STORE_ATTR_WITH_HINT] = { .nuops = 2, .uops = { { _GUARD_TYPE_VERSION, 2, 1 }, { _STORE_ATTR_WITH_HINT, 1, 3 } } }, [STORE_DEREF] = { .nuops = 1, .uops = { { _STORE_DEREF, OPARG_SIMPLE, 0 } } }, diff --git a/Include/internal/pycore_uop_ids.h b/Include/internal/pycore_uop_ids.h index 42793d940b51b7..5b0c5c89ddb2df 100644 --- a/Include/internal/pycore_uop_ids.h +++ b/Include/internal/pycore_uop_ids.h @@ -1051,7 +1051,7 @@ extern "C" { #define _SPILL_OR_RELOAD_r32 1244 #define _START_EXECUTOR_r00 1245 #define _STORE_ATTR_r20 1246 -#define _STORE_ATTR_INSTANCE_VALUE_r20 1247 +#define _STORE_ATTR_INSTANCE_VALUE_r21 1247 #define _STORE_ATTR_SLOT_r20 1248 #define _STORE_ATTR_WITH_HINT_r20 1249 #define _STORE_DEREF_r10 1250 diff --git a/Include/internal/pycore_uop_metadata.h b/Include/internal/pycore_uop_metadata.h index ec47c526ff122d..8342bf64e60391 100644 --- a/Include/internal/pycore_uop_metadata.h +++ b/Include/internal/pycore_uop_metadata.h @@ -1858,7 +1858,7 @@ const _PyUopCachingInfo _PyUop_Caching[MAX_UOP_ID+1] = { .entries = { { -1, -1, -1 }, { -1, -1, -1 }, - { 0, 2, _STORE_ATTR_INSTANCE_VALUE_r20 }, + { 1, 2, _STORE_ATTR_INSTANCE_VALUE_r21 }, { -1, -1, -1 }, }, }, @@ -3587,7 +3587,7 @@ const uint16_t _PyUop_Uncached[MAX_UOP_REGS_ID+1] = { [_GUARD_DORV_NO_DICT_r11] = _GUARD_DORV_NO_DICT, [_GUARD_DORV_NO_DICT_r22] = _GUARD_DORV_NO_DICT, [_GUARD_DORV_NO_DICT_r33] = _GUARD_DORV_NO_DICT, - [_STORE_ATTR_INSTANCE_VALUE_r20] = _STORE_ATTR_INSTANCE_VALUE, + [_STORE_ATTR_INSTANCE_VALUE_r21] = _STORE_ATTR_INSTANCE_VALUE, [_STORE_ATTR_WITH_HINT_r20] = _STORE_ATTR_WITH_HINT, [_STORE_ATTR_SLOT_r20] = _STORE_ATTR_SLOT, [_COMPARE_OP_r21] = _COMPARE_OP, @@ -4839,7 +4839,7 @@ const char *const _PyOpcode_uop_name[MAX_UOP_REGS_ID+1] = { [_STORE_ATTR] = "_STORE_ATTR", [_STORE_ATTR_r20] = "_STORE_ATTR_r20", [_STORE_ATTR_INSTANCE_VALUE] = "_STORE_ATTR_INSTANCE_VALUE", - [_STORE_ATTR_INSTANCE_VALUE_r20] = "_STORE_ATTR_INSTANCE_VALUE_r20", + [_STORE_ATTR_INSTANCE_VALUE_r21] = "_STORE_ATTR_INSTANCE_VALUE_r21", [_STORE_ATTR_SLOT] = "_STORE_ATTR_SLOT", [_STORE_ATTR_SLOT_r20] = "_STORE_ATTR_SLOT_r20", [_STORE_ATTR_WITH_HINT] = "_STORE_ATTR_WITH_HINT", diff --git a/Lib/test/test_capi/test_opt.py b/Lib/test/test_capi/test_opt.py index e17367ca71ea38..6d271a840420d9 100644 --- a/Lib/test/test_capi/test_opt.py +++ b/Lib/test/test_capi/test_opt.py @@ -2511,6 +2511,23 @@ def testfunc(n): self.assertNotIn("_GUARD_TOS_INT", uops) self.assertNotIn("_GUARD_NOS_INT", uops) + def test_store_attr_instance_value(self): + def testfunc(n): + class C: + pass + c = C() + for i in range(n): + c.a = i + return c.a + res, ex = self._run_with_optimizer(testfunc, TIER2_THRESHOLD) + self.assertEqual(res, TIER2_THRESHOLD - 1) + self.assertIsNotNone(ex) + uops = get_opnames(ex) + + self.assertIn("_STORE_ATTR_INSTANCE_VALUE", uops) + self.assertNotIn("_POP_TOP", uops) + self.assertIn("_POP_TOP_NOP", uops) + def test_store_subscr_int(self): def testfunc(n): l = [0, 0, 0, 0] diff --git a/Misc/NEWS.d/next/Core_and_Builtins/2025-12-16-01-17-21.gh-issue-134584.tsxYYw.rst b/Misc/NEWS.d/next/Core_and_Builtins/2025-12-16-01-17-21.gh-issue-134584.tsxYYw.rst new file mode 100644 index 00000000000000..66bafbfc734bb1 --- /dev/null +++ b/Misc/NEWS.d/next/Core_and_Builtins/2025-12-16-01-17-21.gh-issue-134584.tsxYYw.rst @@ -0,0 +1 @@ +Eliminate redundant refcounting from ``_STORE_ATTR_INSTANCE_VALUE``. diff --git a/Python/bytecodes.c b/Python/bytecodes.c index 3e6147961f1822..b47effcfde9a6b 100644 --- a/Python/bytecodes.c +++ b/Python/bytecodes.c @@ -2609,7 +2609,7 @@ dummy_func( } } - op(_STORE_ATTR_INSTANCE_VALUE, (offset/1, value, owner --)) { + op(_STORE_ATTR_INSTANCE_VALUE, (offset/1, value, owner -- o)) { PyObject *owner_o = PyStackRef_AsPyObjectBorrow(owner); STAT_INC(STORE_ATTR, hit); @@ -2623,7 +2623,8 @@ dummy_func( _PyDictValues_AddToInsertionOrder(values, index); } UNLOCK_OBJECT(owner_o); - PyStackRef_CLOSE(owner); + o = owner; + DEAD(owner); Py_XDECREF(old_value); } @@ -2631,7 +2632,8 @@ dummy_func( unused/1 + _GUARD_TYPE_VERSION_AND_LOCK + _GUARD_DORV_NO_DICT + - _STORE_ATTR_INSTANCE_VALUE; + _STORE_ATTR_INSTANCE_VALUE + + POP_TOP; op(_STORE_ATTR_WITH_HINT, (hint/1, value, owner --)) { PyObject *owner_o = PyStackRef_AsPyObjectBorrow(owner); diff --git a/Python/executor_cases.c.h b/Python/executor_cases.c.h index e7f6bb2ed0ce0d..cf1ce4cd3c4de5 100644 --- a/Python/executor_cases.c.h +++ b/Python/executor_cases.c.h @@ -8766,11 +8766,12 @@ break; } - case _STORE_ATTR_INSTANCE_VALUE_r20: { + case _STORE_ATTR_INSTANCE_VALUE_r21: { CHECK_CURRENT_CACHED_VALUES(2); assert(WITHIN_STACK_BOUNDS_WITH_CACHE()); _PyStackRef owner; _PyStackRef value; + _PyStackRef o; _PyStackRef _stack_item_0 = _tos_cache0; _PyStackRef _stack_item_1 = _tos_cache1; owner = _stack_item_1; @@ -8788,14 +8789,19 @@ _PyDictValues_AddToInsertionOrder(values, index); } UNLOCK_OBJECT(owner_o); + o = owner; + stack_pointer[0] = o; + stack_pointer += 1; + ASSERT_WITHIN_STACK_BOUNDS(__FILE__, __LINE__); _PyFrame_SetStackPointer(frame, stack_pointer); - PyStackRef_CLOSE(owner); Py_XDECREF(old_value); stack_pointer = _PyFrame_GetStackPointer(frame); - _tos_cache0 = PyStackRef_ZERO_BITS; + _tos_cache0 = o; _tos_cache1 = PyStackRef_ZERO_BITS; _tos_cache2 = PyStackRef_ZERO_BITS; - SET_CURRENT_CACHED_VALUES(0); + SET_CURRENT_CACHED_VALUES(1); + stack_pointer += -1; + ASSERT_WITHIN_STACK_BOUNDS(__FILE__, __LINE__); assert(WITHIN_STACK_BOUNDS_WITH_CACHE()); break; } diff --git a/Python/generated_cases.c.h b/Python/generated_cases.c.h index 9c828cba877ad4..c67df8c71360ac 100644 --- a/Python/generated_cases.c.h +++ b/Python/generated_cases.c.h @@ -10487,6 +10487,7 @@ static_assert(INLINE_CACHE_ENTRIES_STORE_ATTR == 4, "incorrect cache size"); _PyStackRef owner; _PyStackRef value; + _PyStackRef o; /* Skip 1 cache entry */ // _GUARD_TYPE_VERSION_AND_LOCK { @@ -10540,13 +10541,23 @@ _PyDictValues_AddToInsertionOrder(values, index); } UNLOCK_OBJECT(owner_o); - stack_pointer += -2; + o = owner; + stack_pointer[-2] = o; + stack_pointer += -1; ASSERT_WITHIN_STACK_BOUNDS(__FILE__, __LINE__); _PyFrame_SetStackPointer(frame, stack_pointer); - PyStackRef_CLOSE(owner); Py_XDECREF(old_value); stack_pointer = _PyFrame_GetStackPointer(frame); } + // _POP_TOP + { + value = o; + stack_pointer += -1; + ASSERT_WITHIN_STACK_BOUNDS(__FILE__, __LINE__); + _PyFrame_SetStackPointer(frame, stack_pointer); + PyStackRef_XCLOSE(value); + stack_pointer = _PyFrame_GetStackPointer(frame); + } DISPATCH(); } diff --git a/Python/optimizer_bytecodes.c b/Python/optimizer_bytecodes.c index 5023f84213b359..1f1df92d313267 100644 --- a/Python/optimizer_bytecodes.c +++ b/Python/optimizer_bytecodes.c @@ -99,6 +99,11 @@ dummy_func(void) { GETLOCAL(oparg) = temp; } + op(_STORE_ATTR_INSTANCE_VALUE, (offset/1, value, owner -- o)) { + (void)value; + o = owner; + } + op(_STORE_FAST, (value --)) { GETLOCAL(oparg) = value; } diff --git a/Python/optimizer_cases.c.h b/Python/optimizer_cases.c.h index 72564ea32db772..3ffc147b1d9814 100644 --- a/Python/optimizer_cases.c.h +++ b/Python/optimizer_cases.c.h @@ -1898,8 +1898,17 @@ } case _STORE_ATTR_INSTANCE_VALUE: { - CHECK_STACK_BOUNDS(-2); - stack_pointer += -2; + JitOptRef owner; + JitOptRef value; + JitOptRef o; + owner = stack_pointer[-1]; + value = stack_pointer[-2]; + uint16_t offset = (uint16_t)this_instr->operand0; + (void)value; + o = owner; + CHECK_STACK_BOUNDS(-1); + stack_pointer[-2] = o; + stack_pointer += -1; ASSERT_WITHIN_STACK_BOUNDS(__FILE__, __LINE__); break; } From bee7841efa3633b4951e83cbdd5f25444b66faa7 Mon Sep 17 00:00:00 2001 From: Manjusaka Date: Tue, 16 Dec 2025 01:21:29 +0800 Subject: [PATCH 2/2] fix lint Signed-off-by: Manjusaka --- Python/bytecodes.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Python/bytecodes.c b/Python/bytecodes.c index b47effcfde9a6b..145d97636ebafc 100644 --- a/Python/bytecodes.c +++ b/Python/bytecodes.c @@ -2624,7 +2624,7 @@ dummy_func( } UNLOCK_OBJECT(owner_o); o = owner; - DEAD(owner); + DEAD(owner); Py_XDECREF(old_value); }