Skip to content

bpo-45947: Place dict and values pointer at fixed (negative) offset just before GC header. #29879

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
markshannon merged 21 commits into from
Dec 7, 2021
Merged

bpo-45947: Place dict and values pointer at fixed (negative) offset just before GC header. #29879

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
21 commits
Select commit Hold shift + click to select a range
ee59772
Remove unsafe _PyObject_GC_Calloc function.
markshannon Aug 5, 2021
72b71cc
Place __dict__ immediately before GC header for variable sized object…
markshannon Aug 5, 2021
cd22dac
Restore documented behavior of tp_dictoffset.
markshannon Nov 29, 2021
4c83f77
Merge branch 'main' into regular-dict-placement
markshannon Nov 29, 2021
8a8593c
Fix up lazy dict creation logic to use managed dict pointers.
markshannon Nov 29, 2021
34b5cea
Manage values pointer, placing them directly before managed dict poin…
markshannon Nov 30, 2021
e8c74ab
Refactor a bit.
markshannon Nov 30, 2021
1bf13b0
Fix specialization of managed values.
markshannon Nov 30, 2021
a025dfb
Convert hint-based load/store attr specialization target managed dict…
markshannon Nov 30, 2021
5a012a8
Specialize LOAD_METHOD for managed dict objects.
markshannon Nov 30, 2021
e7734b8
Merge branch 'main' into regular-dict-placement
markshannon Dec 1, 2021
14d41ab
Use newer API internally.
markshannon Dec 1, 2021
123171a
Add NEWS.
markshannon Dec 1, 2021
48d6a58
Use inline functions instead of magic constants.
markshannon Dec 1, 2021
79e61bf
Remove unsafe _PyObject_GC_Malloc() function.
markshannon Dec 1, 2021
ce0f65b
Remove invalid assert.
markshannon Dec 2, 2021
98ddaed
Add comment explaning use of Py_TPFLAGS_MANAGED_DICT.
markshannon Dec 3, 2021
0f376b5
Use inline function, not magic constant.
markshannon Dec 7, 2021
d724812
Tidy up struct layout a bit.
markshannon Dec 7, 2021
9435bae
Tidy up gdb/libpython.py.
markshannon Dec 7, 2021
302f46f
Fix whitespace.
markshannon Dec 7, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 4 additions & 5 deletions Python/ceval.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3546,8 +3546,8 @@ _PyEval_EvalFrameDefault(PyThreadState *tstate, InterpreterFrame *frame, int thr
_PyAttrCache *cache1 = &caches[-1].attr;
assert(cache1->tp_version != 0);
DEOPT_IF(tp->tp_version_tag != cache1->tp_version, LOAD_ATTR);
assert(tp->tp_dictoffset > 0);
PyDictObject *dict = *(PyDictObject **)(((char *)owner) + tp->tp_dictoffset);
assert(tp->tp_flags & Py_TPFLAGS_MANAGED_DICT);
PyDictObject *dict = *(PyDictObject **)_PyObject_ManagedDictPointer(owner);
DEOPT_IF(dict == NULL, LOAD_ATTR);
assert(PyDict_CheckExact((PyObject *)dict));
PyObject *name = GETITEM(names, cache0->original_oparg);
Expand Down Expand Up @@ -3614,7 +3614,6 @@ _PyEval_EvalFrameDefault(PyThreadState *tstate, InterpreterFrame *frame, int thr
_PyAttrCache *cache1 = &caches[-1].attr;
assert(cache1->tp_version != 0);
DEOPT_IF(tp->tp_version_tag != cache1->tp_version, STORE_ATTR);
assert(tp->tp_dictoffset < 0);
assert(tp->tp_flags & Py_TPFLAGS_MANAGED_DICT);
PyDictValues *values = ((PyDictValues **)owner)[-4];
DEOPT_IF(values == NULL, STORE_ATTR);
Expand Down Expand Up @@ -3644,8 +3643,8 @@ _PyEval_EvalFrameDefault(PyThreadState *tstate, InterpreterFrame *frame, int thr
_PyAttrCache *cache1 = &caches[-1].attr;
assert(cache1->tp_version != 0);
DEOPT_IF(tp->tp_version_tag != cache1->tp_version, STORE_ATTR);
assert(tp->tp_dictoffset > 0);
PyDictObject *dict = *(PyDictObject **)(((char *)owner) + tp->tp_dictoffset);
assert(tp->tp_flags & Py_TPFLAGS_MANAGED_DICT);
PyDictObject *dict = *(PyDictObject **)_PyObject_ManagedDictPointer(owner);
DEOPT_IF(dict == NULL, STORE_ATTR);
assert(PyDict_CheckExact((PyObject *)dict));
PyObject *name = GETITEM(names, cache0->original_oparg);
Expand Down
40 changes: 11 additions & 29 deletions Python/specialize.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -447,6 +447,7 @@ initial_counter_value(void) {
#define SPEC_FAIL_NON_OBJECT_SLOT 14
#define SPEC_FAIL_READ_ONLY 15
#define SPEC_FAIL_AUDITED_SLOT 16
#define SPEC_FAIL_NOT_MANAGED_DICT 17

/* Methods */

Expand Down Expand Up @@ -626,7 +627,13 @@ specialize_dict_access(
assert(kind == NON_OVERRIDING || kind == NON_DESCRIPTOR || kind == ABSENT ||
kind == BUILTIN_CLASSMETHOD || kind == PYTHON_CLASSMETHOD);
// No descriptor, or non overriding.
if (type->tp_flags & Py_TPFLAGS_MANAGED_DICT) {
if ((type->tp_flags & Py_TPFLAGS_MANAGED_DICT) == 0) {
SPECIALIZATION_FAIL(base_op, SPEC_FAIL_NOT_MANAGED_DICT);
return 0;
}
PyObject **dictptr = _PyObject_ManagedDictPointer(owner);
PyDictObject *dict = (PyDictObject *)*dictptr;
if (dict == NULL) {
// Virtual dictionary
PyDictKeysObject *keys = ((PyHeapTypeObject *)type)->ht_cached_keys;
assert(PyUnicode_CheckExact(name));
Expand All @@ -639,16 +646,9 @@ specialize_dict_access(
cache1->tp_version = type->tp_version_tag;
cache0->index = (uint16_t)index;
*instr = _Py_MAKECODEUNIT(values_op, _Py_OPARG(*instr));
return 0;
}
if (type->tp_dictoffset < 0) {
SPECIALIZATION_FAIL(base_op, SPEC_FAIL_OUT_OF_RANGE);
return 0;
}
if (type->tp_dictoffset > 0) {
PyObject **dictptr = (PyObject **) ((char *)owner + type->tp_dictoffset);
PyDictObject *dict = (PyDictObject *)*dictptr;
if (dict == NULL || !PyDict_CheckExact(dict)) {
else {
if (!PyDict_CheckExact(dict)) {
Comment on lines +650 to +651
Copy link
Member

@gvanrossum gvanrossum Dec 6, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm guessing that if we have a class of objects and only some of those have a materialized __dict__ that will wreak havoc with our specialization, right? Because it will flop-flop as it encounters instances with and without __dict__. In this case we have to hope that the vast majority don't have a __dict__.

Copy link
Member Author

@markshannon markshannon Dec 7, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes.
The specialized case is so much faster though, that it might not be any worse than not specializing at all, even in that case.

SPECIALIZATION_FAIL(base_op, SPEC_FAIL_NO_DICT);
return 0;
}
Expand All @@ -663,26 +663,8 @@ specialize_dict_access(
cache1->dk_version_or_hint = (uint32_t)hint;
cache1->tp_version = type->tp_version_tag;
*instr = _Py_MAKECODEUNIT(hint_op, _Py_OPARG(*instr));
return 1;
}
assert(type->tp_dictoffset == 0);
/* No attribute in instance dictionary */
switch(kind) {
case NON_OVERRIDING:
case BUILTIN_CLASSMETHOD:
case PYTHON_CLASSMETHOD:
SPECIALIZATION_FAIL(base_op, SPEC_FAIL_NON_OVERRIDING_DESCRIPTOR);
return 0;
case NON_DESCRIPTOR:
/* To do -- Optimize this case */
SPECIALIZATION_FAIL(base_op, SPEC_FAIL_NOT_DESCRIPTOR);
return 0;
case ABSENT:
SPECIALIZATION_FAIL(base_op, SPEC_FAIL_EXPECTED_ERROR);
return 0;
default:
Py_UNREACHABLE();
}
return 1;
}

int
Expand Down