Commit 3c0d99cf authored by Kevin Modzelewski's avatar Kevin Modzelewski

Fix some more gc issues

(turning up the collection frequency helped with this a lot)

- register some memory as a static root
- use StlCompatAllocator in the ast_interpreter
- zero out memory in the (hopefully uncommon) cases we call PystonType_GenericAlloc
- when we realloc() a conservative block, update its recorded size
parent 603d7d35
...@@ -663,6 +663,7 @@ calculate_path(void) ...@@ -663,6 +663,7 @@ calculate_path(void)
strcat(buf, exec_prefix); strcat(buf, exec_prefix);
/* And publish the results */ /* And publish the results */
PyGC_AddRoot((void*)buf);
module_search_path = buf; module_search_path = buf;
} }
......
...@@ -42,6 +42,12 @@ ...@@ -42,6 +42,12 @@
#include "runtime/set.h" #include "runtime/set.h"
#include "runtime/types.h" #include "runtime/types.h"
#ifndef NDEBUG
#define DEBUG 1
#else
#define DEBUG 0
#endif
namespace pyston { namespace pyston {
namespace { namespace {
...@@ -55,7 +61,10 @@ union Value { ...@@ -55,7 +61,10 @@ union Value {
Value(bool b) : b(b) {} Value(bool b) : b(b) {}
Value(int64_t n = 0) : n(n) {} Value(int64_t n = 0) : n(n) {}
Value(double d) : d(d) {} Value(double d) : d(d) {}
Value(Box* o) : o(o) {} Value(Box* o) : o(o) {
if (DEBUG >= 2)
assert(gc::isValidGCObject(o));
}
}; };
class ASTInterpreter { class ASTInterpreter {
...@@ -187,7 +196,7 @@ void ASTInterpreter::initArguments(int nargs, BoxedClosure* _closure, BoxedGener ...@@ -187,7 +196,7 @@ void ASTInterpreter::initArguments(int nargs, BoxedClosure* _closure, BoxedGener
if (scope_info->createsClosure()) if (scope_info->createsClosure())
created_closure = createClosure(passed_closure); created_closure = createClosure(passed_closure);
std::vector<Box*> argsArray{ arg1, arg2, arg3 }; std::vector<Box*, StlCompatAllocator<Box*>> argsArray{ arg1, arg2, arg3 };
for (int i = 3; i < nargs; ++i) for (int i = 3; i < nargs; ++i)
argsArray.push_back(args[i - 3]); argsArray.push_back(args[i - 3]);
...@@ -445,7 +454,7 @@ Value ASTInterpreter::visit_jump(AST_Jump* node) { ...@@ -445,7 +454,7 @@ Value ASTInterpreter::visit_jump(AST_Jump* node) {
OSRExit exit(compiled_func, found_entry); OSRExit exit(compiled_func, found_entry);
std::vector<Box*> arg_array; std::vector<Box*, StlCompatAllocator<Box*>> arg_array;
for (auto& it : sorted_symbol_table) { for (auto& it : sorted_symbol_table) {
arg_array.push_back(it.second); arg_array.push_back(it.second);
} }
...@@ -638,7 +647,7 @@ Value ASTInterpreter::visit_return(AST_Return* node) { ...@@ -638,7 +647,7 @@ Value ASTInterpreter::visit_return(AST_Return* node) {
Box* ASTInterpreter::createFunction(AST* node, AST_arguments* args, const std::vector<AST_stmt*>& body) { Box* ASTInterpreter::createFunction(AST* node, AST_arguments* args, const std::vector<AST_stmt*>& body) {
CLFunction* cl = wrapFunction(node, args, body, source_info); CLFunction* cl = wrapFunction(node, args, body, source_info);
std::vector<Box*> defaults; std::vector<Box*, StlCompatAllocator<Box*>> defaults;
for (AST_expr* d : args->defaults) for (AST_expr* d : args->defaults)
defaults.push_back(visit_expr(d).o); defaults.push_back(visit_expr(d).o);
defaults.push_back(0); defaults.push_back(0);
...@@ -685,7 +694,7 @@ Box* ASTInterpreter::createFunction(AST* node, AST_arguments* args, const std::v ...@@ -685,7 +694,7 @@ Box* ASTInterpreter::createFunction(AST* node, AST_arguments* args, const std::v
Value ASTInterpreter::visit_functionDef(AST_FunctionDef* node) { Value ASTInterpreter::visit_functionDef(AST_FunctionDef* node) {
AST_arguments* args = node->args; AST_arguments* args = node->args;
std::vector<Box*> decorators; std::vector<Box*, StlCompatAllocator<Box*>> decorators;
for (AST_expr* d : node->decorator_list) for (AST_expr* d : node->decorator_list)
decorators.push_back(visit_expr(d).o); decorators.push_back(visit_expr(d).o);
...@@ -708,7 +717,7 @@ Value ASTInterpreter::visit_classDef(AST_ClassDef* node) { ...@@ -708,7 +717,7 @@ Value ASTInterpreter::visit_classDef(AST_ClassDef* node) {
BoxedTuple* basesTuple = new BoxedTuple(std::move(bases)); BoxedTuple* basesTuple = new BoxedTuple(std::move(bases));
std::vector<Box*> decorators; std::vector<Box*, StlCompatAllocator<Box*>> decorators;
for (AST_expr* d : node->decorator_list) for (AST_expr* d : node->decorator_list)
decorators.push_back(visit_expr(d).o); decorators.push_back(visit_expr(d).o);
...@@ -919,7 +928,7 @@ Value ASTInterpreter::visit_call(AST_Call* node) { ...@@ -919,7 +928,7 @@ Value ASTInterpreter::visit_call(AST_Call* node) {
func = visit_expr(node->func); func = visit_expr(node->func);
} }
std::vector<Box*> args; std::vector<Box*, StlCompatAllocator<Box*>> args;
for (AST_expr* e : node->args) for (AST_expr* e : node->args)
args.push_back(visit_expr(e).o); args.push_back(visit_expr(e).o);
......
...@@ -35,6 +35,12 @@ ...@@ -35,6 +35,12 @@
//#undef VERBOSITY //#undef VERBOSITY
//#define VERBOSITY(x) 2 //#define VERBOSITY(x) 2
#ifndef NDEBUG
#define DEBUG 1
#else
#define DEBUG 0
#endif
namespace pyston { namespace pyston {
namespace gc { namespace gc {
...@@ -222,7 +228,7 @@ void GCVisitor::visitPotentialRange(void* const* start, void* const* end) { ...@@ -222,7 +228,7 @@ void GCVisitor::visitPotentialRange(void* const* start, void* const* end) {
} }
} }
static void markPhase() { void markPhase() {
#ifndef NVALGRIND #ifndef NVALGRIND
// Have valgrind close its eyes while we do the conservative stack and data scanning, // Have valgrind close its eyes while we do the conservative stack and data scanning,
// since we'll be looking at potentially-uninitialized values: // since we'll be looking at potentially-uninitialized values:
...@@ -263,6 +269,12 @@ static void markPhase() { ...@@ -263,6 +269,12 @@ static void markPhase() {
continue; continue;
} else if (kind_id == GCKind::CONSERVATIVE) { } else if (kind_id == GCKind::CONSERVATIVE) {
uint32_t bytes = al->kind_data; uint32_t bytes = al->kind_data;
if (DEBUG >= 2) {
if (global_heap.small_arena.contains(p)) {
SmallArena::Block* b = SmallArena::Block::forPointer(p);
assert(b->size >= bytes);
}
}
visitor.visitPotentialRange((void**)p, (void**)((char*)p + bytes)); visitor.visitPotentialRange((void**)p, (void**)((char*)p + bytes));
} else if (kind_id == GCKind::PRECISE) { } else if (kind_id == GCKind::PRECISE) {
uint32_t bytes = al->kind_data; uint32_t bytes = al->kind_data;
......
...@@ -272,6 +272,7 @@ private: ...@@ -272,6 +272,7 @@ private:
char _data[ATOM_SIZE]; char _data[ATOM_SIZE];
}; };
public:
struct Block { struct Block {
union { union {
struct { struct {
...@@ -299,7 +300,7 @@ private: ...@@ -299,7 +300,7 @@ private:
static_assert(offsetof(Block, _header_end) >= BLOCK_HEADER_SIZE, "bad header size"); static_assert(offsetof(Block, _header_end) >= BLOCK_HEADER_SIZE, "bad header size");
static_assert(offsetof(Block, _header_end) <= BLOCK_HEADER_SIZE, "bad header size"); static_assert(offsetof(Block, _header_end) <= BLOCK_HEADER_SIZE, "bad header size");
private:
struct ThreadBlockCache { struct ThreadBlockCache {
Heap* heap; Heap* heap;
SmallArena* small; SmallArena* small;
...@@ -473,14 +474,24 @@ public: ...@@ -473,14 +474,24 @@ public:
// reused. Would be nice to factor it all out here into this // reused. Would be nice to factor it all out here into this
// method. // method.
GCAllocation* rtn;
if (large_arena.contains(alloc)) { if (large_arena.contains(alloc)) {
return large_arena.realloc(alloc, bytes); rtn = large_arena.realloc(alloc, bytes);
} else if (huge_arena.contains(alloc)) { } else if (huge_arena.contains(alloc)) {
return huge_arena.realloc(alloc, bytes); rtn = huge_arena.realloc(alloc, bytes);
} else {
assert(small_arena.contains(alloc));
rtn = small_arena.realloc(alloc, bytes);
} }
assert(small_arena.contains(alloc)); // We keep track of the size of conservative objects in the "kind_data" field,
return small_arena.realloc(alloc, bytes); // so with a realloc we have to update that:
if (rtn->kind_id == GCKind::CONSERVATIVE) {
// Round up to a multiple of sizeof(void*):
rtn->kind_data = (bytes + sizeof(void*) - 1) & (~(sizeof(void*) - 1));
}
return rtn;
} }
GCAllocation* __attribute__((__malloc__)) alloc(size_t bytes) { GCAllocation* __attribute__((__malloc__)) alloc(size_t bytes) {
...@@ -523,6 +534,7 @@ public: ...@@ -523,6 +534,7 @@ public:
return NULL; return NULL;
} }
// not thread safe: // not thread safe:
void freeUnmarked(std::list<Box*, StlCompatAllocator<Box*>>& weakly_referenced) { void freeUnmarked(std::list<Box*, StlCompatAllocator<Box*>>& weakly_referenced) {
small_arena.freeUnmarked(weakly_referenced); small_arena.freeUnmarked(weakly_referenced);
...@@ -531,6 +543,8 @@ public: ...@@ -531,6 +543,8 @@ public:
} }
void dumpHeapStatistics(); void dumpHeapStatistics();
friend void markPhase();
}; };
extern Heap global_heap; extern Heap global_heap;
......
...@@ -602,7 +602,7 @@ Box* map(Box* f, BoxedTuple* args) { ...@@ -602,7 +602,7 @@ Box* map(Box* f, BoxedTuple* args) {
assert(args_end.size() == num_iterable); assert(args_end.size() == num_iterable);
Box* rtn = new BoxedList(); Box* rtn = new BoxedList();
std::vector<Box*> current_val(num_iterable); std::vector<Box*, StlCompatAllocator<Box*>> current_val(num_iterable);
while (true) { while (true) {
int num_done = 0; int num_done = 0;
for (int i = 0; i < num_iterable; ++i) { for (int i = 0; i < num_iterable; ++i) {
......
...@@ -4003,6 +4003,7 @@ Box* typeCallInternal(BoxedFunctionBase* f, CallRewriteArgs* rewrite_args, ArgPa ...@@ -4003,6 +4003,7 @@ Box* typeCallInternal(BoxedFunctionBase* f, CallRewriteArgs* rewrite_args, ArgPa
static Box* object_new = NULL; static Box* object_new = NULL;
static Box* object_init = NULL; static Box* object_init = NULL;
// this is ok with not using StlCompatAllocator since we will manually register these objects with the GC
static std::vector<Box*> allowable_news; static std::vector<Box*> allowable_news;
if (!object_new) { if (!object_new) {
object_new = typeLookup(object_cls, new_str, NULL); object_new = typeLookup(object_cls, new_str, NULL);
......
...@@ -42,7 +42,8 @@ public: ...@@ -42,7 +42,8 @@ public:
BoxedSuper* o = static_cast<BoxedSuper*>(_o); BoxedSuper* o = static_cast<BoxedSuper*>(_o);
boxGCHandler(v, o); boxGCHandler(v, o);
v->visit(o->type); if (o->type)
v->visit(o->type);
if (o->obj) if (o->obj)
v->visit(o->obj); v->visit(o->obj);
if (o->obj_type) if (o->obj_type)
......
...@@ -120,12 +120,14 @@ extern "C" PyObject* PystonType_GenericAlloc(BoxedClass* cls, Py_ssize_t nitems) ...@@ -120,12 +120,14 @@ extern "C" PyObject* PystonType_GenericAlloc(BoxedClass* cls, Py_ssize_t nitems)
} }
#endif #endif
// Maybe we should only zero the extension memory?
// I'm not sure we have the information at the moment, but when we were in Box::operator new()
// we knew which memory was beyond C++ class.
void* mem = gc_alloc(size, gc::GCKind::PYTHON); void* mem = gc_alloc(size, gc::GCKind::PYTHON);
RELEASE_ASSERT(mem, ""); RELEASE_ASSERT(mem, "");
// Not sure if we can get away with not initializing this memory.
// I think there are small optimizations we can do, like not initializing cls (always
// the first 8 bytes) since it will get written by PyObject_Init.
memset(mem, '\0', size);
Box* rtn = static_cast<Box*>(mem); Box* rtn = static_cast<Box*>(mem);
PyObject_Init(rtn, cls); PyObject_Init(rtn, cls);
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment