Merge pull request #666 from rudi-c/gcfinalizers3

Preparation for finalizer code, refactoring some simple_destructor logic.

from_cpython/Include/Python.h

@@ -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

from_cpython/Include/object.h

@@ -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;

from_cpython/Modules/_sre.c

@@ -2695,7 +2695,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),
@@ -3739,7 +3739,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),
@@ -3923,7 +3923,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,

from_cpython/Modules/itertoolsmodule.c

@@ -4080,6 +4080,34 @@ static PyMethodDef module_methods[] = {
     {NULL,              NULL}           /* sentinel */
 };
 
+// Pyston change: These are types defined in this file that I manually
+// checked. The ones that aren't commented out have a `tp_dealloc` that
+// doesn't do anything in Pyston as we switched to garbage collection and
+// the finalizer logic in Pyston wants to know that for optimization purposes.
+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)
 {

from_cpython/Objects/weakrefobject.c

@@ -544,7 +544,9 @@ proxy_dealloc(PyWeakReference *self)
     if (self->wr_callback != NULL)
         PyObject_GC_UnTrack((PyObject *)self);
     clear_weakref(self);
-    PyObject_GC_Del(self);
+    // Pyston change: we monkey-patch this function to use the Pyston GC
+    // Shouldn't need to call tp_free either.
+    // PyObject_GC_Del(self);
 }
 
 /* sequence slots */

src/capi/typeobject.cpp

@@ -3134,6 +3134,10 @@ extern "C" int PyType_Ready(PyTypeObject* cls) noexcept {
     // this should get automatically initialized to 0 on this path:
     assert(cls->attrs_offset == 0);
 
+    if (Py_TPFLAGS_BASE_EXC_SUBCLASS & cls->tp_flags) {
+        exception_types.push_back(cls);
+    }
+
     return 0;
 }
 

src/codegen/ast_interpreter.cpp

@@ -1888,7 +1888,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();
 }
 }

src/gc/heap.cpp

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

src/runtime/file.cpp

@@ -1757,7 +1757,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) }));

src/runtime/frame.cpp

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

src/runtime/generator.cpp

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

src/runtime/objmodel.cpp

@@ -340,6 +340,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
@@ -358,7 +368,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),
@@ -4960,15 +4969,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;

src/runtime/objmodel.h

@@ -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__));

src/runtime/types.cpp

@@ -94,6 +94,8 @@ void setupGC();
 
 bool IN_SHUTDOWN = false;
 
+std::vector<BoxedClass*> exception_types;
+
 void FrameInfo::gcVisit(GCVisitor* visitor) {
     visitor->visit(boxedLocals);
     visitor->visit(exc.traceback);
@@ -1155,11 +1157,6 @@ static void proxy_to_tp_traverse(GCVisitor* v, Box* b) {
     b->cls->tp_traverse(b, call_gc_visit, v);
 }
 
-static void proxy_to_tp_clear(Box* b) {
-    assert(b->cls->tp_clear);
-    b->cls->tp_clear(b);
-}
-
 // This probably belongs in tuple.cpp?
 extern "C" void tupleGCHandler(GCVisitor* v, Box* b) {
     boxGCHandler(v, b);
@@ -2936,6 +2933,55 @@ extern "C" PyUnicodeObject* _PyUnicode_New(Py_ssize_t length) noexcept {
     return unicode;
 }
 
+// Normally we don't call the Python tp_ slots that are present to support
+// CPython's reference-counted garbage collection.
+static void setTypeGCProxy(BoxedClass* cls) {
+    cls->tp_alloc = PystonType_GenericAlloc;
+    cls->tp_free = default_free;
+    cls->gc_visit = proxy_to_tp_traverse;
+    cls->has_safe_tp_dealloc = true;
+    cls->is_pyston_class = true;
+}
+
+// By calling this function on a class we assign it Pyston's GC handling
+// and no finalizers.
+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);
+    unicode_cls->gc_visit = unicode_visit;
+
+    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() {
 
@@ -3038,7 +3084,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,
@@ -3413,32 +3460,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;
-
-    unicode_cls->tp_alloc = PystonType_GenericAlloc;
-    unicode_cls->gc_visit = unicode_visit;
-    unicode_cls->tp_dealloc = NULL;
-    unicode_cls->is_pyston_class = true;
+    setupDefaultClassGCParticipation();
 
     assert(object_cls->tp_setattro == PyObject_GenericSetAttr);
     assert(none_cls->tp_setattro == PyObject_GenericSetAttr);

src/runtime/types.h

@@ -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;
 }
@@ -156,13 +185,6 @@ public:
 
     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
@@ -173,6 +195,17 @@ public:
     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,
@@ -1032,27 +1065,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;