Skip to content

Commit

Permalink
Store simple_destructor in tp_dealloc.
Browse files Browse the repository at this point in the history
Replace the function pointer to the simple_destructor with a boolean
indicating that the tp_dealloc function is safe to call whenever the
simple_destructor used to be instead.

A few additional classes are also specified to have a safe_tp_dealloc.

For exceptions, use a hack where we look for the creation of exception
classes and store them in a list so we can set their destructor at the
same time as other classes.
  • Loading branch information
rudi-c committed Jul 2, 2015
1 parent 7dffffb commit fa9e971
Show file tree
Hide file tree
Showing 13 changed files with 134 additions and 78 deletions.
5 changes: 5 additions & 0 deletions from_cpython/Include/Python.h
Original file line number Diff line number Diff line change
Expand Up @@ -135,6 +135,11 @@ PyObject* PyGC_AddRoot(PyObject*) PYSTON_NOEXCEPT;
// to reduce any chances of compiler reorderings or a GC somehow happening between the assignment
// to the static slot and the call to PyGC_AddRoot.

// Pyston change : expose these type objects
extern PyTypeObject Pattern_Type;
extern PyTypeObject Match_Type;
extern PyTypeObject Scanner_Type;

#define PyDoc_VAR(name) static char name[]
#define PyDoc_STRVAR(name, str) PyDoc_VAR(name) = PyDoc_STR(str)
#define PyDoc_STR(str) str
Expand Down
3 changes: 1 addition & 2 deletions from_cpython/Include/object.h
Original file line number Diff line number Diff line change
Expand Up @@ -456,9 +456,8 @@ struct _typeobject {
void* _hcattrs;
char _ics[32];
void* _gcvisit_func;
void* _dtor;
int _attrs_offset;
bool _flags[3];
bool _flags[4];
void* _tpp_descr_get;
void* _tpp_hasnext;
void* _tpp_call;
Expand Down
6 changes: 3 additions & 3 deletions from_cpython/Modules/_sre.c
Original file line number Diff line number Diff line change
Expand Up @@ -2685,7 +2685,7 @@ static PyMemberDef pattern_members[] = {
{NULL} /* Sentinel */
};

statichere PyTypeObject Pattern_Type = {
PyTypeObject Pattern_Type = {
PyObject_HEAD_INIT(NULL)
0, "_" SRE_MODULE ".SRE_Pattern",
sizeof(PatternObject), sizeof(SRE_CODE),
Expand Down Expand Up @@ -3729,7 +3729,7 @@ static PyMemberDef match_members[] = {
/* FIXME: implement setattr("string", None) as a special case (to
detach the associated string, if any */

static PyTypeObject Match_Type = {
PyTypeObject Match_Type = {
PyVarObject_HEAD_INIT(NULL, 0)
"_" SRE_MODULE ".SRE_Match",
sizeof(MatchObject), sizeof(Py_ssize_t),
Expand Down Expand Up @@ -3913,7 +3913,7 @@ static PyMemberDef scanner_members[] = {
{NULL} /* Sentinel */
};

statichere PyTypeObject Scanner_Type = {
PyTypeObject Scanner_Type = {
PyObject_HEAD_INIT(NULL)
0, "_" SRE_MODULE ".SRE_Scanner",
sizeof(ScannerObject), 0,
Expand Down
11 changes: 11 additions & 0 deletions src/capi/typeobject.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2993,6 +2993,17 @@ void commonClassSetup(BoxedClass* cls) {
assert(cls->tp_dict && cls->tp_dict->cls == attrwrapper_cls);
}

static void checkIfExceptionType(BoxedClass* cls) {
BoxedClass* base = cls;
while (base) {
if (base == PyExc_BaseException) {
exception_types.push_back(cls);
break;
}
base = base->tp_base;
}
}

extern "C" void PyType_Modified(PyTypeObject* type) noexcept {
// We don't cache anything yet that would need to be invalidated:
}
Expand Down
3 changes: 2 additions & 1 deletion src/codegen/ast_interpreter.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1485,7 +1485,8 @@ BoxedClosure* passedClosureForInterpretedFrame(void* frame_ptr) {
void setupInterpreter() {
astinterpreter_cls = BoxedHeapClass::create(type_cls, object_cls, ASTInterpreter::gcHandler, 0, 0,
sizeof(ASTInterpreter), false, "astinterpreter");
astinterpreter_cls->simple_destructor = ASTInterpreter::simpleDestructor;
astinterpreter_cls->tp_dealloc = ASTInterpreter::simpleDestructor;
astinterpreter_cls->has_safe_tp_dealloc = true;
astinterpreter_cls->freeze();
}
}
14 changes: 7 additions & 7 deletions src/gc/heap.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
#include "core/common.h"
#include "core/util.h"
#include "gc/gc_alloc.h"
#include "runtime/objmodel.h"
#include "runtime/types.h"

#ifndef NVALGRIND
Expand Down Expand Up @@ -124,6 +125,8 @@ void _bytesAllocatedTripped() {
Heap global_heap;

__attribute__((always_inline)) bool _doFree(GCAllocation* al, std::vector<Box*>* weakly_referenced) {
static StatCounter gc_safe_destructors("gc_safe_destructor_calls");

#ifndef NVALGRIND
VALGRIND_DISABLE_ERROR_REPORTING;
#endif
Expand Down Expand Up @@ -151,13 +154,10 @@ __attribute__((always_inline)) bool _doFree(GCAllocation* al, std::vector<Box*>*
}
}

// XXX: we are currently ignoring destructors (tp_dealloc) for extension objects, since we have
// historically done that (whoops) and there are too many to be worth changing for now as long
// as we can get real destructor support soon.
ASSERT(b->cls->tp_dealloc == NULL || alloc_kind == GCKind::CONSERVATIVE_PYTHON, "%s", getTypeName(b));

if (b->cls->simple_destructor)
b->cls->simple_destructor(b);
if (b->cls->tp_dealloc != dealloc_null && b->cls->has_safe_tp_dealloc) {
gc_safe_destructors.log();
b->cls->tp_dealloc(b);
}
}
return true;
}
Expand Down
3 changes: 2 additions & 1 deletion src/runtime/file.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1742,7 +1742,8 @@ void BoxedFile::gcHandler(GCVisitor* v, Box* b) {
}

void setupFile() {
file_cls->simple_destructor = fileDestructor;
file_cls->tp_dealloc = fileDestructor;
file_cls->has_safe_tp_dealloc = true;

file_cls->giveAttr("read",
new BoxedFunction(boxRTFunction((void*)fileRead, STR, 2, 1, false, false), { boxInt(-1) }));
Expand Down
3 changes: 2 additions & 1 deletion src/runtime/frame.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -152,7 +152,8 @@ Box* getFrame(int depth) {
void setupFrame() {
frame_cls = BoxedHeapClass::create(type_cls, object_cls, &BoxedFrame::gchandler, 0, 0, sizeof(BoxedFrame), false,
"frame");
frame_cls->simple_destructor = BoxedFrame::simpleDestructor;
frame_cls->tp_dealloc = BoxedFrame::simpleDestructor;
frame_cls->has_safe_tp_dealloc = true;

frame_cls->giveAttr("f_code", new (pyston_getset_cls) BoxedGetsetDescriptor(BoxedFrame::code, NULL, NULL));
frame_cls->giveAttr("f_locals", new (pyston_getset_cls) BoxedGetsetDescriptor(BoxedFrame::locals, NULL, NULL));
Expand Down
3 changes: 2 additions & 1 deletion src/runtime/generator.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -436,7 +436,8 @@ void setupGenerator() {
generator_cls
= BoxedHeapClass::create(type_cls, object_cls, &generatorGCHandler, 0, offsetof(BoxedGenerator, weakreflist),
sizeof(BoxedGenerator), false, "generator");
generator_cls->simple_destructor = generatorDestructor;
generator_cls->tp_dealloc = generatorDestructor;
generator_cls->has_safe_tp_dealloc = true;
generator_cls->giveAttr("__iter__",
new BoxedFunction(boxRTFunction((void*)generatorIter, typeFromClass(generator_cls), 1)));

Expand Down
22 changes: 10 additions & 12 deletions src/runtime/objmodel.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -320,6 +320,16 @@ extern "C" Box** unpackIntoArray(Box* obj, int64_t expected_size) {
return &elts[0];
}

void dealloc_null(Box* box) {
assert(box->cls->tp_del == NULL);
}

// We don't need CPython's version of tp_free since we have GC.
// We still need to set tp_free to something and not a NULL pointer,
// because C extensions might still call tp_free from tp_dealloc.
void default_free(void*) {
}

void BoxedClass::freeze() {
assert(!is_constant);
assert(tp_name); // otherwise debugging will be very hard
Expand All @@ -338,7 +348,6 @@ BoxedClass::BoxedClass(BoxedClass* base, gcvisit_func gc_visit, int attrs_offset
int instance_size, bool is_user_defined)
: attrs(HiddenClass::makeSingleton()),
gc_visit(gc_visit),
simple_destructor(NULL),
attrs_offset(attrs_offset),
is_constant(false),
is_user_defined(is_user_defined),
Expand Down Expand Up @@ -4825,17 +4834,6 @@ Box* typeNew(Box* _cls, Box* arg1, Box* arg2, Box** _args) {
else
made->tp_alloc = PyType_GenericAlloc;

assert(!made->simple_destructor);
for (auto b : *bases) {
if (!isSubclass(b->cls, type_cls))
continue;
BoxedClass* b_cls = static_cast<BoxedClass*>(b);
RELEASE_ASSERT(made->simple_destructor == base->simple_destructor || made->simple_destructor == NULL
|| base->simple_destructor == NULL,
"Conflicting simple destructors!");
made->simple_destructor = base->simple_destructor;
}

return made;
}

Expand Down
4 changes: 4 additions & 0 deletions src/runtime/objmodel.h
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,10 @@ void _printStacktrace();

extern "C" Box* deopt(AST_expr* expr, Box* value);

// Finalizer-related
void default_free(void*);
void dealloc_null(Box* box);

// helper function for raising from the runtime:
void raiseExcHelper(BoxedClass*, const char* fmt, ...) __attribute__((__noreturn__));
void raiseExcHelper(BoxedClass*, Box* arg) __attribute__((__noreturn__));
Expand Down
67 changes: 45 additions & 22 deletions src/runtime/types.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -94,6 +94,8 @@ void setupGC();

bool IN_SHUTDOWN = false;

std::vector<BoxedClass*> exception_types;

#define SLICE_START_OFFSET ((char*)&(((BoxedSlice*)0x01)->start) - (char*)0x1)
#define SLICE_STOP_OFFSET ((char*)&(((BoxedSlice*)0x01)->stop) - (char*)0x1)
#define SLICE_STEP_OFFSET ((char*)&(((BoxedSlice*)0x01)->step) - (char*)0x1)
Expand Down Expand Up @@ -2850,6 +2852,46 @@ static PyObject* type_richcompare(PyObject* v, PyObject* w, int op) noexcept {
return result;
}

static void setTypeGCProxy(BoxedClass* cls) {
cls->tp_alloc = PystonType_GenericAlloc;
cls->gc_visit = proxy_to_tp_traverse;
cls->tp_dealloc = proxy_to_tp_clear;
cls->has_safe_tp_dealloc = true;
cls->is_pyston_class = true;
}

static void setTypeGCNone(BoxedClass* cls) {
cls->tp_alloc = PystonType_GenericAlloc;
cls->tp_free = default_free;
cls->tp_dealloc = dealloc_null;
cls->has_safe_tp_dealloc = true;
cls->is_pyston_class = true;
}

static void setupDefaultClassGCParticipation() {
// some additional setup to ensure weakrefs participate in our GC
setTypeGCProxy(&_PyWeakref_RefType);
setTypeGCProxy(&_PyWeakref_ProxyType);
setTypeGCProxy(&_PyWeakref_CallableProxyType);

// This is an optimization to speed up the handling of unicode objects,
// exception objects, regular expression objects, etc in garbage collection.
// There's no reason to have them part of finalizer ordering.
//
// This is important in tests like django-template which allocates
// hundreds of thousands of unicode strings.
setTypeGCNone(unicode_cls);

for (BoxedClass* cls : exception_types) {
setTypeGCNone(cls);
}

setTypeGCNone(&Scanner_Type);
setTypeGCNone(&Match_Type);
setTypeGCNone(&Pattern_Type);
setTypeGCNone(&PyCallIter_Type);
}

bool TRACK_ALLOCATIONS = false;
void setupRuntime() {

Expand Down Expand Up @@ -2952,7 +2994,8 @@ void setupRuntime() {
BoxedHeapClass(object_cls, &functionGCHandler, 0, offsetof(BoxedBuiltinFunctionOrMethod, in_weakreflist),
sizeof(BoxedBuiltinFunctionOrMethod), false,
static_cast<BoxedString*>(boxString("builtin_function_or_method")));
function_cls->simple_destructor = builtin_function_or_method_cls->simple_destructor = functionDtor;
function_cls->tp_dealloc = builtin_function_or_method_cls->tp_dealloc = functionDtor;
function_cls->has_safe_tp_dealloc = builtin_function_or_method_cls->has_safe_tp_dealloc = true;


module_cls = new (0) BoxedHeapClass(object_cls, &BoxedModule::gcHandler, offsetof(BoxedModule, attrs), 0,
Expand Down Expand Up @@ -3327,27 +3370,7 @@ void setupRuntime() {
PyMarshal_Init();
initstrop();

// some additional setup to ensure weakrefs participate in our GC
BoxedClass* weakref_ref_cls = &_PyWeakref_RefType;
weakref_ref_cls->tp_alloc = PystonType_GenericAlloc;
weakref_ref_cls->tp_dealloc = NULL;
weakref_ref_cls->gc_visit = proxy_to_tp_traverse;
weakref_ref_cls->simple_destructor = proxy_to_tp_clear;
weakref_ref_cls->is_pyston_class = true;

BoxedClass* weakref_proxy_cls = &_PyWeakref_ProxyType;
weakref_proxy_cls->tp_alloc = PystonType_GenericAlloc;
weakref_proxy_cls->tp_dealloc = NULL;
weakref_proxy_cls->gc_visit = proxy_to_tp_traverse;
weakref_proxy_cls->simple_destructor = proxy_to_tp_clear;
weakref_proxy_cls->is_pyston_class = true;

BoxedClass* weakref_callableproxy = &_PyWeakref_CallableProxyType;
weakref_callableproxy->tp_alloc = PystonType_GenericAlloc;
weakref_callableproxy->tp_dealloc = NULL;
weakref_callableproxy->gc_visit = proxy_to_tp_traverse;
weakref_callableproxy->simple_destructor = proxy_to_tp_clear;
weakref_callableproxy->is_pyston_class = true;
setupDefaultClassGCParticipation();

assert(object_cls->tp_setattro == PyObject_GenericSetAttr);
assert(none_cls->tp_setattro == PyObject_GenericSetAttr);
Expand Down
68 changes: 40 additions & 28 deletions src/runtime/types.h
Original file line number Diff line number Diff line change
Expand Up @@ -94,6 +94,35 @@ extern BoxedClass* object_cls, *type_cls, *bool_cls, *int_cls, *long_cls, *float
#define unicode_cls (&PyUnicode_Type)
#define memoryview_cls (&PyMemoryView_Type)

#define unicode_cls (&PyUnicode_Type)
#define memoryview_cls (&PyMemoryView_Type)

#define SystemError ((BoxedClass*)PyExc_SystemError)
#define StopIteration ((BoxedClass*)PyExc_StopIteration)
#define NameError ((BoxedClass*)PyExc_NameError)
#define UnboundLocalError ((BoxedClass*)PyExc_UnboundLocalError)
#define BaseException ((BoxedClass*)PyExc_BaseException)
#define TypeError ((BoxedClass*)PyExc_TypeError)
#define AssertionError ((BoxedClass*)PyExc_AssertionError)
#define ValueError ((BoxedClass*)PyExc_ValueError)
#define SystemExit ((BoxedClass*)PyExc_SystemExit)
#define SyntaxError ((BoxedClass*)PyExc_SyntaxError)
#define Exception ((BoxedClass*)PyExc_Exception)
#define AttributeError ((BoxedClass*)PyExc_AttributeError)
#define RuntimeError ((BoxedClass*)PyExc_RuntimeError)
#define ZeroDivisionError ((BoxedClass*)PyExc_ZeroDivisionError)
#define ImportError ((BoxedClass*)PyExc_ImportError)
#define IndexError ((BoxedClass*)PyExc_IndexError)
#define GeneratorExit ((BoxedClass*)PyExc_GeneratorExit)
#define IOError ((BoxedClass*)PyExc_IOError)
#define KeyError ((BoxedClass*)PyExc_KeyError)
#define OverflowError ((BoxedClass*)PyExc_OverflowError)

// Contains a list classes that have BaseException as a parent. This list is NOT guaranteed to be
// comprehensive - it will not contain user-defined exception types. This is mainly for optimization
// purposes, where it's useful to speed up the garbage collection of some exceptions.
extern std::vector<BoxedClass*> exception_types;

extern "C" {
extern Box* None, *NotImplemented, *True, *False;
}
Expand Down Expand Up @@ -161,13 +190,6 @@ class BoxedClass : public BoxVar {

gcvisit_func gc_visit;

// A "simple" destructor -- one that is allowed to be called at any point after the object is dead.
// In particular, this means that it can't touch any Python objects or other gc-managed memory,
// since it will be in an undefined state.
// (Context: in Python destructors are supposed to be called in topological order, due to reference counting.
// We don't support that yet, but still want some simple ability to run code when an object gets freed.)
void (*simple_destructor)(Box*);

// Offset of the HCAttrs object or 0 if there are no hcattrs.
// Negative offset is from the end of the class (useful for variable-size objects with the attrs at the end)
// Analogous to tp_dictoffset
Expand All @@ -178,6 +200,17 @@ class BoxedClass : public BoxVar {
bool instancesHaveHCAttrs() { return attrs_offset != 0; }
bool instancesHaveDictAttrs() { return tp_dictoffset != 0; }

// A "safe" tp_dealloc destructor/finalizer is one we believe:
// 1) Can be called at any point after the object is dead.
// (implies it's references could be finalized already)
// 2) Won't take a lot of time to run.
// 3) Won't take up a lot of memory (requiring another GC run).
// 4) Won't resurrect itself.
//
// We specify that such destructors are safe for optimization purposes. We call the tp_dealloc
// as the object gets freed.
bool has_safe_tp_dealloc;

// Whether this class object is constant or not, ie whether or not class-level
// attributes can be changed or added.
// Does not necessarily imply that the instances of this class are constant,
Expand Down Expand Up @@ -988,27 +1021,6 @@ void attrwrapperDel(Box* b, llvm::StringRef attr);
Box* boxAst(AST* ast);
AST* unboxAst(Box* b);

#define SystemError ((BoxedClass*)PyExc_SystemError)
#define StopIteration ((BoxedClass*)PyExc_StopIteration)
#define NameError ((BoxedClass*)PyExc_NameError)
#define UnboundLocalError ((BoxedClass*)PyExc_UnboundLocalError)
#define BaseException ((BoxedClass*)PyExc_BaseException)
#define TypeError ((BoxedClass*)PyExc_TypeError)
#define AssertionError ((BoxedClass*)PyExc_AssertionError)
#define ValueError ((BoxedClass*)PyExc_ValueError)
#define SystemExit ((BoxedClass*)PyExc_SystemExit)
#define SyntaxError ((BoxedClass*)PyExc_SyntaxError)
#define Exception ((BoxedClass*)PyExc_Exception)
#define AttributeError ((BoxedClass*)PyExc_AttributeError)
#define RuntimeError ((BoxedClass*)PyExc_RuntimeError)
#define ZeroDivisionError ((BoxedClass*)PyExc_ZeroDivisionError)
#define ImportError ((BoxedClass*)PyExc_ImportError)
#define IndexError ((BoxedClass*)PyExc_IndexError)
#define GeneratorExit ((BoxedClass*)PyExc_GeneratorExit)
#define IOError ((BoxedClass*)PyExc_IOError)
#define KeyError ((BoxedClass*)PyExc_KeyError)
#define OverflowError ((BoxedClass*)PyExc_OverflowError)

// Our default for tp_alloc:
extern "C" PyObject* PystonType_GenericAlloc(BoxedClass* cls, Py_ssize_t nitems) noexcept;

Expand Down

0 comments on commit fa9e971

Please sign in to comment.