Skip to content

Commit

Permalink
Revert immortalization diffs
Browse files Browse the repository at this point in the history
Summary:
Revert D29304798 (8607d27), D29304967 (d1b4410), and D29432328 (f5c4bab). The first two because we
suspect they're causing a regression, and the last because it builds on the
first two.

Differential Revision: D29523258

fbshipit-source-id: ad6c72f
  • Loading branch information
swtaarrs authored and facebook-github-bot committed Jul 6, 2021
1 parent 8841a45 commit 6504d12
Show file tree
Hide file tree
Showing 61 changed files with 253 additions and 291 deletions.
4 changes: 2 additions & 2 deletions Include/boolobject.h
Expand Up @@ -22,8 +22,8 @@ PyAPI_DATA(struct _longobject) _Py_FalseStruct, _Py_TrueStruct;
#define Py_True ((PyObject *) &_Py_TrueStruct)

/* Macros for returning Py_True or Py_False, respectively */
#define Py_RETURN_TRUE return Py_INCREF_IMMORTAL(Py_True), Py_True
#define Py_RETURN_FALSE return Py_INCREF_IMMORTAL(Py_False), Py_False
#define Py_RETURN_TRUE return Py_INCREF(Py_True), Py_True
#define Py_RETURN_FALSE return Py_INCREF(Py_False), Py_False

/* Function to return a bool from a C long */
PyAPI_FUNC(PyObject *) PyBool_FromLong(long);
Expand Down
28 changes: 7 additions & 21 deletions Include/object.h
Expand Up @@ -84,16 +84,9 @@ whose size is determined when the object is allocated.
{ _PyObject_EXTRA_INIT \
1, type },

#define PyObject_HEAD_INIT_IMMORTAL(type) \
{ _PyObject_EXTRA_INIT \
kImmortalInitialCount, type },

#define PyVarObject_HEAD_INIT(type, size) \
{ PyObject_HEAD_INIT(type) size },

#define PyVarObject_HEAD_INIT_IMMORTAL(type, size) \
{ PyObject_HEAD_INIT_IMMORTAL(type) size },

/* PyObject_VAR_HEAD defines the initial segment of all variable-size
* container objects. These end with a declaration of an array with 1
* element, but enough space is malloc'ed so that the array actually
Expand Down Expand Up @@ -512,6 +505,9 @@ PyAPI_FUNC(void) _Py_dec_count(struct _typeobject *);
when a memory block is reused from a free list. */
PyAPI_FUNC(int) _PyTraceMalloc_NewReference(PyObject *op);

// TODO(eelizondo): Update cinder builds to set -DPy_IMMORTAL_INSTANCES
/* facebook start */
#define Py_IMMORTAL_INSTANCES
/* facebook end */

/* Immortalizing causes the instance to not participate in reference counting.
Expand All @@ -520,8 +516,6 @@ PyAPI_FUNC(int) _PyTraceMalloc_NewReference(PyObject *op);
* a common python heap across many processes. */
#ifdef Py_IMMORTAL_INSTANCES

static const int kImmortalInstances = 1;

/* The GC bit-shifts refcounts left by two, and after that shift we still
* need this to be >>0, so leave three high zero bits (the sign bit and
* room for a shift of two.) */
Expand All @@ -539,19 +533,11 @@ static const Py_ssize_t kImmortalInitialCount = kImmortalBit;
((PyObject *)op)->ob_refcnt = refcnt; \
} while (0)

#define Py_INCREF_IMMORTAL(op) (void)0

#else

static const int kImmortalInstances = 0;

static const Py_ssize_t kImmortalInitialCount = 1;

#define Py_IS_IMMORTAL(op) ((void)(op), 0)

#define Py_SET_REFCNT(op, refcnt) (((PyObject *)op)->ob_refcnt = refcnt)

#define Py_INCREF_IMMORTAL(op) Py_INCREF(op)
#define Py_SET_REFCNT(op, refcnt) (((PyObject *)op)->ob_refcnt = refcnt)

#endif

Expand Down Expand Up @@ -587,12 +573,12 @@ PyAPI_FUNC(void) _Py_Dealloc(PyObject *);

static inline void _Py_INCREF(PyObject *op)
{
_Py_INC_REFTOTAL;
#ifdef Py_IMMORTAL_INSTANCES
if (Py_IS_IMMORTAL(op)) {
return;
}
#endif
_Py_INC_REFTOTAL;
op->ob_refcnt++;
}

Expand All @@ -603,12 +589,12 @@ static inline void _Py_DECREF(const char *filename, int lineno,
{
(void)filename; /* may be unused, shut up -Wunused-parameter */
(void)lineno; /* may be unused, shut up -Wunused-parameter */
_Py_DEC_REFTOTAL;
#ifdef Py_IMMORTAL_INSTANCES
if (Py_IS_IMMORTAL(op)) {
return;
}
#endif
_Py_DEC_REFTOTAL;
if (--op->ob_refcnt != 0) {
#ifdef Py_REF_DEBUG
if (op->ob_refcnt < 0) {
Expand Down Expand Up @@ -702,7 +688,7 @@ PyAPI_DATA(PyObject) _Py_NoneStruct; /* Don't use this directly */
#define Py_None (&_Py_NoneStruct)

/* Macro for returning Py_None from a function */
#define Py_RETURN_NONE return Py_INCREF_IMMORTAL(Py_None), Py_None
#define Py_RETURN_NONE return Py_INCREF(Py_None), Py_None

/*
Py_NotImplemented is a singleton used to signal that an operation is
Expand Down
12 changes: 1 addition & 11 deletions Jit/hir/refcount_insertion.cpp
Expand Up @@ -443,17 +443,7 @@ std::vector<PredState> collectPredStates(Env& env, BasicBlock* block) {
// Return true iff the given Register is definitely not a reference-counted
// value.
bool isUncounted(const Register* reg) {
Type ty = reg->type();
if (!ty.couldBe(TObject)) {
return true;
}

if (kImmortalInstances) {
PyObject* obj = ty.asObject();
return ty <= (TNoneType | TBool) || (obj != nullptr && Py_IS_IMMORTAL(obj));
}

return false;
return !reg->type().couldBe(TObject);
}

// Insert an Incref of `reg` before `cursor`.
Expand Down
80 changes: 37 additions & 43 deletions Jit/lir/generator.cpp
Expand Up @@ -408,27 +408,6 @@ void LIRGenerator::MakeIncref(
cont);
}

auto r1 = GetSafeTempName();

bbb.AppendCode(
"Load {}, {}, {:#x}\n",
r1,
obj,
GET_STRUCT_MEMBER_OFFSET(PyObject, ob_refcnt));

#ifdef Py_IMMORTAL_INSTANCES
auto mortal = GetSafeLabelName();
bbb.AppendCode(
"BitTest {}, {}\n"
"BranchC {}\n"
"{}:\n",
r1, // BitTest
kImmortalBitPos,
end_incref, // BranchC
mortal // label
);
#endif

#ifdef Py_DEBUG
auto r0 = GetSafeTempName();
bbb.AppendCode(
Expand All @@ -442,10 +421,28 @@ void LIRGenerator::MakeIncref(
reinterpret_cast<uint64_t>(&_Py_RefTotal));
#endif

auto r1 = GetSafeTempName();
auto cond_incref = GetSafeLabelName();

bbb.AppendCode(
"Load {}, {}, {:#x}\n"
#ifdef Py_IMMORTAL_INSTANCES
"BitTest {}, {}\n"
"BranchC {}\n"
#endif
"{}:\n"
"Inc {}\n"
"Store {}, {}, {:#x}\n"
"{}:",
r1, // Load
obj,
GET_STRUCT_MEMBER_OFFSET(PyObject, ob_refcnt),
#ifdef Py_IMMORTAL_INSTANCES
r1, // BitTest
kImmortalBitPos,
end_incref, // BranchC
#endif
cond_incref, // label
r1, // Inc
r1, // Store
obj,
Expand All @@ -471,28 +468,6 @@ void LIRGenerator::MakeDecref(
cont);
}

auto r1 = GetSafeTempName();
auto r2 = GetSafeTempName();

bbb.AppendCode(
"Load {}, {}, {:#x}\n",
r1,
obj,
GET_STRUCT_MEMBER_OFFSET(PyObject, ob_refcnt));

#ifdef Py_IMMORTAL_INSTANCES
auto mortal = GetSafeLabelName();
bbb.AppendCode(
"BitTest {}, {}\n"
"BranchC {}\n"
"{}:\n",
r1, // BitTest
kImmortalBitPos,
end_decref, // BranchC
mortal // label
);
#endif

#ifdef Py_DEBUG
auto r0 = GetSafeTempName();
bbb.AppendCode(
Expand All @@ -506,14 +481,33 @@ void LIRGenerator::MakeDecref(
reinterpret_cast<uint64_t>(&_Py_RefTotal));
#endif

auto r1 = GetSafeTempName();
auto r2 = GetSafeTempName();
auto cond_decref = GetSafeLabelName();
auto dealloc = GetSafeLabelName();

bbb.AppendCode(
"Load {}, {}, {:#x}\n"
#ifdef Py_IMMORTAL_INSTANCES
"BitTest {}, {}\n"
"BranchC {}\n"
#endif
"{}:\n"
"Sub {}, {}, 1\n"
"Store {}, {}, {:#x}\n"
"BranchNZ {}\n"
"{}:\n"
"Invoke {:#x}, {}\n"
"{}:",
r1, // Load
obj,
GET_STRUCT_MEMBER_OFFSET(PyObject, ob_refcnt),
#ifdef Py_IMMORTAL_INSTANCES
r1, // BitTest
kImmortalBitPos,
end_decref, // BranchC
#endif
cond_decref, // label
r2, // Sub
r1,
r2, // Store
Expand Down
16 changes: 16 additions & 0 deletions Lib/ctypes/test/test_python_api.py
Expand Up @@ -38,6 +38,22 @@ def test_PyString_FromString(self):
del pyob
self.assertEqual(grc(s), refcnt)

@support.refcount_test
def test_PyLong_Long(self):
ref42 = grc(42)
pythonapi.PyLong_FromLong.restype = py_object
self.assertEqual(pythonapi.PyLong_FromLong(42), 42)

self.assertEqual(grc(42), ref42)

pythonapi.PyLong_AsLong.argtypes = (py_object,)
pythonapi.PyLong_AsLong.restype = c_long

res = pythonapi.PyLong_AsLong(42)
self.assertEqual(grc(res), ref42 + 1)
del res
self.assertEqual(grc(42), ref42)

@support.refcount_test
def test_PyObj_FromPtr(self):
s = "abc def ghi jkl"
Expand Down
9 changes: 4 additions & 5 deletions Lib/test/test_sys.py
Expand Up @@ -357,12 +357,11 @@ def test_refcount(self):
# the reference count to increase by 2 instead of 1.
global n
self.assertRaises(TypeError, sys.getrefcount)
obj = [123]
c = sys.getrefcount(obj)
n = obj
self.assertEqual(sys.getrefcount(obj), c+1)
c = sys.getrefcount(None)
n = None
self.assertEqual(sys.getrefcount(None), c)
del n
self.assertEqual(sys.getrefcount(obj), c)
self.assertEqual(sys.getrefcount(None), c)
if hasattr(sys, "gettotalrefcount"):
self.assertIsInstance(sys.gettotalrefcount(), int)

Expand Down
18 changes: 9 additions & 9 deletions Modules/_ctypes/_ctypes.c
Expand Up @@ -909,7 +909,7 @@ UnionType_setattro(PyObject *self, PyObject *key, PyObject *value)


PyTypeObject PyCStructType_Type = {
PyVarObject_HEAD_INIT_IMMORTAL(NULL, 0)
PyVarObject_HEAD_INIT(NULL, 0)
"_ctypes.PyCStructType", /* tp_name */
0, /* tp_basicsize */
0, /* tp_itemsize */
Expand Down Expand Up @@ -1209,7 +1209,7 @@ static PyMethodDef PyCPointerType_methods[] = {
};

PyTypeObject PyCPointerType_Type = {
PyVarObject_HEAD_INIT_IMMORTAL(NULL, 0)
PyVarObject_HEAD_INIT(NULL, 0)
"_ctypes.PyCPointerType", /* tp_name */
0, /* tp_basicsize */
0, /* tp_itemsize */
Expand Down Expand Up @@ -1631,7 +1631,7 @@ PyCArrayType_new(PyTypeObject *type, PyObject *args, PyObject *kwds)
}

PyTypeObject PyCArrayType_Type = {
PyVarObject_HEAD_INIT_IMMORTAL(NULL, 0)
PyVarObject_HEAD_INIT(NULL, 0)
"_ctypes.PyCArrayType", /* tp_name */
0, /* tp_basicsize */
0, /* tp_itemsize */
Expand Down Expand Up @@ -2325,7 +2325,7 @@ static PyMethodDef PyCSimpleType_methods[] = {
};

PyTypeObject PyCSimpleType_Type = {
PyVarObject_HEAD_INIT_IMMORTAL(NULL, 0)
PyVarObject_HEAD_INIT(NULL, 0)
"_ctypes.PyCSimpleType", /* tp_name */
0, /* tp_basicsize */
0, /* tp_itemsize */
Expand Down Expand Up @@ -2607,7 +2607,7 @@ PyCFuncPtrType_new(PyTypeObject *type, PyObject *args, PyObject *kwds)
}

PyTypeObject PyCFuncPtrType_Type = {
PyVarObject_HEAD_INIT_IMMORTAL(NULL, 0)
PyVarObject_HEAD_INIT(NULL, 0)
"_ctypes.PyCFuncPtrType", /* tp_name */
0, /* tp_basicsize */
0, /* tp_itemsize */
Expand Down Expand Up @@ -2912,7 +2912,7 @@ static PyMethodDef PyCData_methods[] = {
};

PyTypeObject PyCData_Type = {
PyVarObject_HEAD_INIT_IMMORTAL(NULL, 0)
PyVarObject_HEAD_INIT(NULL, 0)
"_ctypes._CData",
sizeof(CDataObject), /* tp_basicsize */
0, /* tp_itemsize */
Expand Down Expand Up @@ -4307,7 +4307,7 @@ static PyNumberMethods PyCFuncPtr_as_number = {
};

PyTypeObject PyCFuncPtr_Type = {
PyVarObject_HEAD_INIT_IMMORTAL(NULL, 0)
PyVarObject_HEAD_INIT(NULL, 0)
"_ctypes.PyCFuncPtr",
sizeof(PyCFuncPtrObject), /* tp_basicsize */
0, /* tp_itemsize */
Expand Down Expand Up @@ -4819,7 +4819,7 @@ static PyMappingMethods Array_as_mapping = {
};

PyTypeObject PyCArray_Type = {
PyVarObject_HEAD_INIT_IMMORTAL(NULL, 0)
PyVarObject_HEAD_INIT(NULL, 0)
"_ctypes.Array",
sizeof(CDataObject), /* tp_basicsize */
0, /* tp_itemsize */
Expand Down Expand Up @@ -5421,7 +5421,7 @@ static PyNumberMethods Pointer_as_number = {
};

PyTypeObject PyCPointer_Type = {
PyVarObject_HEAD_INIT_IMMORTAL(NULL, 0)
PyVarObject_HEAD_INIT(NULL, 0)
"_ctypes._Pointer",
sizeof(CDataObject), /* tp_basicsize */
0, /* tp_itemsize */
Expand Down
2 changes: 1 addition & 1 deletion Modules/_ctypes/callbacks.c
Expand Up @@ -43,7 +43,7 @@ CThunkObject_clear(PyObject *myself)
}

PyTypeObject PyCThunk_Type = {
PyVarObject_HEAD_INIT_IMMORTAL(NULL, 0)
PyVarObject_HEAD_INIT(NULL, 0)
"_ctypes.CThunkObject",
sizeof(CThunkObject), /* tp_basicsize */
sizeof(ffi_type), /* tp_itemsize */
Expand Down
2 changes: 1 addition & 1 deletion Modules/_ctypes/callproc.c
Expand Up @@ -556,7 +556,7 @@ static PyMemberDef PyCArgType_members[] = {
};

PyTypeObject PyCArg_Type = {
PyVarObject_HEAD_INIT_IMMORTAL(NULL, 0)
PyVarObject_HEAD_INIT(NULL, 0)
"CArgObject",
sizeof(PyCArgObject),
0,
Expand Down
2 changes: 1 addition & 1 deletion Modules/_ctypes/cfield.c
Expand Up @@ -300,7 +300,7 @@ PyCField_repr(CFieldObject *self)
}

PyTypeObject PyCField_Type = {
PyVarObject_HEAD_INIT_IMMORTAL(NULL, 0)
PyVarObject_HEAD_INIT(NULL, 0)
"_ctypes.CField", /* tp_name */
sizeof(CFieldObject), /* tp_basicsize */
0, /* tp_itemsize */
Expand Down

0 comments on commit 6504d12

Please sign in to comment.