From faac2d8b780b482eca8503ba5afcb4139bfd2d7a Mon Sep 17 00:00:00 2001 From: Joshua Haberman Date: Thu, 30 Dec 2021 01:04:49 -0800 Subject: [PATCH] Updated some terminology and added comments. --- python/map.c | 47 +++++++++++++++++++++++++---------------------- python/map.h | 21 +++++++++++++++++---- python/message.c | 23 +++++++++++------------ python/repeated.c | 6 +++--- python/repeated.h | 12 +++++------- 5 files changed, 61 insertions(+), 48 deletions(-) diff --git a/python/map.c b/python/map.c index aed6ab5b90..8e5ac24b29 100644 --- a/python/map.c +++ b/python/map.c @@ -38,11 +38,15 @@ typedef struct { PyObject_HEAD PyObject* arena; - uintptr_t field; // upb_fielddef*, low bit 1 == unset + // The field descriptor (upb_fielddef*). + // The low bit indicates whether the container is reified (see ptr below). + // - low bit set: repeated field is a stub (no underlying data). + // - low bit clear: repeated field is reified (points to upb_array). + uintptr_t field; union { - upb_map* map; // when set, the data for this array. - PyObject* parent; // when unset owning pointer to parent message. - }; + PyObject* parent; // stub: owning pointer to parent message. + upb_map* map; // reified: the data for this array. + } ptr; int version; } PyUpb_MapContainer; @@ -53,7 +57,7 @@ static bool PyUpb_MapContainer_IsUnset(PyUpb_MapContainer* self) { } static upb_map* PyUpb_MapContainer_GetIfWritable(PyUpb_MapContainer* self) { - return PyUpb_MapContainer_IsUnset(self) ? NULL : self->map; + return PyUpb_MapContainer_IsUnset(self) ? NULL : self->ptr.map; } static const upb_fielddef* PyUpb_MapContainer_GetField( @@ -65,10 +69,11 @@ static void PyUpb_MapContainer_Dealloc(void* _self) { PyUpb_MapContainer* self = _self; Py_DECREF(self->arena); if (PyUpb_MapContainer_IsUnset(self)) { - PyUpb_CMessage_CacheDelete(self->parent, PyUpb_MapContainer_GetField(self)); - Py_DECREF(self->parent); + PyUpb_CMessage_CacheDelete(self->ptr.parent, + PyUpb_MapContainer_GetField(self)); + Py_DECREF(self->ptr.parent); } else { - PyUpb_ObjCache_Delete(self->map); + PyUpb_ObjCache_Delete(self->ptr.map); } PyUpb_Dealloc(_self); } @@ -80,26 +85,24 @@ PyTypeObject* PyUpb_MapContainer_GetClass(const upb_fielddef* f) { : state->scalar_map_container_type; } -PyObject* PyUpb_MapContainer_NewUnset(PyObject* parent, const upb_fielddef* f, - PyObject* arena) { +PyObject* PyUpb_MapContainer_NewStub(PyObject* parent, const upb_fielddef* f, + PyObject* arena) { PyTypeObject* cls = PyUpb_MapContainer_GetClass(f); - // We are GC because of the MutableMapping base class. Ideally we could - // implement them ourselves so we don't need a base so we don't need to be GC. PyUpb_MapContainer* map = (void*)PyType_GenericAlloc(cls, 0); map->arena = arena; map->field = (uintptr_t)f | 1; - map->parent = parent; + map->ptr.parent = parent; map->version = 0; Py_INCREF(arena); Py_INCREF(parent); return &map->ob_base; } -void PyUpb_MapContainer_SwitchToSet(PyObject* _self, upb_map* map) { +void PyUpb_MapContainer_Reify(PyObject* _self, upb_map* map) { PyUpb_MapContainer* self = (PyUpb_MapContainer*)_self; PyUpb_ObjCache_Add(map, &self->ob_base); - Py_DECREF(self->parent); - self->map = map; // Overwrites self->parent. + Py_DECREF(self->ptr.parent); + self->ptr.map = map; // Overwrites self->ptr.parent. self->field = self->field & ~(uintptr_t)1; assert(!PyUpb_MapContainer_IsUnset(self)); } @@ -121,8 +124,8 @@ static upb_map* PyUpb_MapContainer_AssureWritable(PyUpb_MapContainer* self) { const upb_fielddef* val_f = upb_msgdef_field(entry_m, 1); map = upb_map_new(arena, upb_fielddef_type(key_f), upb_fielddef_type(val_f)); upb_msgval msgval = {.map_val = map}; - PyUpb_CMessage_SetConcreteSubobj(self->parent, f, msgval); - PyUpb_MapContainer_SwitchToSet((PyObject*)self, map); + PyUpb_CMessage_SetConcreteSubobj(self->ptr.parent, f, msgval); + PyUpb_MapContainer_Reify((PyObject*)self, map); return map; } @@ -294,21 +297,21 @@ PyObject* PyUpb_MapContainer_Subscript(PyObject* _self, PyObject* key) { return PyUpb_UpbToPy(u_val, val_f, self->arena); } -PyObject* PyUpb_MapContainer_GetOrCreateWrapper(upb_map* u_map, +PyObject* PyUpb_MapContainer_GetOrCreateWrapper(upb_map* map, const upb_fielddef* f, PyObject* arena) { - PyObject* ret = PyUpb_ObjCache_Get(u_map); + PyObject* ret = PyUpb_ObjCache_Get(map); if (!ret) { PyTypeObject* cls = PyUpb_MapContainer_GetClass(f); PyUpb_MapContainer* map = (void*)PyType_GenericAlloc(cls, 0); map->arena = arena; map->field = (uintptr_t)f; - map->map = u_map; + map->ptr.map = map; map->version = 0; ret = &map->ob_base; Py_INCREF(arena); - PyUpb_ObjCache_Add(u_map, ret); + PyUpb_ObjCache_Add(map, ret); } return ret; diff --git a/python/map.h b/python/map.h index 59d647cbb4..9adc3def17 100644 --- a/python/map.h +++ b/python/map.h @@ -33,16 +33,29 @@ #include "python/python.h" #include "upb/def.h" -PyObject* PyUpb_MapContainer_NewUnset(PyObject* parent, const upb_fielddef* f, - PyObject* arena); -PyObject* PyUpb_MapContainer_GetOrCreateWrapper(upb_map* u_map, +// Creates a new repeated field stub for field `f` of message object `parent`. +PyObject* PyUpb_MapContainer_NewStub(PyObject* parent, const upb_fielddef* f, + PyObject* arena); + +// Returns a map object wrapping `map`, of field type `f`, which must be on +// `arena`. If an existing wrapper object exists, it will be returned, +// otherwise a new object will be created. The caller always owns a ref on the +// returned value. +PyObject* PyUpb_MapContainer_GetOrCreateWrapper(upb_map* map, const upb_fielddef* f, PyObject* arena); -void PyUpb_MapContainer_SwitchToSet(PyObject* self, upb_map* map); + +// Reifies a map stub to point to the concrete data in `map`. +void PyUpb_MapContainer_Reify(PyObject* self, upb_map* map); + +// Assigns `self[key] = val` for the map `self`. int PyUpb_MapContainer_AssignSubscript(PyObject* self, PyObject* key, PyObject* val); + +// Invalidates any existing iterators for the map `obj`. void PyUpb_MapContainer_Invalidate(PyObject* obj); +// Module-level init. bool PyUpb_Map_Init(PyObject* m); #endif // PYUPB_MAP_H__ diff --git a/python/message.c b/python/message.c index 3fe1f6bc24..99ed793f4c 100644 --- a/python/message.c +++ b/python/message.c @@ -435,9 +435,8 @@ static int PyUpb_CMessage_Init(PyObject* _self, PyObject* args, return PyUpb_CMessage_InitAttributes(_self, args, kwargs); } -static PyObject* PyUpb_CMessage_NewUnset(PyObject* parent, - const upb_fielddef* f, - PyObject* arena) { +static PyObject* PyUpb_CMessage_NewStub(PyObject* parent, const upb_fielddef* f, + PyObject* arena) { const upb_msgdef* sub_m = upb_fielddef_msgsubdef(f); PyObject* cls = PyUpb_Descriptor_GetClass(sub_m); @@ -539,14 +538,14 @@ void PyUpb_CMessage_AssureWritable(PyUpb_CMessage* self) { static void PyUpb_CMessage_SyncSubobjs(PyUpb_CMessage* self); /* - * PyUpb_CMessage_SwitchToSet() + * PyUpb_CMessage_Reify() * - * The message equivalent of PyUpb_*Container_SwitchToSet(), this transitions + * The message equivalent of PyUpb_*Container_Reify(), this transitions * the wrapper from the unset state (owning a reference on self->ptr.parent) to the * set state (having a non-owning pointer to self->ptr.msg). */ -static void PyUpb_CMessage_SwitchToSet(PyUpb_CMessage* self, - const upb_fielddef* f, upb_msg* msg) { +static void PyUpb_CMessage_Reify(PyUpb_CMessage* self, const upb_fielddef* f, + upb_msg* msg) { assert(f == PyUpb_CMessage_GetFieldDef(self)); PyUpb_ObjCache_Add(msg, &self->ob_base); Py_DECREF(&self->ptr.parent->ob_base); @@ -596,14 +595,14 @@ static void PyUpb_CMessage_SyncSubobjs(PyUpb_CMessage* self) { PyUpb_WeakMap_DeleteIter(subobj_map, &iter); if (upb_fielddef_ismap(f)) { if (!msgval.map_val) continue; - PyUpb_MapContainer_SwitchToSet(obj, (upb_map*)msgval.map_val); + PyUpb_MapContainer_Reify(obj, (upb_map*)msgval.map_val); } else if (upb_fielddef_isseq(f)) { if (!msgval.array_val) continue; PyUpb_RepeatedContainer_Reify(obj, (upb_array*)msgval.array_val); } else { PyUpb_CMessage* sub = (void*)obj; assert(self == sub->ptr.parent); - PyUpb_CMessage_SwitchToSet(sub, f, (upb_msg*)msgval.msg_val); + PyUpb_CMessage_Reify(sub, f, (upb_msg*)msgval.msg_val); } } @@ -727,11 +726,11 @@ PyObject* PyUpb_CMessage_GetUnsetWrapper(PyUpb_CMessage* self, if (subobj) return subobj; if (upb_fielddef_ismap(field)) { - subobj = PyUpb_MapContainer_NewUnset(_self, field, self->arena); + subobj = PyUpb_MapContainer_NewStub(_self, field, self->arena); } else if (upb_fielddef_isseq(field)) { - subobj = PyUpb_RepeatedContainer_NewUnset(_self, field, self->arena); + subobj = PyUpb_RepeatedContainer_NewStub(_self, field, self->arena); } else { - subobj = PyUpb_CMessage_NewUnset(&self->ob_base, field, self->arena); + subobj = PyUpb_CMessage_NewStub(&self->ob_base, field, self->arena); } PyUpb_WeakMap_Add(self->unset_subobj_map, field, subobj); diff --git a/python/repeated.c b/python/repeated.c index fd41800ec6..de9e0c6b71 100644 --- a/python/repeated.c +++ b/python/repeated.c @@ -155,9 +155,9 @@ static Py_ssize_t PyUpb_RepeatedContainer_Length(PyObject* self) { return arr ? upb_array_size(arr) : 0; } -PyObject* PyUpb_RepeatedContainer_NewUnset(PyObject* parent, - const upb_fielddef* f, - PyObject* arena) { +PyObject* PyUpb_RepeatedContainer_NewStub(PyObject* parent, + const upb_fielddef* f, + PyObject* arena) { PyTypeObject* cls = PyUpb_RepeatedContainer_GetClass(f); PyUpb_RepeatedContainer* repeated = (void*)PyType_GenericAlloc(cls, 0); repeated->arena = arena; diff --git a/python/repeated.h b/python/repeated.h index 5e5f6f94ba..06945bf0c2 100644 --- a/python/repeated.h +++ b/python/repeated.h @@ -33,11 +33,10 @@ #include "python/python.h" #include "upb/def.h" -// Creates a new repeated field in the unset state for field `f` of message -// object `parent`. -PyObject* PyUpb_RepeatedContainer_NewUnset(PyObject* parent, - const upb_fielddef* f, - PyObject* arena); +// Creates a new repeated field stub for field `f` of message object `parent`. +PyObject* PyUpb_RepeatedContainer_NewStub(PyObject* parent, + const upb_fielddef* f, + PyObject* arena); // Returns a repeated field object wrapping `arr`, of field type `f`, which // must be on `arena`. If an existing wrapper object exists, it will be @@ -47,8 +46,7 @@ PyObject* PyUpb_RepeatedContainer_GetOrCreateWrapper(upb_array* arr, const upb_fielddef* f, PyObject* arena); -// Switches a repeated field in the unset state to be set, with `arr` as the -// data being pointed to. +// Reifies a repeated field stub to point to the concrete data in `arr`. void PyUpb_RepeatedContainer_Reify(PyObject* self, upb_array* arr); // Implements repeated_field.extend(iterable). `_self` must be a repeated