Skip to content

Commit

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

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.