From 4f6fd75d5e7b107c48a958759689c5776f96683a Mon Sep 17 00:00:00 2001 From: Tobias Wrigstad Date: Wed, 15 Jan 2025 16:02:10 +0100 Subject: [PATCH 1/7] Added write barriers to object.c; uses the TP_REGION_AWARE flag --- Include/internal/pycore_regions.h | 5 ++++- Objects/cellobject.c | 3 ++- Objects/complexobject.c | 3 ++- Objects/cown.c | 2 +- Objects/longobject.c | 3 ++- Objects/object.c | 24 +++++++++++++++++------- Objects/regions.c | 17 ++++++++++++++++- Objects/tupleobject.c | 3 ++- Objects/typeobject.c | 19 +++++++++++++++---- 9 files changed, 61 insertions(+), 18 deletions(-) diff --git a/Include/internal/pycore_regions.h b/Include/internal/pycore_regions.h index 6a2c808ec2987a..c4c770046362b5 100644 --- a/Include/internal/pycore_regions.h +++ b/Include/internal/pycore_regions.h @@ -12,6 +12,9 @@ extern "C" { #include "object.h" #include "regions.h" +#define Py_IS_REGION_AWARE(tp) (tp->tp_flags & Py_TPFLAGS_REGION_AWARE) +#define Py_OBJ_IS_REGION_AWARE(op) Py_IS_REGION_AWARE(Py_TYPE(op)) + #define Py_CHECKWRITE(op) ((op) && !Py_IsImmutable(op)) #define Py_REQUIREWRITE(op, msg) {if (Py_CHECKWRITE(op)) { _PyObject_ASSERT_FAILED_MSG(op, msg); }} @@ -56,7 +59,6 @@ PyObject* _Py_EnableInvariant(void); PyObject* _Py_ResetInvariant(void); #define Py_ResetInvariant() _Py_ResetInvariant() -// Invariant placeholder bool _Py_RegionAddReference(PyObject* src, PyObject* new_tgt); #define Py_REGIONADDREFERENCE(a, b) _Py_RegionAddReference(_PyObject_CAST(a), b) @@ -92,6 +94,7 @@ PyObject *_PyCown_close_region(PyObject* ob); #define PyCown_close_region(op) _PyCown_close_region(_PyObject_CAST(op)) int _PyRegion_is_closed(PyObject* op); #define PyRegion_is_closed(op) _PyRegion_is_closed(_PyObject_CAST(op)) +void _PyObject_mark_region_as_dirty(PyObject *op); #ifdef _Py_TYPEOF diff --git a/Objects/cellobject.c b/Objects/cellobject.c index 0c7b648605e440..044a334f580f0e 100644 --- a/Objects/cellobject.c +++ b/Objects/cellobject.c @@ -1,6 +1,7 @@ /* Cell object implementation */ #include "Python.h" +#include "object.h" #include "pycore_object.h" #include "pycore_regions.h" @@ -186,7 +187,7 @@ PyTypeObject PyCell_Type = { PyObject_GenericGetAttr, /* tp_getattro */ 0, /* tp_setattro */ 0, /* tp_as_buffer */ - Py_TPFLAGS_DEFAULT | Py_TPFLAGS_HAVE_GC, /* tp_flags */ + Py_TPFLAGS_DEFAULT | Py_TPFLAGS_HAVE_GC | Py_TPFLAGS_REGION_AWARE, /* tp_flags */ cell_new_doc, /* tp_doc */ (traverseproc)cell_traverse, /* tp_traverse */ (inquiry)cell_clear, /* tp_clear */ diff --git a/Objects/complexobject.c b/Objects/complexobject.c index aee03ddfb07aec..0fd14ea3fcd760 100644 --- a/Objects/complexobject.c +++ b/Objects/complexobject.c @@ -6,6 +6,7 @@ /* Submitted by Jim Hugunin */ #include "Python.h" +#include "object.h" #include "pycore_call.h" // _PyObject_CallNoArgs() #include "pycore_long.h" // _PyLong_GetZero() #include "pycore_object.h" // _PyObject_Init() @@ -1087,7 +1088,7 @@ PyTypeObject PyComplex_Type = { PyObject_GenericGetAttr, /* tp_getattro */ 0, /* tp_setattro */ 0, /* tp_as_buffer */ - Py_TPFLAGS_DEFAULT | Py_TPFLAGS_BASETYPE, /* tp_flags */ + Py_TPFLAGS_DEFAULT | Py_TPFLAGS_BASETYPE | Py_TPFLAGS_REGION_AWARE, /* tp_flags */ complex_new__doc__, /* tp_doc */ 0, /* tp_traverse */ 0, /* tp_clear */ diff --git a/Objects/cown.c b/Objects/cown.c index 384fdaba33fb57..44745cdf8c8dd4 100644 --- a/Objects/cown.c +++ b/Objects/cown.c @@ -320,7 +320,7 @@ PyTypeObject PyCown_Type = { 0, /* tp_getattro */ 0, /* tp_setattro */ 0, /* tp_as_buffer */ - Py_TPFLAGS_DEFAULT | Py_TPFLAGS_HAVE_GC, /* tp_flags */ + Py_TPFLAGS_DEFAULT | Py_TPFLAGS_HAVE_GC | Py_TPFLAGS_REGION_AWARE, /* tp_flags */ 0, /* tp_doc */ (traverseproc)PyCown_traverse, /* tp_traverse */ (inquiry)PyCown_clear, /* tp_clear */ diff --git a/Objects/longobject.c b/Objects/longobject.c index 5d9b413861478a..a33b5dffc0c4a4 100644 --- a/Objects/longobject.c +++ b/Objects/longobject.c @@ -3,6 +3,7 @@ /* XXX The functional organization of this file is terrible */ #include "Python.h" +#include "object.h" #include "pycore_bitutils.h" // _Py_popcount32() #include "pycore_initconfig.h" // _PyStatus_OK() #include "pycore_long.h" // _Py_SmallInts @@ -6271,7 +6272,7 @@ PyTypeObject PyLong_Type = { 0, /* tp_setattro */ 0, /* tp_as_buffer */ Py_TPFLAGS_DEFAULT | Py_TPFLAGS_BASETYPE | - Py_TPFLAGS_LONG_SUBCLASS | + Py_TPFLAGS_LONG_SUBCLASS | Py_TPFLAGS_REGION_AWARE | _Py_TPFLAGS_MATCH_SELF, /* tp_flags */ long_doc, /* tp_doc */ 0, /* tp_traverse */ diff --git a/Objects/object.c b/Objects/object.c index a9813a999fdb62..c8c51038de9c59 100644 --- a/Objects/object.c +++ b/Objects/object.c @@ -1173,9 +1173,12 @@ PyObject_SetAttr(PyObject *v, PyObject *name, PyObject *value) PyUnicode_InternInPlace(&name); if (tp->tp_setattro != NULL) { - if(Py_CHECKWRITE(v)){ + if (Py_CHECKWRITE(v)) { + if (!Py_IS_REGION_AWARE(tp)) { + _PyObject_mark_region_as_dirty(v); + } err = (*tp->tp_setattro)(v, name, value); - }else{ + } else { PyErr_WriteToImmutable(v); err = -1; } @@ -1190,9 +1193,12 @@ PyObject_SetAttr(PyObject *v, PyObject *name, PyObject *value) return -1; } - if(Py_CHECKWRITE(v)){ + if (Py_CHECKWRITE(v)) { + if (!Py_IS_REGION_AWARE(tp)) { + _PyObject_mark_region_as_dirty(v); + } err = (*tp->tp_setattr)(v, (char *)name_str, value); - }else{ + } else { PyErr_WriteToImmutable(v); err = -1; } @@ -1667,8 +1673,12 @@ PyObject_GenericSetDict(PyObject *obj, PyObject *value, void *context) "not a '%.200s'", Py_TYPE(value)->tp_name); return -1; } - Py_XSETREF(*dictptr, Py_NewRef(value)); - return 0; + if (Py_REGIONADDREFERENCE(obj, value)) { + Py_XSETREF(*dictptr, Py_NewRef(value)); + return 0; + } else { + return -1; + } } @@ -1895,7 +1905,7 @@ PyTypeObject _PyNone_Type = { 0, /*tp_getattro */ 0, /*tp_setattro */ 0, /*tp_as_buffer */ - Py_TPFLAGS_DEFAULT, /*tp_flags */ + Py_TPFLAGS_DEFAULT | Py_TPFLAGS_REGION_AWARE, /*tp_flags */ 0, /*tp_doc */ 0, /*tp_traverse */ 0, /*tp_clear */ diff --git a/Objects/regions.c b/Objects/regions.c index ab7e602e211663..8cedaf061f6cc5 100644 --- a/Objects/regions.c +++ b/Objects/regions.c @@ -240,6 +240,13 @@ static bool regionmetadata_is_open(Py_region_ptr_t self) { #define regionmetadata_is_open(self) \ regionmetadata_is_open(REGION_PTR_CAST(self)) +void _PyObject_mark_region_as_dirty(PyObject *op) { + Py_region_ptr_t rp = _Py_REGION(op); + if (HAS_METADATA(rp)) { + REGION_DATA_CAST(rp)->is_dirty = true; + } +} + static void regionmetadata_inc_osc(Py_region_ptr_t self_ptr) { if (!HAS_METADATA(self_ptr)) { @@ -1874,7 +1881,7 @@ PyTypeObject PyRegion_Type = { 0, /* tp_getattro */ 0, /* tp_setattro */ 0, /* tp_as_buffer */ - Py_TPFLAGS_DEFAULT | Py_TPFLAGS_HAVE_GC, /* tp_flags */ + Py_TPFLAGS_DEFAULT | Py_TPFLAGS_HAVE_GC | Py_TPFLAGS_REGION_AWARE, /* tp_flags */ "TODO =^.^=", /* tp_doc */ (traverseproc)PyRegion_traverse, /* tp_traverse */ (inquiry)PyRegion_clear, /* tp_clear */ @@ -1920,6 +1927,14 @@ static const char *get_region_name(PyObject* obj) { } } +// TODO replace with write barrier code +bool _Pyrona_RemoveReference(PyObject *src, PyObject *tgt) { +#ifndef NDEBUG + _Py_VPYDBG("Removed %p --> %p (owner: '%s')\n", src, tgt, get_region_name(src)); +#endif + return true; // TODO: implement +} + // x.f = y ==> _Pyrona_AddReference(src=x, tgt=y) bool _Py_RegionAddReference(PyObject *src, PyObject *tgt) { if (Py_REGION(src) == Py_REGION(tgt)) { diff --git a/Objects/tupleobject.c b/Objects/tupleobject.c index e043054197ae05..c5aa35c6f9c9c4 100644 --- a/Objects/tupleobject.c +++ b/Objects/tupleobject.c @@ -2,6 +2,7 @@ /* Tuple object implementation */ #include "Python.h" +#include "object.h" #include "pycore_abstract.h" // _PyIndex_Check() #include "pycore_gc.h" // _PyObject_GC_IS_TRACKED() #include "pycore_initconfig.h" // _PyStatus_OK() @@ -879,7 +880,7 @@ PyTypeObject PyTuple_Type = { 0, /* tp_as_buffer */ Py_TPFLAGS_DEFAULT | Py_TPFLAGS_HAVE_GC | Py_TPFLAGS_BASETYPE | Py_TPFLAGS_TUPLE_SUBCLASS | - _Py_TPFLAGS_MATCH_SELF | Py_TPFLAGS_SEQUENCE, /* tp_flags */ + _Py_TPFLAGS_MATCH_SELF | Py_TPFLAGS_SEQUENCE | Py_TPFLAGS_REGION_AWARE, /* tp_flags */ tuple_new__doc__, /* tp_doc */ (traverseproc)tupletraverse, /* tp_traverse */ 0, /* tp_clear */ diff --git a/Objects/typeobject.c b/Objects/typeobject.c index 8547f0f2a86277..a85e4f75746847 100644 --- a/Objects/typeobject.c +++ b/Objects/typeobject.c @@ -1,8 +1,10 @@ /* Type object implementation */ #include "Python.h" +#include "object.h" #include "pycore_call.h" #include "pycore_code.h" // CO_FAST_FREE +#include "pycore_regions.h" #include "pycore_symtable.h" // _Py_Mangle() #include "pycore_dict.h" // _PyDict_KeysSize() #include "pycore_initconfig.h" // _PyStatus_OK() @@ -2940,8 +2942,15 @@ subtype_setdict(PyObject *obj, PyObject *value, void *context) "not a '%.200s'", Py_TYPE(value)->tp_name); return -1; } - Py_XSETREF(*dictptr, Py_XNewRef(value)); - return 0; + if (value == NULL || Py_REGIONADDREFERENCE(obj, value)) { + if (*dictptr) { + Py_REGIONREMOVEREFERENCE(obj, *dictptr); + } + Py_XSETREF(*dictptr, Py_XNewRef(value)); + return 0; + } else { + return -1; + } } static PyObject * @@ -3326,7 +3335,7 @@ type_new_alloc(type_new_ctx *ctx) // All heap types need GC, since we can create a reference cycle by storing // an instance on one of its parents. type->tp_flags = (Py_TPFLAGS_DEFAULT | Py_TPFLAGS_HEAPTYPE | - Py_TPFLAGS_BASETYPE | Py_TPFLAGS_HAVE_GC); + Py_TPFLAGS_BASETYPE | Py_TPFLAGS_HAVE_GC | Py_TPFLAGS_REGION_AWARE); // Initialize essential fields type->tp_as_async = &et->as_async; @@ -5349,6 +5358,7 @@ PyTypeObject PyType_Type = { Py_TPFLAGS_DEFAULT | Py_TPFLAGS_HAVE_GC | Py_TPFLAGS_BASETYPE | Py_TPFLAGS_TYPE_SUBCLASS | Py_TPFLAGS_HAVE_VECTORCALL | + Py_TPFLAGS_REGION_AWARE | Py_TPFLAGS_ITEMS_AT_END, /* tp_flags */ type_doc, /* tp_doc */ (traverseproc)type_traverse, /* tp_traverse */ @@ -6559,7 +6569,8 @@ PyTypeObject PyBaseObject_Type = { PyObject_GenericGetAttr, /* tp_getattro */ PyObject_GenericSetAttr, /* tp_setattro */ 0, /* tp_as_buffer */ - Py_TPFLAGS_DEFAULT | Py_TPFLAGS_BASETYPE, /* tp_flags */ + Py_TPFLAGS_DEFAULT | Py_TPFLAGS_BASETYPE | + Py_TPFLAGS_REGION_AWARE, /* tp_flags */ object_doc, /* tp_doc */ 0, /* tp_traverse */ 0, /* tp_clear */ From 15cdeaaadf45a1a335fa92135bdfe2864be77cd3 Mon Sep 17 00:00:00 2001 From: Tobias Wrigstad Date: Wed, 15 Jan 2025 20:17:06 +0100 Subject: [PATCH 2/7] Update Objects/regions.c Co-authored-by: Fridtjof Stoldt --- Objects/regions.c | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/Objects/regions.c b/Objects/regions.c index 8cedaf061f6cc5..064602a12d3754 100644 --- a/Objects/regions.c +++ b/Objects/regions.c @@ -241,10 +241,7 @@ static bool regionmetadata_is_open(Py_region_ptr_t self) { regionmetadata_is_open(REGION_PTR_CAST(self)) void _PyObject_mark_region_as_dirty(PyObject *op) { - Py_region_ptr_t rp = _Py_REGION(op); - if (HAS_METADATA(rp)) { - REGION_DATA_CAST(rp)->is_dirty = true; - } + regionmetadata_mark_as_dirty(_Py_REGION(op)); } static void regionmetadata_inc_osc(Py_region_ptr_t self_ptr) From 7444e96b3f2d7df00acee4d57386a5ea636d5722 Mon Sep 17 00:00:00 2001 From: Tobias Wrigstad Date: Wed, 15 Jan 2025 21:47:00 +0100 Subject: [PATCH 3/7] Marking a region dirty now marks its parent regions dirty as well --- Objects/regions.c | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/Objects/regions.c b/Objects/regions.c index 064602a12d3754..a9f8a31f48edb2 100644 --- a/Objects/regions.c +++ b/Objects/regions.c @@ -159,8 +159,14 @@ static void regionmetadata_mark_as_dirty(Py_region_ptr_t self_ptr) { if (!HAS_METADATA(self_ptr)) { return; } + regionmetadata *self = REGION_DATA_CAST(self_ptr); + self->is_dirty = true; - REGION_DATA_CAST(self_ptr)->is_dirty = true; + // Mark any parent as dirty as well + regionmetadata *parent = regionmetadata_get_parent(self); + if (parent) { + regionmetadata_mark_as_dirty(REGION_PTR_CAST(parent)); + } } # define regionmetadata_mark_as_dirty(data) \ (regionmetadata_mark_as_dirty(REGION_PTR_CAST(data))) From 1577c5788ca9bec082b5f558ef071f499bb8084f Mon Sep 17 00:00:00 2001 From: Tobias Wrigstad Date: Mon, 20 Jan 2025 13:24:50 +0100 Subject: [PATCH 4/7] Removing the marking of parents as dirty --- Objects/regions.c | 6 ------ 1 file changed, 6 deletions(-) diff --git a/Objects/regions.c b/Objects/regions.c index a9f8a31f48edb2..26749de346e00c 100644 --- a/Objects/regions.c +++ b/Objects/regions.c @@ -161,12 +161,6 @@ static void regionmetadata_mark_as_dirty(Py_region_ptr_t self_ptr) { } regionmetadata *self = REGION_DATA_CAST(self_ptr); self->is_dirty = true; - - // Mark any parent as dirty as well - regionmetadata *parent = regionmetadata_get_parent(self); - if (parent) { - regionmetadata_mark_as_dirty(REGION_PTR_CAST(parent)); - } } # define regionmetadata_mark_as_dirty(data) \ (regionmetadata_mark_as_dirty(REGION_PTR_CAST(data))) From f1692b887a14341bb24269bacdeec0bb69840c16 Mon Sep 17 00:00:00 2001 From: Tobias Wrigstad Date: Mon, 20 Jan 2025 21:19:47 +0100 Subject: [PATCH 5/7] Store a bridge object ref r into a region-unaware type => dirty r --- Objects/object.c | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/Objects/object.c b/Objects/object.c index c8c51038de9c59..912d9154f3649e 100644 --- a/Objects/object.c +++ b/Objects/object.c @@ -31,6 +31,7 @@ extern "C" { /* Defined in tracemalloc.c */ extern void _PyMem_DumpTraceback(int fd, const void *ptr); +extern int _Py_is_bridge_object(PyObject *op); int @@ -1176,6 +1177,13 @@ PyObject_SetAttr(PyObject *v, PyObject *name, PyObject *value) if (Py_CHECKWRITE(v)) { if (!Py_IS_REGION_AWARE(tp)) { _PyObject_mark_region_as_dirty(v); + // If x.f = y and type(x) isn't region aware, + // when y is a bridge object, mark its region + // as dirty since we probably just violated the + // external uniqueness invariant. + if (_Py_is_bridge_object(value)) { + _PyObject_mark_region_as_dirty(value); + } } err = (*tp->tp_setattro)(v, name, value); } else { @@ -1196,6 +1204,13 @@ PyObject_SetAttr(PyObject *v, PyObject *name, PyObject *value) if (Py_CHECKWRITE(v)) { if (!Py_IS_REGION_AWARE(tp)) { _PyObject_mark_region_as_dirty(v); + // If x.f = y and type(x) isn't region aware, + // when y is a bridge object, mark its region + // as dirty since we probably just violated the + // external uniqueness invariant. + if (_Py_is_bridge_object(value)) { + _PyObject_mark_region_as_dirty(value); + } } err = (*tp->tp_setattr)(v, (char *)name_str, value); } else { From 2f92d2c05a4b8008eab62302f1f4d538b7ad0131 Mon Sep 17 00:00:00 2001 From: Tobias Wrigstad Date: Sat, 1 Feb 2025 15:08:30 +0100 Subject: [PATCH 6/7] Added Py_XSETREF_WITH_REGION Currently called from one place. --- Include/cpython/object.h | 22 ++++++++++++++++++++++ Objects/typeobject.c | 5 +---- 2 files changed, 23 insertions(+), 4 deletions(-) diff --git a/Include/cpython/object.h b/Include/cpython/object.h index 413e44f28ec955..83fa85d748a055 100644 --- a/Include/cpython/object.h +++ b/Include/cpython/object.h @@ -386,6 +386,28 @@ PyAPI_FUNC(PyObject *) _PyObject_FunctionStr(PyObject *); } while (0) #endif +/* Py_XSETREF_WITH_REGION() is a region-aware variant of Py_XSETREF */ +#ifdef _Py_TYPEOF +#define Py_XSETREF_WITH_REGION(dst, src, obj) \ + do { \ + _Py_TYPEOF(dst)* _tmp_dst_ptr = &(dst); \ + _Py_TYPEOF(dst) _tmp_old_dst = (*_tmp_dst_ptr); \ + *_tmp_dst_ptr = (src); \ + if (_tmp_old_dst) Py_REGIONREMOVEREFERENCE(obj, _tmp_old_dst); \ + Py_XDECREF(_tmp_old_dst); \ + } while (0) +#else +#define Py_XSETREF_WITH_REGION(dst, src, obj) \ + do { \ + PyObject **_tmp_dst_ptr = _Py_CAST(PyObject**, &(dst)); \ + PyObject *_tmp_old_dst = (*_tmp_dst_ptr); \ + PyObject *_tmp_src = _PyObject_CAST(src); \ + memcpy(_tmp_dst_ptr, &_tmp_src, sizeof(PyObject*)); \ + if (_tmp_old_dst) Py_REGIONREMOVEREFERENCE(obj, _tmp_old_dst); \ + Py_XDECREF(_tmp_old_dst); \ + } while (0) +#endif + PyAPI_DATA(PyTypeObject) _PyNone_Type; PyAPI_DATA(PyTypeObject) _PyNotImplemented_Type; diff --git a/Objects/typeobject.c b/Objects/typeobject.c index a85e4f75746847..3e79a0ff1f16d5 100644 --- a/Objects/typeobject.c +++ b/Objects/typeobject.c @@ -2943,10 +2943,7 @@ subtype_setdict(PyObject *obj, PyObject *value, void *context) return -1; } if (value == NULL || Py_REGIONADDREFERENCE(obj, value)) { - if (*dictptr) { - Py_REGIONREMOVEREFERENCE(obj, *dictptr); - } - Py_XSETREF(*dictptr, Py_XNewRef(value)); + Py_XSETREF_WITH_REGION(*dictptr, Py_XNewRef(value), obj); return 0; } else { return -1; From 610dcb006fb74380a79cb25b1ef9527acf426784 Mon Sep 17 00:00:00 2001 From: Tobias Wrigstad Date: Mon, 24 Feb 2025 17:57:49 +0100 Subject: [PATCH 7/7] Called XSET_REF_WITH_REGION also in GenericSetDict --- Objects/object.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Objects/object.c b/Objects/object.c index 912d9154f3649e..29805784645b9c 100644 --- a/Objects/object.c +++ b/Objects/object.c @@ -1689,7 +1689,7 @@ PyObject_GenericSetDict(PyObject *obj, PyObject *value, void *context) return -1; } if (Py_REGIONADDREFERENCE(obj, value)) { - Py_XSETREF(*dictptr, Py_NewRef(value)); + Py_XSETREF_WITH_REGION(*dictptr, Py_NewRef(value), obj); return 0; } else { return -1;