Skip to content

Commit

Permalink
gh-115999: Specialize LOAD_SUPER_ATTR in free-threaded builds (gh-1…
Browse files Browse the repository at this point in the history
…27128)

Use existing helpers to atomically modify the bytecode.  Add unit tests
to ensure specializing is happening as expected.  Add test_specialize.py
that can be used with ThreadSanitizer to detect data races.  
Fix thread safety issue with cell_set_contents().
  • Loading branch information
nascheme authored Dec 3, 2024
1 parent 8ba9f5b commit 0cb5222
Show file tree
Hide file tree
Showing 6 changed files with 51 additions and 21 deletions.
39 changes: 39 additions & 0 deletions Lib/test/test_opcache.py
Original file line number Diff line number Diff line change
Expand Up @@ -1249,6 +1249,45 @@ def binary_op_add_unicode():
self.assert_specialized(binary_op_add_unicode, "BINARY_OP_ADD_UNICODE")
self.assert_no_opcode(binary_op_add_unicode, "BINARY_OP")

@cpython_only
@requires_specialization_ft
def test_load_super_attr(self):
"""Ensure that LOAD_SUPER_ATTR is specialized as expected."""

class A:
def __init__(self):
meth = super().__init__
super().__init__()

for _ in range(100):
A()

self.assert_specialized(A.__init__, "LOAD_SUPER_ATTR_ATTR")
self.assert_specialized(A.__init__, "LOAD_SUPER_ATTR_METHOD")
self.assert_no_opcode(A.__init__, "LOAD_SUPER_ATTR")

# Temporarily replace super() with something else.
real_super = super

def fake_super():
def init(self):
pass

return init

# Force unspecialize
globals()['super'] = fake_super
try:
# Should be unspecialized after enough calls.
for _ in range(100):
A()
finally:
globals()['super'] = real_super

# Ensure the specialized instructions are not present
self.assert_no_opcode(A.__init__, "LOAD_SUPER_ATTR_ATTR")
self.assert_no_opcode(A.__init__, "LOAD_SUPER_ATTR_METHOD")

@cpython_only
@requires_specialization_ft
def test_contain_op(self):
Expand Down
5 changes: 3 additions & 2 deletions Objects/cellobject.c
Original file line number Diff line number Diff line change
Expand Up @@ -145,8 +145,9 @@ cell_get_contents(PyObject *self, void *closure)
static int
cell_set_contents(PyObject *self, PyObject *obj, void *Py_UNUSED(ignored))
{
PyCellObject *op = _PyCell_CAST(self);
Py_XSETREF(op->ob_ref, Py_XNewRef(obj));
PyCellObject *cell = _PyCell_CAST(self);
Py_XINCREF(obj);
PyCell_SetTakeRef((PyCellObject *)cell, obj);
return 0;
}

Expand Down
4 changes: 2 additions & 2 deletions Python/bytecodes.c
Original file line number Diff line number Diff line change
Expand Up @@ -1946,7 +1946,7 @@ dummy_func(
};

specializing op(_SPECIALIZE_LOAD_SUPER_ATTR, (counter/1, global_super_st, class_st, unused -- global_super_st, class_st, unused)) {
#if ENABLE_SPECIALIZATION
#if ENABLE_SPECIALIZATION_FT
int load_method = oparg & 1;
if (ADAPTIVE_COUNTER_TRIGGERS(counter)) {
next_instr = this_instr;
Expand All @@ -1955,7 +1955,7 @@ dummy_func(
}
OPCODE_DEFERRED_INC(LOAD_SUPER_ATTR);
ADVANCE_ADAPTIVE_COUNTER(this_instr[1].counter);
#endif /* ENABLE_SPECIALIZATION */
#endif /* ENABLE_SPECIALIZATION_FT */
}

tier1 op(_LOAD_SUPER_ATTR, (global_super_st, class_st, self_st -- attr, null if (oparg & 1))) {
Expand Down
1 change: 0 additions & 1 deletion Python/ceval.c
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,6 @@
#include "pycore_setobject.h" // _PySet_Update()
#include "pycore_sliceobject.h" // _PyBuildSlice_ConsumeRefs
#include "pycore_tuple.h" // _PyTuple_ITEMS()
#include "pycore_typeobject.h" // _PySuper_Lookup()
#include "pycore_uop_ids.h" // Uops
#include "pycore_pyerrors.h"

Expand Down
4 changes: 2 additions & 2 deletions Python/generated_cases.c.h

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

19 changes: 5 additions & 14 deletions Python/specialize.c
Original file line number Diff line number Diff line change
Expand Up @@ -794,9 +794,8 @@ _Py_Specialize_LoadSuperAttr(_PyStackRef global_super_st, _PyStackRef cls_st, _P
PyObject *global_super = PyStackRef_AsPyObjectBorrow(global_super_st);
PyObject *cls = PyStackRef_AsPyObjectBorrow(cls_st);

assert(ENABLE_SPECIALIZATION);
assert(ENABLE_SPECIALIZATION_FT);
assert(_PyOpcode_Caches[LOAD_SUPER_ATTR] == INLINE_CACHE_ENTRIES_LOAD_SUPER_ATTR);
_PySuperAttrCache *cache = (_PySuperAttrCache *)(instr + 1);
if (global_super != (PyObject *)&PySuper_Type) {
SPECIALIZATION_FAIL(LOAD_SUPER_ATTR, SPEC_FAIL_SUPER_SHADOWED);
goto fail;
Expand All @@ -805,19 +804,11 @@ _Py_Specialize_LoadSuperAttr(_PyStackRef global_super_st, _PyStackRef cls_st, _P
SPECIALIZATION_FAIL(LOAD_SUPER_ATTR, SPEC_FAIL_SUPER_BAD_CLASS);
goto fail;
}
instr->op.code = load_method ? LOAD_SUPER_ATTR_METHOD : LOAD_SUPER_ATTR_ATTR;
goto success;

fail:
STAT_INC(LOAD_SUPER_ATTR, failure);
assert(!PyErr_Occurred());
instr->op.code = LOAD_SUPER_ATTR;
cache->counter = adaptive_counter_backoff(cache->counter);
uint8_t load_code = load_method ? LOAD_SUPER_ATTR_METHOD : LOAD_SUPER_ATTR_ATTR;
specialize(instr, load_code);
return;
success:
STAT_INC(LOAD_SUPER_ATTR, success);
assert(!PyErr_Occurred());
cache->counter = adaptive_counter_cooldown();
fail:
unspecialize(instr);
}

typedef enum {
Expand Down

0 comments on commit 0cb5222

Please sign in to comment.