Merge pull request #422 from toshok/gc-perf

a few changes which help out GC performance

src/codegen/ast_interpreter.cpp

@@ -30,6 +30,7 @@
 #include "core/ast.h"
 #include "core/cfg.h"
 #include "core/common.h"
+#include "core/contiguous_map.h"
 #include "core/stats.h"
 #include "core/thread_utils.h"
 #include "core/util.h"
@@ -69,7 +70,7 @@ union Value {
 
 class ASTInterpreter {
 public:
-    typedef llvm::DenseMap<InternedString, Box*> SymMap;
+    typedef ContiguousMap<InternedString, Box*> SymMap;
 
     ASTInterpreter(CompiledFunction* compiled_function);
 
@@ -195,10 +196,7 @@ void ASTInterpreter::setFrameInfo(const FrameInfo* frame_info) {
 }
 
 void ASTInterpreter::gcVisit(GCVisitor* visitor) {
-    for (const auto& p2 : getSymbolTable()) {
-        visitor->visitPotential(p2.second);
-    }
-
+    visitor->visitRange((void* const*)&sym_table.vector()[0], (void* const*)&sym_table.vector()[sym_table.size()]);
     if (passed_closure)
         visitor->visit(passed_closure);
     if (created_closure)
@@ -315,7 +313,7 @@ void ASTInterpreter::eraseDeadSymbols() {
                                                 source_info->liveness, scope_info);
 
     std::vector<InternedString> dead_symbols;
-    for (auto&& it : sym_table) {
+    for (auto& it : sym_table) {
         if (!source_info->liveness->isLiveAtEnd(it.first, current_block)) {
             dead_symbols.push_back(it.first);
         } else if (source_info->phis->isRequiredAfter(it.first, current_block)) {
@@ -456,11 +454,11 @@ Value ASTInterpreter::visit_jump(AST_Jump* node) {
                     // TODO only mangle once
                     sorted_symbol_table[getIsDefinedName(name, source_info->getInternedStrings())] = (Box*)is_defined;
                     if (is_defined)
-                        assert(it->second != NULL);
-                    sorted_symbol_table[name] = is_defined ? it->second : NULL;
+                        assert(sym_table.getMapped(it->second) != NULL);
+                    sorted_symbol_table[name] = is_defined ? sym_table.getMapped(it->second) : NULL;
                 } else {
                     ASSERT(it != sym_table.end(), "%s", name.c_str());
-                    sorted_symbol_table[it->first] = it->second;
+                    sorted_symbol_table[it->first] = sym_table.getMapped(it->second);
                 }
             }
 
@@ -1105,7 +1103,7 @@ Value ASTInterpreter::visit_name(AST_Name* node) {
         case ScopeInfo::VarScopeType::CLOSURE: {
             SymMap::iterator it = sym_table.find(node->id);
             if (it != sym_table.end())
-                return it->second;
+                return sym_table.getMapped(it->second);
 
             assertNameDefined(0, node->id.c_str(), UnboundLocalError, true);
             return Value();
@@ -1286,11 +1284,11 @@ BoxedDict* localsForInterpretedFrame(void* frame_ptr, bool only_user_visible) {
     ASTInterpreter* interpreter = s_interpreterMap[frame_ptr];
     assert(interpreter);
     BoxedDict* rtn = new BoxedDict();
-    for (auto&& l : interpreter->getSymbolTable()) {
+    for (auto& l : interpreter->getSymbolTable()) {
         if (only_user_visible && (l.first.str()[0] == '!' || l.first.str()[0] == '#'))
             continue;
 
-        rtn->d[new BoxedString(l.first.str())] = l.second;
+        rtn->d[boxString(l.first.str())] = interpreter->getSymbolTable().getMapped(l.second);
     }
 
     return rtn;

src/core/contiguous_map.h

+// Copyright (c) 2014-2015 Dropbox, Inc.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//    http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+#ifndef PYSTON_CORE_CONTIGUOUSMAP_H
+#define PYSTON_CORE_CONTIGUOUSMAP_H
+
+#include <llvm/ADT/DenseMap.h>
+#include <utility>
+#include <vector>
+
+namespace pyston {
+
+template <class TKey, class TVal, class TMap = llvm::DenseMap<TKey, int>> class ContiguousMap {
+    typedef TMap map_type;
+    typedef std::vector<TVal> vec_type;
+
+    map_type map;
+    vec_type vec;
+
+    std::vector<int> free_list;
+
+public:
+    typedef typename map_type::iterator iterator;
+    typedef typename map_type::const_iterator const_iterator;
+    typedef typename std::vector<TVal>::size_type size_type;
+
+    iterator find(TKey key) { return map.find(key); }
+
+    iterator begin() noexcept { return map.begin(); }
+    iterator end() noexcept { return map.end(); }
+
+    const_iterator begin() const noexcept { return map.begin(); }
+    const_iterator end() const noexcept { return map.end(); }
+
+    iterator erase(const_iterator position) {
+        int idx = map[position->first];
+        free_list.push_back(idx);
+        vec[idx] = TVal();
+        map.erase(position->first);
+        return begin(); // this is broken...
+    }
+
+    size_type erase(const TKey& key) {
+        auto it = map.find(key);
+        if (it == end())
+            return 0;
+        int idx = it->second;
+        free_list.push_back(idx);
+        vec[idx] = TVal();
+        map.erase(it);
+        return 1;
+    }
+
+    size_type count(const TKey& key) const { return map.count(key); }
+
+    TVal& operator[](const TKey& key) {
+        auto it = map.find(key);
+        if (it == map.end()) {
+            int idx;
+            if (free_list.size() > 0) {
+                idx = free_list.back();
+                free_list.pop_back();
+            } else {
+                idx = vec.size();
+            }
+            map[key] = idx;
+            vec.push_back(TVal());
+            return vec[idx];
+        } else {
+            return vec[it->second];
+        }
+    }
+
+    TVal& operator[](TKey&& key) {
+        auto it = map.find(key);
+        if (it == map.end()) {
+            int idx;
+            if (free_list.size() > 0) {
+                idx = free_list.back();
+                free_list.pop_back();
+            } else {
+                idx = vec.size();
+            }
+            map[key] = idx;
+            vec.push_back(TVal());
+            return vec[idx];
+        } else {
+            return vec[it->second];
+        }
+    }
+
+    TVal getMapped(int idx) const { return vec[idx]; }
+
+    size_type size() const { return vec.size(); }
+    const vec_type& vector() { return vec; }
+};
+}
+#endif

src/gc/collector.cpp

@@ -150,6 +150,7 @@ static std::unordered_set<void*> nonheap_roots;
 // typically all have lower addresses than the heap roots, so this can serve as a cheap
 // way to verify it's not a nonheap root (the full check requires a hashtable lookup).
 static void* max_nonheap_root = 0;
+static void* min_nonheap_root = (void*)~0;
 void registerNonheapRootObject(void* obj) {
     // I suppose that things could work fine even if this were true, but why would it happen?
     assert(global_heap.getAllocationFromInteriorPointer(obj) == NULL);
@@ -158,10 +159,13 @@ void registerNonheapRootObject(void* obj) {
     nonheap_roots.insert(obj);
 
     max_nonheap_root = std::max(obj, max_nonheap_root);
+    min_nonheap_root = std::min(obj, min_nonheap_root);
 }
 
 bool isNonheapRoot(void* p) {
-    return p <= max_nonheap_root && nonheap_roots.count(p) != 0;
+    if (p > max_nonheap_root || p < min_nonheap_root)
+        return false;
+    return nonheap_roots.count(p) != 0;
 }
 
 bool isValidGCObject(void* p) {
@@ -187,6 +191,9 @@ bool GCVisitor::isValid(void* p) {
 }
 
 void GCVisitor::visit(void* p) {
+    if (!p)
+        return;
+
     if (isNonheapRoot(p)) {
         return;
     } else {
@@ -252,7 +259,7 @@ void markPhase() {
     }
 
     for (auto h : *getRootHandles()) {
-        visitor.visitPotential(h->value);
+        visitor.visit(h->value);
     }
 
     // if (VERBOSITY()) printf("Found %d roots\n", stack.size());

src/runtime/dict.cpp

@@ -67,6 +67,7 @@ Box* dictCopy(BoxedDict* self) {
 Box* dictItems(BoxedDict* self) {
     BoxedList* rtn = new BoxedList();
 
+    rtn->ensure(self->d.size());
     for (const auto& p : self->d) {
         BoxedTuple::GCVector elts;
         elts.push_back(p.first);
@@ -80,6 +81,7 @@ Box* dictItems(BoxedDict* self) {
 
 Box* dictValues(BoxedDict* self) {
     BoxedList* rtn = new BoxedList();
+    rtn->ensure(self->d.size());
     for (const auto& p : self->d) {
         listAppendInternal(rtn, p.second);
     }
@@ -90,6 +92,7 @@ Box* dictKeys(BoxedDict* self) {
     RELEASE_ASSERT(isSubclass(self->cls, dict_cls), "");
 
     BoxedList* rtn = new BoxedList();
+    rtn->ensure(self->d.size());
     for (const auto& p : self->d) {
         listAppendInternal(rtn, p.first);
     }

src/runtime/objmodel.cpp

@@ -478,7 +478,7 @@ HiddenClass* HiddenClass::getOrMakeChild(const std::string& attr) {
 
     auto it = children.find(attr);
     if (it != children.end())
-        return it->second;
+        return children.getMapped(it->second);
 
     static StatCounter num_hclses("num_hidden_classes");
     num_hclses.log();

src/runtime/traceback.cpp

@@ -96,6 +96,7 @@ Box* BoxedTraceback::getLines(Box* b) {
 
     if (!tb->py_lines) {
         BoxedList* lines = new BoxedList();
+        lines->ensure(tb->lines.size());
         for (auto line : tb->lines) {
             auto l = new BoxedTuple({ boxString(line->file), boxString(line->func), boxInt(line->line) });
             listAppendInternal(lines, l);

src/runtime/types.h

@@ -22,6 +22,7 @@
 #include "structmember.h"
 
 #include "codegen/irgen/future.h"
+#include "core/contiguous_map.h"
 #include "core/threading.h"
 #include "core/types.h"
 #include "gc/gc_alloc.h"
@@ -286,7 +287,7 @@ private:
 
     // Only makes sense for NORMAL hidden classes.  Clients should access through getAttrOffsets():
     llvm::StringMap<int> attr_offsets;
-    llvm::StringMap<HiddenClass*> children;
+    ContiguousMap<llvm::StringRef, HiddenClass*, llvm::StringMap<int>> children;
 
 public:
     static HiddenClass* makeRoot() {
@@ -308,12 +309,9 @@ public:
 
     void gc_visit(GCVisitor* visitor) {
         // Visit children even for the dict-backed case, since children will just be empty
-        for (const auto& p : children) {
-            visitor->visit(p.second);
-        }
+        visitor->visitRange((void* const*)&children.vector()[0], (void* const*)&children.vector()[children.size()]);
     }
 
-
     // Only makes sense for NORMAL hidden classes:
     const llvm::StringMap<int>& getAttrOffsets() {
         assert(type == NORMAL);