gh-115999: Specialize STORE_ATTR in free-threaded builds. (gh-127838)

* Add `_PyDictKeys_StringLookupSplit` which does locking on dict keys and
  use in place of `_PyDictKeys_StringLookup`.

* Change `_PyObject_TryGetInstanceAttribute` to use that function
  in the case of split keys.

* Add `unicodekeys_lookup_split` helper which allows code sharing
  between `_Py_dict_lookup` and `_PyDictKeys_StringLookupSplit`.

* Fix locking for `STORE_ATTR_INSTANCE_VALUE`.  Create
  `_GUARD_TYPE_VERSION_AND_LOCK` uop so that object stays locked and
  `tp_version_tag` cannot change.

* Pass `tp_version_tag` to `specialize_dict_access()`, ensuring
  the version we store on the cache is the correct one (in case of
  it changing during the specalize analysis).

* Split `analyze_descriptor` into `analyze_descriptor_load` and
  `analyze_descriptor_store` since those don't share much logic.
  Add `descriptor_is_class` helper function.

* In `specialize_dict_access`, double check `_PyObject_GetManagedDict()`
  in case we race and dict was materialized before the lock.

* Avoid borrowed references in `_Py_Specialize_StoreAttr()`.

* Use `specialize()` and `unspecialize()` helpers.

* Add unit tests to ensure specializing happens as expected in FT builds.

* Add unit tests to attempt to trigger data races (useful for running under TSAN).

* Add `has_split_table` function to `_testinternalcapi`.
This commit is contained in:
Neil Schemenauer 2024-12-19 10:21:17 -08:00 committed by GitHub
parent d2f1d917e8
commit 1b15c89a17
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
13 changed files with 716 additions and 297 deletions

View file

@ -1467,7 +1467,7 @@ dummy_func(
};
specializing op(_SPECIALIZE_STORE_ATTR, (counter/1, owner -- owner)) {
#if ENABLE_SPECIALIZATION
#if ENABLE_SPECIALIZATION_FT
if (ADAPTIVE_COUNTER_TRIGGERS(counter)) {
PyObject *name = GETITEM(FRAME_CO_NAMES, oparg);
next_instr = this_instr;
@ -1476,7 +1476,7 @@ dummy_func(
}
OPCODE_DEFERRED_INC(STORE_ATTR);
ADVANCE_ADAPTIVE_COUNTER(this_instr[1].counter);
#endif /* ENABLE_SPECIALIZATION */
#endif /* ENABLE_SPECIALIZATION_FT */
}
op(_STORE_ATTR, (v, owner --)) {
@ -2129,7 +2129,18 @@ dummy_func(
op(_GUARD_TYPE_VERSION, (type_version/2, owner -- owner)) {
PyTypeObject *tp = Py_TYPE(PyStackRef_AsPyObjectBorrow(owner));
assert(type_version != 0);
EXIT_IF(tp->tp_version_tag != type_version);
EXIT_IF(FT_ATOMIC_LOAD_UINT_RELAXED(tp->tp_version_tag) != type_version);
}
op(_GUARD_TYPE_VERSION_AND_LOCK, (type_version/2, owner -- owner)) {
PyObject *owner_o = PyStackRef_AsPyObjectBorrow(owner);
assert(type_version != 0);
EXIT_IF(!LOCK_OBJECT(owner_o));
PyTypeObject *tp = Py_TYPE(owner_o);
if (FT_ATOMIC_LOAD_UINT_RELAXED(tp->tp_version_tag) != type_version) {
UNLOCK_OBJECT(owner_o);
EXIT_IF(true);
}
}
op(_CHECK_MANAGED_OBJECT_HAS_VALUES, (owner -- owner)) {
@ -2336,8 +2347,11 @@ dummy_func(
assert(Py_TYPE(owner_o)->tp_dictoffset < 0);
assert(Py_TYPE(owner_o)->tp_flags & Py_TPFLAGS_INLINE_VALUES);
EXIT_IF(_PyObject_GetManagedDict(owner_o));
EXIT_IF(_PyObject_InlineValues(owner_o)->valid == 0);
if (_PyObject_GetManagedDict(owner_o) ||
!FT_ATOMIC_LOAD_UINT8(_PyObject_InlineValues(owner_o)->valid)) {
UNLOCK_OBJECT(owner_o);
EXIT_IF(true);
}
}
op(_STORE_ATTR_INSTANCE_VALUE, (offset/1, value, owner --)) {
@ -2347,21 +2361,20 @@ dummy_func(
assert(_PyObject_GetManagedDict(owner_o) == NULL);
PyObject **value_ptr = (PyObject**)(((char *)owner_o) + offset);
PyObject *old_value = *value_ptr;
*value_ptr = PyStackRef_AsPyObjectSteal(value);
FT_ATOMIC_STORE_PTR_RELEASE(*value_ptr, PyStackRef_AsPyObjectSteal(value));
if (old_value == NULL) {
PyDictValues *values = _PyObject_InlineValues(owner_o);
Py_ssize_t index = value_ptr - values->values;
_PyDictValues_AddToInsertionOrder(values, index);
}
else {
Py_DECREF(old_value);
}
UNLOCK_OBJECT(owner_o);
Py_XDECREF(old_value);
PyStackRef_CLOSE(owner);
}
macro(STORE_ATTR_INSTANCE_VALUE) =
unused/1 +
_GUARD_TYPE_VERSION +
_GUARD_TYPE_VERSION_AND_LOCK +
_GUARD_DORV_NO_DICT +
_STORE_ATTR_INSTANCE_VALUE;
@ -2370,16 +2383,34 @@ dummy_func(
assert(Py_TYPE(owner_o)->tp_flags & Py_TPFLAGS_MANAGED_DICT);
PyDictObject *dict = _PyObject_GetManagedDict(owner_o);
DEOPT_IF(dict == NULL);
DEOPT_IF(!LOCK_OBJECT(dict));
#ifdef Py_GIL_DISABLED
if (dict != _PyObject_GetManagedDict(owner_o)) {
UNLOCK_OBJECT(dict);
DEOPT_IF(true);
}
#endif
assert(PyDict_CheckExact((PyObject *)dict));
PyObject *name = GETITEM(FRAME_CO_NAMES, oparg);
DEOPT_IF(hint >= (size_t)dict->ma_keys->dk_nentries);
DEOPT_IF(!DK_IS_UNICODE(dict->ma_keys));
if (hint >= (size_t)dict->ma_keys->dk_nentries ||
!DK_IS_UNICODE(dict->ma_keys)) {
UNLOCK_OBJECT(dict);
DEOPT_IF(true);
}
PyDictUnicodeEntry *ep = DK_UNICODE_ENTRIES(dict->ma_keys) + hint;
DEOPT_IF(ep->me_key != name);
if (ep->me_key != name) {
UNLOCK_OBJECT(dict);
DEOPT_IF(true);
}
PyObject *old_value = ep->me_value;
DEOPT_IF(old_value == NULL);
if (old_value == NULL) {
UNLOCK_OBJECT(dict);
DEOPT_IF(true);
}
_PyDict_NotifyEvent(tstate->interp, PyDict_EVENT_MODIFIED, dict, name, PyStackRef_AsPyObjectBorrow(value));
ep->me_value = PyStackRef_AsPyObjectSteal(value);
FT_ATOMIC_STORE_PTR_RELEASE(ep->me_value, PyStackRef_AsPyObjectSteal(value));
UNLOCK_OBJECT(dict);
// old_value should be DECREFed after GC track checking is done, if not, it could raise a segmentation fault,
// when dict only holds the strong reference to value in ep->me_value.
Py_XDECREF(old_value);
@ -2395,10 +2426,12 @@ dummy_func(
op(_STORE_ATTR_SLOT, (index/1, value, owner --)) {
PyObject *owner_o = PyStackRef_AsPyObjectBorrow(owner);
DEOPT_IF(!LOCK_OBJECT(owner_o));
char *addr = (char *)owner_o + index;
STAT_INC(STORE_ATTR, hit);
PyObject *old_value = *(PyObject **)addr;
*(PyObject **)addr = PyStackRef_AsPyObjectSteal(value);
FT_ATOMIC_STORE_PTR_RELEASE(*(PyObject **)addr, PyStackRef_AsPyObjectSteal(value));
UNLOCK_OBJECT(owner_o);
Py_XDECREF(old_value);
PyStackRef_CLOSE(owner);
}