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 8, 2015
1 parent 26c720e commit 808b807
Show file tree
Hide file tree
Showing 14 changed files with 174 additions and 74 deletions.
7 changes: 7 additions & 0 deletions from_cpython/Include/Python.h
Original file line number Diff line number Diff line change
Expand Up @@ -135,6 +135,13 @@ 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;

extern PyTypeObject* Itertool_SafeDealloc_Types[];

#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
24 changes: 24 additions & 0 deletions from_cpython/Modules/itertoolsmodule.c
Original file line number Diff line number Diff line change
Expand Up @@ -4080,6 +4080,30 @@ static PyMethodDef module_methods[] = {
{NULL, NULL} /* sentinel */
};

PyTypeObject* Itertool_SafeDealloc_Types[] = {
// &combinations_type,
// &cwr_type,
&cycle_type,
&dropwhile_type,
&takewhile_type,
&islice_type,
&starmap_type,
&imap_type,
&chain_type,
&compress_type,
&ifilter_type,
&ifilterfalse_type,
&count_type,
&izip_type,
&iziplongest_type,
// &permutations_type,
// &product_type,
&repeat_type,
&groupby_type,
NULL
};


PyMODINIT_FUNC
inititertools(void)
{
Expand Down
13 changes: 13 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 Expand Up @@ -3092,6 +3103,8 @@ extern "C" int PyType_Ready(PyTypeObject* cls) noexcept {
// this should get automatically initialized to 0 on this path:
assert(cls->attrs_offset == 0);

checkIfExceptionType(cls);

return 0;
}

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 @@ -1849,7 +1849,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
26 changes: 18 additions & 8 deletions src/runtime/objmodel.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -342,6 +342,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 @@ -360,7 +370,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 @@ -4958,15 +4967,16 @@ Box* typeNew(Box* _cls, Box* arg1, Box* arg2, Box** _args) {
else
made->tp_alloc = PyType_GenericAlloc;

assert(!made->simple_destructor);
assert(!made->has_safe_tp_dealloc);
for (auto b : *bases) {
if (!isSubclass(b->cls, type_cls))
BoxedClass* base = static_cast<BoxedClass*>(b);
if (!isSubclass(base->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;
if (base->has_safe_tp_dealloc) {
made->tp_dealloc = base->tp_dealloc;
made->has_safe_tp_dealloc = true;
break;
}
}

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
71 changes: 49 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 @@ -2910,6 +2912,50 @@ extern "C" PyUnicodeObject* _PyUnicode_New(Py_ssize_t length) noexcept {
return unicode;
}

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);
}

for (int i = 0; Itertool_SafeDealloc_Types[i] != NULL; i++) {
setTypeGCNone(Itertool_SafeDealloc_Types[i]);
}

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

bool TRACK_ALLOCATIONS = false;
void setupRuntime() {

Expand Down Expand Up @@ -3012,7 +3058,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 @@ -3387,27 +3434,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();

unicode_cls->tp_alloc = PystonType_GenericAlloc;
unicode_cls->gc_visit = unicode_visit;
Expand Down
Loading

0 comments on commit 808b807

Please sign in to comment.