Skip to content

GH-95245: Store object values and dict pointers in single tagged pointer. #95278

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 6 commits into from
Aug 1, 2022
Merged

GH-95245: Store object values and dict pointers in single tagged pointer. #95278

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
ffde281
Store object values and dict pointers in single tagged pointer.
markshannon Jul 21, 2022
48563b3
Merge branch 'main' into merge-dict-and-values-pointers
markshannon Jul 26, 2022
9fdad79
Fixup merge by deduplicating visit/clear functions and moving declara…
markshannon Jul 26, 2022
da9b388
Remove some pointless inline functions and better inforce invariants.
markshannon Jul 26, 2022
0d06096
Add news
markshannon Jul 26, 2022
562c103
Implement kumaraditya303 suggestions.
markshannon Jul 28, 2022
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
3 changes: 0 additions & 3 deletions Include/cpython/dictobject.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -83,6 +83,3 @@ typedef struct {

PyAPI_FUNC(PyObject *) _PyDictView_New(PyObject *, PyTypeObject *);
PyAPI_FUNC(PyObject *) _PyDictView_Intersect(PyObject* self, PyObject *other);

PyAPI_FUNC(int) _PyObject_VisitManagedDict(PyObject *self, visitproc visit, void *arg);
PyAPI_FUNC(void) _PyObject_ClearManagedDict(PyObject *self);
4 changes: 4 additions & 0 deletions Include/cpython/object.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -509,3 +509,7 @@ Py_DEPRECATED(3.11) typedef int UsingDeprecatedTrashcanMacro;
#define Py_TRASHCAN_SAFE_END(op) \
Py_TRASHCAN_END; \
} while(0);


PyAPI_FUNC(int) _PyObject_VisitManagedDict(PyObject *obj, visitproc visit, void *arg);
PyAPI_FUNC(void) _PyObject_ClearManagedDict(PyObject *obj);
43 changes: 34 additions & 9 deletions Include/internal/pycore_object.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -268,24 +268,49 @@ extern int _PyObject_StoreInstanceAttribute(PyObject *obj, PyDictValues *values,
PyObject * _PyObject_GetInstanceAttribute(PyObject *obj, PyDictValues *values,
PyObject *name);

static inline PyDictValues **_PyObject_ValuesPointer(PyObject *obj)
typedef union {
PyObject *dict;
/* Use a char* to generate a warning if directly assigning a PyDictValues */
char *values;
} PyDictOrValues;

static inline PyDictOrValues *
_PyObject_DictOrValuesPointer(PyObject *obj)
{
assert(Py_TYPE(obj)->tp_flags & Py_TPFLAGS_MANAGED_DICT);
return ((PyDictValues **)obj)-4;
return ((PyDictOrValues *)obj)-3;
}

static inline PyObject **_PyObject_ManagedDictPointer(PyObject *obj)
static inline int
_PyDictOrValues_IsValues(PyDictOrValues dorv)
{
assert(Py_TYPE(obj)->tp_flags & Py_TPFLAGS_MANAGED_DICT);
return ((PyObject **)obj)-3;
return ((uintptr_t)dorv.values) & 1;
}

static inline PyDictValues *
_PyDictOrValues_GetValues(PyDictOrValues dorv)
{
assert(_PyDictOrValues_IsValues(dorv));
return (PyDictValues *)(dorv.values + 1);
}

static inline PyObject *
_PyDictOrValues_GetDict(PyDictOrValues dorv)
{
assert(!_PyDictOrValues_IsValues(dorv));
return dorv.dict;
}

static inline void
_PyDictOrValues_SetValues(PyDictOrValues *ptr, PyDictValues *values)
{
ptr->values = ((char *)values) - 1;
Copy link
Contributor

@matthiasgoergens matthiasgoergens Aug 26, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

I'm curious why this one is a -1? Naively, I would have expected something like (|1), ie setting the lowest bit. Which would be equivalent to +1.

I assume there's something that goes horribly wrong with that native approach that I am missing, though?

(As far as I can tell, subtracting one should also work. Not sure what C does with null pointers in this context, though.)

Copy link
Contributor

@erlend-aasland erlend-aasland Aug 26, 2022

Choose a reason for hiding this comment

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

_PyDictOrValues_SetValues mutates the pointer; _PyDictOrValues_GetValues restores the pointer. Think of it as $f(x) = y$ and $f^{-1}(y) = x$. Currently, $f(x) = x - 1$ and $f^{-1}(y) = y + 1$. If we'd used +1, as you suggest, we'd have to undo that using -1 iso. +1. AFAICS, either way is fine.

Copy link
Contributor

@matthiasgoergens matthiasgoergens Aug 26, 2022

Choose a reason for hiding this comment

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

Thanks for explaining!

I am working on a prototype with a tagged pointer elsewhere in the code, I so I was looking for exiting examples to learn from.

Thanks for confirming that either way is fine.

I was just wrecking my brain about whether there's something more going on that I was missing. (And something that I would thus be bound to get horriby wrong in my code, where I want to store either a pointer directly, or an odd integer, in the same memory location.)

}

#define MANAGED_DICT_OFFSET (((int)sizeof(PyObject *))*-3)

extern PyObject ** _PyObject_DictPointer(PyObject *);
extern int _PyObject_VisitInstanceAttributes(PyObject *self, visitproc visit, void *arg);
extern void _PyObject_ClearInstanceAttributes(PyObject *self);
extern void _PyObject_FreeInstanceAttributes(PyObject *self);
extern PyObject ** _PyObject_ComputedDictPointer(PyObject *);
extern void _PyObject_FreeInstanceAttributes(PyObject *obj);
extern int _PyObject_IsInstanceDictEmpty(PyObject *);
extern PyObject* _PyType_GetSubclasses(PyTypeObject *);

Expand Down
2 changes: 2 additions & 0 deletions Misc/NEWS.d/next/Core and Builtins/2022-07-26-12-59-03.gh-issue-95245.GHWczn.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
Merge managed dict and values pointer into a single tagged pointer to save
one word in the pre-header.
161 changes: 90 additions & 71 deletions Objects/dictobject.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5368,7 +5368,7 @@ init_inline_values(PyObject *obj, PyTypeObject *tp)
for (int i = 0; i < size; i++) {
values->values[i] = NULL;
}
*_PyObject_ValuesPointer(obj) = values;
_PyDictOrValues_SetValues(_PyObject_DictOrValuesPointer(obj), values);
return 0;
}

Expand All @@ -5394,7 +5394,7 @@ _PyObject_InitializeDict(PyObject *obj)
if (dict == NULL) {
return -1;
}
PyObject **dictptr = _PyObject_DictPointer(obj);
PyObject **dictptr = _PyObject_ComputedDictPointer(obj);
*dictptr = dict;
return 0;
}
Expand Down Expand Up @@ -5422,7 +5422,6 @@ make_dict_from_instance_attributes(PyDictKeysObject *keys, PyDictValues *values)
PyObject *
_PyObject_MakeDictFromInstanceAttributes(PyObject *obj, PyDictValues *values)
{
assert(Py_TYPE(obj)->tp_flags & Py_TPFLAGS_MANAGED_DICT);
PyDictKeysObject *keys = CACHED_KEYS(Py_TYPE(obj));
OBJECT_STAT_INC(dict_materialized_on_request);
return make_dict_from_instance_attributes(keys, values);
Expand Down Expand Up @@ -5458,8 +5457,7 @@ _PyObject_StoreInstanceAttribute(PyObject *obj, PyDictValues *values,
if (dict == NULL) {
return -1;
}
*_PyObject_ValuesPointer(obj) = NULL;
*_PyObject_ManagedDictPointer(obj) = dict;
_PyObject_DictOrValuesPointer(obj)->dict = dict;
if (value == NULL) {
return PyDict_DelItem(dict, name);
}
Expand Down Expand Up @@ -5488,6 +5486,37 @@ _PyObject_StoreInstanceAttribute(PyObject *obj, PyDictValues *values,
return 0;
}

/* Sanity check for managed dicts */
#if 0
#define CHECK(val) assert(val); if (!(val)) { return 0; }

int
_PyObject_ManagedDictValidityCheck(PyObject *obj)
{
PyTypeObject *tp = Py_TYPE(obj);
CHECK(tp->tp_flags & Py_TPFLAGS_MANAGED_DICT);
PyDictOrValues *dorv_ptr = _PyObject_DictOrValuesPointer(obj);
if (_PyDictOrValues_IsValues(*dorv_ptr)) {
PyDictValues *values = _PyDictOrValues_GetValues(*dorv_ptr);
int size = ((uint8_t *)values)[-2];
int count = 0;
PyDictKeysObject *keys = CACHED_KEYS(tp);
for (Py_ssize_t i = 0; i < keys->dk_nentries; i++) {
if (values->values[i] != NULL) {
count++;
}
}
CHECK(size == count);
}
else {
if (dorv_ptr->dict != NULL) {
CHECK(PyDict_Check(dorv_ptr->dict));
}
}
return 1;
}
#endif

PyObject *
_PyObject_GetInstanceAttribute(PyObject *obj, PyDictValues *values,
PyObject *name)
Expand All @@ -5511,105 +5540,94 @@ _PyObject_IsInstanceDictEmpty(PyObject *obj)
if (tp->tp_dictoffset == 0) {
return 1;
}
PyObject **dictptr;
PyObject *dict;
if (tp->tp_flags & Py_TPFLAGS_MANAGED_DICT) {
PyDictValues *values = *_PyObject_ValuesPointer(obj);
if (values) {
PyDictOrValues dorv = *_PyObject_DictOrValuesPointer(obj);
if (_PyDictOrValues_IsValues(dorv)) {
PyDictKeysObject *keys = CACHED_KEYS(tp);
for (Py_ssize_t i = 0; i < keys->dk_nentries; i++) {
if (values->values[i] != NULL) {
if (_PyDictOrValues_GetValues(dorv)->values[i] != NULL) {
return 0;
}
}
return 1;
}
dictptr = _PyObject_ManagedDictPointer(obj);
dict = _PyDictOrValues_GetDict(dorv);
}
else {
dictptr = _PyObject_DictPointer(obj);
PyObject **dictptr = _PyObject_ComputedDictPointer(obj);
dict = *dictptr;
}
PyObject *dict = *dictptr;
if (dict == NULL) {
return 1;
}
return ((PyDictObject *)dict)->ma_used == 0;
}


int
_PyObject_VisitInstanceAttributes(PyObject *self, visitproc visit, void *arg)
{
PyTypeObject *tp = Py_TYPE(self);
assert(Py_TYPE(self)->tp_flags & Py_TPFLAGS_MANAGED_DICT);
PyDictValues **values_ptr = _PyObject_ValuesPointer(self);
if (*values_ptr == NULL) {
return 0;
}
PyDictKeysObject *keys = CACHED_KEYS(tp);
for (Py_ssize_t i = 0; i < keys->dk_nentries; i++) {
Py_VISIT((*values_ptr)->values[i]);
}
return 0;
}

void
_PyObject_ClearInstanceAttributes(PyObject *self)
{
PyTypeObject *tp = Py_TYPE(self);
assert(Py_TYPE(self)->tp_flags & Py_TPFLAGS_MANAGED_DICT);
PyDictValues **values_ptr = _PyObject_ValuesPointer(self);
if (*values_ptr == NULL) {
return;
}
PyDictKeysObject *keys = CACHED_KEYS(tp);
for (Py_ssize_t i = 0; i < keys->dk_nentries; i++) {
Py_CLEAR((*values_ptr)->values[i]);
}
}

void
_PyObject_FreeInstanceAttributes(PyObject *self)
{
PyTypeObject *tp = Py_TYPE(self);
assert(Py_TYPE(self)->tp_flags & Py_TPFLAGS_MANAGED_DICT);
PyDictValues **values_ptr = _PyObject_ValuesPointer(self);
if (*values_ptr == NULL) {
PyDictOrValues dorv = *_PyObject_DictOrValuesPointer(self);
if (!_PyDictOrValues_IsValues(dorv)) {
return;
}
PyDictValues *values = _PyDictOrValues_GetValues(dorv);
PyDictKeysObject *keys = CACHED_KEYS(tp);
for (Py_ssize_t i = 0; i < keys->dk_nentries; i++) {
Py_XDECREF((*values_ptr)->values[i]);
Py_XDECREF(values->values[i]);
}
free_values(*values_ptr);
free_values(values);
}

int
_PyObject_VisitManagedDict(PyObject *self, visitproc visit, void *arg)
_PyObject_VisitManagedDict(PyObject *obj, visitproc visit, void *arg)
{
PyTypeObject *tp = Py_TYPE(self);
PyTypeObject *tp = Py_TYPE(obj);
if((tp->tp_flags & Py_TPFLAGS_MANAGED_DICT) == 0) {
return 0;
}
assert(tp->tp_dictoffset);
int err = _PyObject_VisitInstanceAttributes(self, visit, arg);
if (err) {
return err;
PyDictOrValues dorv = *_PyObject_DictOrValuesPointer(obj);
if (_PyDictOrValues_IsValues(dorv)) {
PyDictValues *values = _PyDictOrValues_GetValues(dorv);
PyDictKeysObject *keys = CACHED_KEYS(tp);
for (Py_ssize_t i = 0; i < keys->dk_nentries; i++) {
Py_VISIT(values->values[i]);
}
}
else {
PyObject *dict = _PyDictOrValues_GetDict(dorv);
Py_VISIT(dict);
}
Py_VISIT(*_PyObject_ManagedDictPointer(self));
return 0;
}


void
_PyObject_ClearManagedDict(PyObject *self)
_PyObject_ClearManagedDict(PyObject *obj)
{
PyTypeObject *tp = Py_TYPE(self);
PyTypeObject *tp = Py_TYPE(obj);
if((tp->tp_flags & Py_TPFLAGS_MANAGED_DICT) == 0) {
return;
}
_PyObject_FreeInstanceAttributes(self);
*_PyObject_ValuesPointer(self) = NULL;
Py_CLEAR(*_PyObject_ManagedDictPointer(self));
PyDictOrValues *dorv_ptr = _PyObject_DictOrValuesPointer(obj);
if (_PyDictOrValues_IsValues(*dorv_ptr)) {
PyDictValues *values = _PyDictOrValues_GetValues(*dorv_ptr);
PyDictKeysObject *keys = CACHED_KEYS(tp);
for (Py_ssize_t i = 0; i < keys->dk_nentries; i++) {
Py_CLEAR(values->values[i]);
}
dorv_ptr->dict = NULL;
free_values(values);
}
else {
PyObject *dict = dorv_ptr->dict;
if (dict) {
dorv_ptr->dict = NULL;
Py_DECREF(dict);
}
}
}

PyObject *
Expand All @@ -5618,25 +5636,26 @@ PyObject_GenericGetDict(PyObject *obj, void *context)
PyObject *dict;
PyTypeObject *tp = Py_TYPE(obj);
if (_PyType_HasFeature(tp, Py_TPFLAGS_MANAGED_DICT)) {
PyDictValues **values_ptr = _PyObject_ValuesPointer(obj);
PyObject **dictptr = _PyObject_ManagedDictPointer(obj);
if (*values_ptr) {
assert(*dictptr == NULL);
PyDictOrValues *dorv_ptr = _PyObject_DictOrValuesPointer(obj);
if (_PyDictOrValues_IsValues(*dorv_ptr)) {
PyDictValues *values = _PyDictOrValues_GetValues(*dorv_ptr);
OBJECT_STAT_INC(dict_materialized_on_request);
*dictptr = dict = make_dict_from_instance_attributes(CACHED_KEYS(tp), *values_ptr);
dict = make_dict_from_instance_attributes(CACHED_KEYS(tp), values);
if (dict != NULL) {
*values_ptr = NULL;
dorv_ptr->dict = dict;
}
}
else if (*dictptr == NULL) {
*dictptr = dict = PyDict_New();
}
else {
dict = *dictptr;
dict = _PyDictOrValues_GetDict(*dorv_ptr);
if (dict == NULL) {
dictkeys_incref(CACHED_KEYS(tp));
dict = new_dict_with_shared_keys(CACHED_KEYS(tp));
dorv_ptr->dict = dict;
}
}
}
else {
PyObject **dictptr = _PyObject_DictPointer(obj);
PyObject **dictptr = _PyObject_ComputedDictPointer(obj);
if (dictptr == NULL) {
PyErr_SetString(PyExc_AttributeError,
"This object has no __dict__");
Expand Down
Loading