Skip to content

Commit 5583ac0

Browse files
committed
pythongh-127022: Simplify PyStackRef_FromPyObjectSteal
This gets rid of the immortal check in `PyStackRef_FromPyObjectSteal()`. Overall, this improves performance about 2% in the free threading build. This also renames `PyStackRef_Is()` to `PyStackRef_IsExactly()` because the macro requires that the tag bits of the arguments match, which is only true in certain special cases.
1 parent 29cbcbd commit 5583ac0

File tree

5 files changed

+80
-72
lines changed

5 files changed

+80
-72
lines changed

Include/internal/pycore_stackref.h

+9-4
Original file line numberDiff line numberDiff line change
@@ -99,8 +99,7 @@ _PyStackRef_FromPyObjectSteal(PyObject *obj)
9999
assert(obj != NULL);
100100
// Make sure we don't take an already tagged value.
101101
assert(((uintptr_t)obj & Py_TAG_BITS) == 0);
102-
unsigned int tag = _Py_IsImmortal(obj) ? (Py_TAG_DEFERRED) : Py_TAG_PTR;
103-
return ((_PyStackRef){.bits = ((uintptr_t)(obj)) | tag});
102+
return (_PyStackRef){ .bits = (uintptr_t)obj };
104103
}
105104
# define PyStackRef_FromPyObjectSteal(obj) _PyStackRef_FromPyObjectSteal(_PyObject_CAST(obj))
106105

@@ -190,9 +189,15 @@ static const _PyStackRef PyStackRef_NULL = { .bits = 0 };
190189

191190
#endif // Py_GIL_DISABLED
192191

193-
// Note: this is a macro because MSVC (Windows) has trouble inlining it.
192+
// Check if a stackref is exactly the same as another stackref, including the
193+
// the deferred bit. This can only be used safely if you know that the deferred
194+
// of `a` and `b` bits match.
195+
#define PyStackRef_IsExactly(a, b) ((a).bits == (b).bits)
194196

195-
#define PyStackRef_Is(a, b) ((a).bits == (b).bits)
197+
// Checks that mask out the deferred bit in the free threading build.
198+
#define PyStackRef_IsNone(ref) (PyStackRef_AsPyObjectBorrow(ref) == Py_None)
199+
#define PyStackRef_IsTrue(ref) (PyStackRef_AsPyObjectBorrow(ref) == Py_True)
200+
#define PyStackRef_IsFalse(ref) (PyStackRef_AsPyObjectBorrow(ref) == Py_False)
196201

197202
// Converts a PyStackRef back to a PyObject *, converting the
198203
// stackref to a new reference.

Python/bytecodes.c

+25-25
Original file line numberDiff line numberDiff line change
@@ -376,7 +376,7 @@ dummy_func(
376376

377377
pure inst(UNARY_NOT, (value -- res)) {
378378
assert(PyStackRef_BoolCheck(value));
379-
res = PyStackRef_Is(value, PyStackRef_False)
379+
res = PyStackRef_IsExactly(value, PyStackRef_False)
380380
? PyStackRef_True : PyStackRef_False;
381381
DEAD(value);
382382
}
@@ -441,7 +441,7 @@ dummy_func(
441441

442442
inst(TO_BOOL_NONE, (unused/1, unused/2, value -- res)) {
443443
// This one is a bit weird, because we expect *some* failures:
444-
EXIT_IF(!PyStackRef_Is(value, PyStackRef_None));
444+
EXIT_IF(!PyStackRef_IsNone(value));
445445
DEAD(value);
446446
STAT_INC(TO_BOOL, hit);
447447
res = PyStackRef_False;
@@ -651,9 +651,7 @@ dummy_func(
651651
// specializations, but there is no output.
652652
// At the end we just skip over the STORE_FAST.
653653
op(_BINARY_OP_INPLACE_ADD_UNICODE, (left, right --)) {
654-
#ifndef NDEBUG
655654
PyObject *left_o = PyStackRef_AsPyObjectBorrow(left);
656-
#endif
657655
PyObject *right_o = PyStackRef_AsPyObjectBorrow(right);
658656

659657
int next_oparg;
@@ -664,7 +662,7 @@ dummy_func(
664662
next_oparg = CURRENT_OPERAND0();
665663
#endif
666664
_PyStackRef *target_local = &GETLOCAL(next_oparg);
667-
DEOPT_IF(!PyStackRef_Is(*target_local, left));
665+
DEOPT_IF(PyStackRef_AsPyObjectBorrow(*target_local) != left_o);
668666
STAT_INC(BINARY_OP, hit);
669667
/* Handle `left = left + right` or `left += right` for str.
670668
*
@@ -1141,7 +1139,7 @@ dummy_func(
11411139
gen_frame->previous = frame;
11421140
DISPATCH_INLINED(gen_frame);
11431141
}
1144-
if (PyStackRef_Is(v, PyStackRef_None) && PyIter_Check(receiver_o)) {
1142+
if (PyStackRef_IsNone(v) && PyIter_Check(receiver_o)) {
11451143
retval_o = Py_TYPE(receiver_o)->tp_iternext(receiver_o);
11461144
}
11471145
else {
@@ -1249,7 +1247,7 @@ dummy_func(
12491247
inst(POP_EXCEPT, (exc_value -- )) {
12501248
_PyErr_StackItem *exc_info = tstate->exc_info;
12511249
Py_XSETREF(exc_info->exc_value,
1252-
PyStackRef_Is(exc_value, PyStackRef_None)
1250+
PyStackRef_IsNone(exc_value)
12531251
? NULL : PyStackRef_AsPyObjectSteal(exc_value));
12541252
}
12551253

@@ -2408,6 +2406,14 @@ dummy_func(
24082406
}
24092407
else {
24102408
res = PyStackRef_FromPyObjectSteal(res_o);
2409+
#ifdef Py_GIL_DISABLED
2410+
// Ensure that Py_TAG_DEFERRED is set for Py_True and Py_False
2411+
// so that ops like _POP_JUMP_IF_FALSE can use the faster
2412+
// PyStackRef_IsExactly() check.
2413+
if (_Py_IsImmortal(res_o)) {
2414+
res.bits |= Py_TAG_DEFERRED;
2415+
}
2416+
#endif
24112417
}
24122418
}
24132419

@@ -2481,13 +2487,7 @@ dummy_func(
24812487
}
24822488

24832489
inst(IS_OP, (left, right -- b)) {
2484-
#ifdef Py_GIL_DISABLED
2485-
// On free-threaded builds, objects are conditionally immortalized.
2486-
// So their bits don't always compare equally.
24872490
int res = Py_Is(PyStackRef_AsPyObjectBorrow(left), PyStackRef_AsPyObjectBorrow(right)) ^ oparg;
2488-
#else
2489-
int res = PyStackRef_Is(left, right) ^ oparg;
2490-
#endif
24912491
DECREF_INPUTS();
24922492
b = res ? PyStackRef_True : PyStackRef_False;
24932493
}
@@ -2693,22 +2693,22 @@ dummy_func(
26932693

26942694
replaced op(_POP_JUMP_IF_FALSE, (cond -- )) {
26952695
assert(PyStackRef_BoolCheck(cond));
2696-
int flag = PyStackRef_Is(cond, PyStackRef_False);
2696+
int flag = PyStackRef_IsExactly(cond, PyStackRef_False);
26972697
DEAD(cond);
26982698
RECORD_BRANCH_TAKEN(this_instr[1].cache, flag);
26992699
JUMPBY(oparg * flag);
27002700
}
27012701

27022702
replaced op(_POP_JUMP_IF_TRUE, (cond -- )) {
27032703
assert(PyStackRef_BoolCheck(cond));
2704-
int flag = PyStackRef_Is(cond, PyStackRef_True);
2704+
int flag = PyStackRef_IsExactly(cond, PyStackRef_True);
27052705
DEAD(cond);
27062706
RECORD_BRANCH_TAKEN(this_instr[1].cache, flag);
27072707
JUMPBY(oparg * flag);
27082708
}
27092709

27102710
op(_IS_NONE, (value -- b)) {
2711-
if (PyStackRef_Is(value, PyStackRef_None)) {
2711+
if (PyStackRef_IsNone(value)) {
27122712
b = PyStackRef_True;
27132713
DEAD(value);
27142714
}
@@ -3752,7 +3752,7 @@ dummy_func(
37523752

37533753
inst(EXIT_INIT_CHECK, (should_be_none -- )) {
37543754
assert(STACK_LEVEL() == 2);
3755-
if (!PyStackRef_Is(should_be_none, PyStackRef_None)) {
3755+
if (!PyStackRef_IsNone(should_be_none)) {
37563756
PyErr_Format(PyExc_TypeError,
37573757
"__init__() should return None, not '%.200s'",
37583758
Py_TYPE(PyStackRef_AsPyObjectBorrow(should_be_none))->tp_name);
@@ -4712,7 +4712,7 @@ dummy_func(
47124712
inst(INSTRUMENTED_POP_JUMP_IF_TRUE, (unused/1 -- )) {
47134713
_PyStackRef cond = POP();
47144714
assert(PyStackRef_BoolCheck(cond));
4715-
int flag = PyStackRef_Is(cond, PyStackRef_True);
4715+
int flag = PyStackRef_IsExactly(cond, PyStackRef_True);
47164716
int offset = flag * oparg;
47174717
RECORD_BRANCH_TAKEN(this_instr[1].cache, flag);
47184718
INSTRUMENTED_JUMP(this_instr, next_instr + offset, PY_MONITORING_EVENT_BRANCH);
@@ -4721,15 +4721,15 @@ dummy_func(
47214721
inst(INSTRUMENTED_POP_JUMP_IF_FALSE, (unused/1 -- )) {
47224722
_PyStackRef cond = POP();
47234723
assert(PyStackRef_BoolCheck(cond));
4724-
int flag = PyStackRef_Is(cond, PyStackRef_False);
4724+
int flag = PyStackRef_IsExactly(cond, PyStackRef_False);
47254725
int offset = flag * oparg;
47264726
RECORD_BRANCH_TAKEN(this_instr[1].cache, flag);
47274727
INSTRUMENTED_JUMP(this_instr, next_instr + offset, PY_MONITORING_EVENT_BRANCH);
47284728
}
47294729

47304730
inst(INSTRUMENTED_POP_JUMP_IF_NONE, (unused/1 -- )) {
47314731
_PyStackRef value_stackref = POP();
4732-
int flag = PyStackRef_Is(value_stackref, PyStackRef_None);
4732+
int flag = PyStackRef_IsNone(value_stackref);
47334733
int offset;
47344734
if (flag) {
47354735
offset = oparg;
@@ -4745,7 +4745,7 @@ dummy_func(
47454745
inst(INSTRUMENTED_POP_JUMP_IF_NOT_NONE, (unused/1 -- )) {
47464746
_PyStackRef value_stackref = POP();
47474747
int offset;
4748-
int nflag = PyStackRef_Is(value_stackref, PyStackRef_None);
4748+
int nflag = PyStackRef_IsNone(value_stackref);
47494749
if (nflag) {
47504750
offset = 0;
47514751
}
@@ -4780,21 +4780,21 @@ dummy_func(
47804780
///////// Tier-2 only opcodes /////////
47814781

47824782
op (_GUARD_IS_TRUE_POP, (flag -- )) {
4783-
int is_true = PyStackRef_Is(flag, PyStackRef_True);
4783+
int is_true = PyStackRef_IsTrue(flag);
47844784
DEAD(flag);
47854785
SYNC_SP();
47864786
EXIT_IF(!is_true);
47874787
}
47884788

47894789
op (_GUARD_IS_FALSE_POP, (flag -- )) {
4790-
int is_false = PyStackRef_Is(flag, PyStackRef_False);
4790+
int is_false = PyStackRef_IsFalse(flag);
47914791
DEAD(flag);
47924792
SYNC_SP();
47934793
EXIT_IF(!is_false);
47944794
}
47954795

47964796
op (_GUARD_IS_NONE_POP, (val -- )) {
4797-
int is_none = PyStackRef_Is(val, PyStackRef_None);
4797+
int is_none = PyStackRef_IsNone(val);
47984798
if (!is_none) {
47994799
PyStackRef_CLOSE(val);
48004800
SYNC_SP();
@@ -4804,7 +4804,7 @@ dummy_func(
48044804
}
48054805

48064806
op (_GUARD_IS_NOT_NONE_POP, (val -- )) {
4807-
int is_none = PyStackRef_Is(val, PyStackRef_None);
4807+
int is_none = PyStackRef_IsNone(val);
48084808
PyStackRef_CLOSE(val);
48094809
SYNC_SP();
48104810
EXIT_IF(is_none);

Python/executor_cases.c.h

+18-18
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

0 commit comments

Comments
 (0)