Merge pull request #278 from undingen/interpreter_speed

Small performance improvement when interpreting

src/analysis/scoping_analysis.cpp

@@ -14,6 +14,8 @@
 
 #include "analysis/scoping_analysis.h"
 
+#include "llvm/ADT/DenseSet.h"
+
 #include "core/ast.h"
 #include "core/common.h"
 #include "core/util.h"
@@ -112,7 +114,7 @@ struct ScopingAnalysis::ScopeNameUsage {
     const std::string* private_name;
     ScopingAnalysis* scoping;
 
-    typedef std::unordered_set<InternedString> StrSet;
+    typedef llvm::DenseSet<InternedString> StrSet;
 
     // Properties determined from crawling the scope:
     StrSet read;

src/codegen/ast_interpreter.cpp

@@ -14,6 +14,7 @@
 
 #include "codegen/ast_interpreter.h"
 
+#include <llvm/ADT/DenseMap.h>
 #include <llvm/ADT/StringMap.h>
 #include <unordered_map>
 
@@ -42,8 +43,10 @@
 
 namespace pyston {
 
-#define OSR_THRESHOLD 100
-#define REOPT_THRESHOLD 10
+namespace {
+
+#define OSR_THRESHOLD 200
+#define REOPT_THRESHOLD 100
 
 union Value {
     bool b;
@@ -59,7 +62,7 @@ union Value {
 
 class ASTInterpreter {
 public:
-    typedef std::unordered_map<InternedString, Box*> SymMap;
+    typedef llvm::DenseMap<InternedString, Box*> SymMap;
 
     ASTInterpreter(CompiledFunction* compiled_function);
 
@@ -142,33 +145,6 @@ public:
     void gcVisit(GCVisitor* visitor);
 };
 
-const void* interpreter_instr_addr = (void*)&ASTInterpreter::execute;
-
-static_assert(THREADING_USE_GIL, "have to make the interpreter map thread safe!");
-static std::unordered_map<void*, ASTInterpreter*> s_interpreterMap;
-
-Box* astInterpretFunction(CompiledFunction* cf, int nargs, Box* closure, Box* generator, Box* arg1, Box* arg2,
-                          Box* arg3, Box** args) {
-    if (unlikely(cf->times_called > REOPT_THRESHOLD)) {
-        CompiledFunction* optimized = reoptCompiledFuncInternal(cf);
-        if (closure && generator)
-            return optimized->closure_generator_call((BoxedClosure*)closure, (BoxedGenerator*)generator, arg1, arg2,
-                                                     arg3, args);
-        else if (closure)
-            return optimized->closure_call((BoxedClosure*)closure, arg1, arg2, arg3, args);
-        else if (generator)
-            return optimized->generator_call((BoxedGenerator*)generator, arg1, arg2, arg3, args);
-        return optimized->call(arg1, arg2, arg3, args);
-    }
-
-    ++cf->times_called;
-    ASTInterpreter interpreter(cf);
-
-    interpreter.initArguments(nargs, (BoxedClosure*)closure, (BoxedGenerator*)generator, arg1, arg2, arg3, args);
-    Value v = ASTInterpreter::execute(interpreter);
-
-    return v.o ? v.o : None;
-}
 
 void ASTInterpreter::addSymbol(InternedString name, Box* value, bool allow_duplicates) {
     if (!allow_duplicates)
@@ -176,107 +152,6 @@ void ASTInterpreter::addSymbol(InternedString name, Box* value, bool allow_dupli
     sym_table[name] = value;
 }
 
-Box* astInterpretFrom(CompiledFunction* cf, AST_expr* after_expr, AST_stmt* enclosing_stmt, Box* expr_val,
-                      BoxedDict* locals) {
-    assert(cf);
-    assert(enclosing_stmt);
-    assert(locals);
-    assert(after_expr);
-    assert(expr_val);
-
-    ASTInterpreter interpreter(cf);
-
-    for (const auto& p : locals->d) {
-        assert(p.first->cls == str_cls);
-        auto name = static_cast<BoxedString*>(p.first)->s;
-        InternedString interned = cf->clfunc->source->getInternedStrings().get(name);
-        interpreter.addSymbol(interned, p.second, false);
-    }
-
-    CFGBlock* start_block = NULL;
-    AST_stmt* starting_statement = NULL;
-    while (true) {
-        if (enclosing_stmt->type == AST_TYPE::Assign) {
-            auto asgn = ast_cast<AST_Assign>(enclosing_stmt);
-            assert(asgn->value == after_expr);
-            assert(asgn->targets.size() == 1);
-            assert(asgn->targets[0]->type == AST_TYPE::Name);
-            auto name = ast_cast<AST_Name>(asgn->targets[0]);
-            assert(name->id.str()[0] == '#');
-            interpreter.addSymbol(name->id, expr_val, true);
-            break;
-        } else if (enclosing_stmt->type == AST_TYPE::Expr) {
-            auto expr = ast_cast<AST_Expr>(enclosing_stmt);
-            assert(expr->value == after_expr);
-            break;
-        } else if (enclosing_stmt->type == AST_TYPE::Invoke) {
-            auto invoke = ast_cast<AST_Invoke>(enclosing_stmt);
-            start_block = invoke->normal_dest;
-            starting_statement = start_block->body[0];
-            enclosing_stmt = invoke->stmt;
-        } else {
-            RELEASE_ASSERT(0, "should not be able to reach here with anything other than an Assign (got %d)",
-                           enclosing_stmt->type);
-        }
-    }
-
-    if (start_block == NULL) {
-        // TODO innefficient
-        for (auto block : cf->clfunc->source->cfg->blocks) {
-            int n = block->body.size();
-            for (int i = 0; i < n; i++) {
-                if (block->body[i] == enclosing_stmt) {
-                    ASSERT(i + 1 < n, "how could we deopt from a non-invoke terminator?");
-                    start_block = block;
-                    starting_statement = block->body[i + 1];
-                    break;
-                }
-            }
-
-            if (start_block)
-                break;
-        }
-
-        ASSERT(start_block, "was unable to find the starting block??");
-        assert(starting_statement);
-    }
-
-    Value v = ASTInterpreter::execute(interpreter, start_block, starting_statement);
-
-    return v.o ? v.o : None;
-}
-
-AST_stmt* getCurrentStatementForInterpretedFrame(void* frame_ptr) {
-    ASTInterpreter* interpreter = s_interpreterMap[frame_ptr];
-    assert(interpreter);
-    return interpreter->getCurrentStatement();
-}
-
-CompiledFunction* getCFForInterpretedFrame(void* frame_ptr) {
-    ASTInterpreter* interpreter = s_interpreterMap[frame_ptr];
-    assert(interpreter);
-    return interpreter->getCF();
-}
-
-FrameInfo* getFrameInfoForInterpretedFrame(void* frame_ptr) {
-    ASTInterpreter* interpreter = s_interpreterMap[frame_ptr];
-    assert(interpreter);
-    return interpreter->getFrameInfo();
-}
-
-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()) {
-        if (only_user_visible && (l.first.str()[0] == '!' || l.first.str()[0] == '#'))
-            continue;
-
-        rtn->d[new BoxedString(l.first.str())] = l.second;
-    }
-    return rtn;
-}
-
 void ASTInterpreter::gcVisit(GCVisitor* visitor) {
     for (const auto& p2 : getSymbolTable()) {
         visitor->visitPotential(p2.second);
@@ -290,13 +165,6 @@ void ASTInterpreter::gcVisit(GCVisitor* visitor) {
         visitor->visit(generator);
 }
 
-void gatherInterpreterRoots(GCVisitor* visitor) {
-    for (const auto& p : s_interpreterMap) {
-        p.second->gcVisit(visitor);
-    }
-}
-
-
 ASTInterpreter::ASTInterpreter(CompiledFunction* compiled_function)
     : compiled_func(compiled_function), source_info(compiled_function->clfunc->source), scope_info(0), current_block(0),
       current_inst(0), last_exception(NULL, NULL, NULL), passed_closure(0), created_closure(0), generator(0),
@@ -339,7 +207,9 @@ void ASTInterpreter::initArguments(int nargs, BoxedClosure* _closure, BoxedGener
     }
 }
 
-namespace {
+static std::unordered_map<void*, ASTInterpreter*> s_interpreterMap;
+static_assert(THREADING_USE_GIL, "have to make the interpreter map thread safe!");
+
 class RegisterHelper {
 private:
     void* frame_addr;
@@ -353,7 +223,6 @@ public:
         s_interpreterMap.erase(frame_addr);
     }
 };
-}
 
 Value ASTInterpreter::execute(ASTInterpreter& interpreter, CFGBlock* start_block, AST_stmt* start_at) {
     threading::allowGLReadPreemption();
@@ -923,6 +792,8 @@ Value ASTInterpreter::visit_delete(AST_Delete* node) {
 }
 
 Value ASTInterpreter::visit_assign(AST_Assign* node) {
+    assert(node->targets.size() == 1 && "cfg should have lowered it to a single target");
+
     Value v = visit_expr(node->value);
     for (AST_expr* e : node->targets)
         doStore(e, v);
@@ -936,6 +807,7 @@ Value ASTInterpreter::visit_print(AST_Print* node) {
 
     Box* dest = node->dest ? visit_expr(node->dest).o : getSysStdout();
     int nvals = node->values.size();
+    assert(nvals <= 1 && "cfg should have lowered it to 0 or 1 values");
     for (int i = 0; i < nvals; i++) {
         Box* var = visit_expr(node->values[i]).o;
 
@@ -1204,3 +1076,137 @@ Value ASTInterpreter::visit_attribute(AST_Attribute* node) {
     return getattr(visit_expr(node->value).o, node->attr.c_str());
 }
 }
+
+const void* interpreter_instr_addr = (void*)&ASTInterpreter::execute;
+
+Box* astInterpretFunction(CompiledFunction* cf, int nargs, Box* closure, Box* generator, Box* arg1, Box* arg2,
+                          Box* arg3, Box** args) {
+    if (unlikely(cf->times_called > REOPT_THRESHOLD)) {
+        CompiledFunction* optimized = reoptCompiledFuncInternal(cf);
+        if (closure && generator)
+            return optimized->closure_generator_call((BoxedClosure*)closure, (BoxedGenerator*)generator, arg1, arg2,
+                                                     arg3, args);
+        else if (closure)
+            return optimized->closure_call((BoxedClosure*)closure, arg1, arg2, arg3, args);
+        else if (generator)
+            return optimized->generator_call((BoxedGenerator*)generator, arg1, arg2, arg3, args);
+        return optimized->call(arg1, arg2, arg3, args);
+    }
+
+    ++cf->times_called;
+    ASTInterpreter interpreter(cf);
+
+    interpreter.initArguments(nargs, (BoxedClosure*)closure, (BoxedGenerator*)generator, arg1, arg2, arg3, args);
+    Value v = ASTInterpreter::execute(interpreter);
+
+    return v.o ? v.o : None;
+}
+
+
+Box* astInterpretFrom(CompiledFunction* cf, AST_expr* after_expr, AST_stmt* enclosing_stmt, Box* expr_val,
+                      BoxedDict* locals) {
+    assert(cf);
+    assert(enclosing_stmt);
+    assert(locals);
+    assert(after_expr);
+    assert(expr_val);
+
+    ASTInterpreter interpreter(cf);
+
+    for (const auto& p : locals->d) {
+        assert(p.first->cls == str_cls);
+        auto name = static_cast<BoxedString*>(p.first)->s;
+        InternedString interned = cf->clfunc->source->getInternedStrings().get(name);
+        interpreter.addSymbol(interned, p.second, false);
+    }
+
+    CFGBlock* start_block = NULL;
+    AST_stmt* starting_statement = NULL;
+    while (true) {
+        if (enclosing_stmt->type == AST_TYPE::Assign) {
+            auto asgn = ast_cast<AST_Assign>(enclosing_stmt);
+            assert(asgn->value == after_expr);
+            assert(asgn->targets.size() == 1);
+            assert(asgn->targets[0]->type == AST_TYPE::Name);
+            auto name = ast_cast<AST_Name>(asgn->targets[0]);
+            assert(name->id.str()[0] == '#');
+            interpreter.addSymbol(name->id, expr_val, true);
+            break;
+        } else if (enclosing_stmt->type == AST_TYPE::Expr) {
+            auto expr = ast_cast<AST_Expr>(enclosing_stmt);
+            assert(expr->value == after_expr);
+            break;
+        } else if (enclosing_stmt->type == AST_TYPE::Invoke) {
+            auto invoke = ast_cast<AST_Invoke>(enclosing_stmt);
+            start_block = invoke->normal_dest;
+            starting_statement = start_block->body[0];
+            enclosing_stmt = invoke->stmt;
+        } else {
+            RELEASE_ASSERT(0, "should not be able to reach here with anything other than an Assign (got %d)",
+                           enclosing_stmt->type);
+        }
+    }
+
+    if (start_block == NULL) {
+        // TODO innefficient
+        for (auto block : cf->clfunc->source->cfg->blocks) {
+            int n = block->body.size();
+            for (int i = 0; i < n; i++) {
+                if (block->body[i] == enclosing_stmt) {
+                    ASSERT(i + 1 < n, "how could we deopt from a non-invoke terminator?");
+                    start_block = block;
+                    starting_statement = block->body[i + 1];
+                    break;
+                }
+            }
+
+            if (start_block)
+                break;
+        }
+
+        ASSERT(start_block, "was unable to find the starting block??");
+        assert(starting_statement);
+    }
+
+    Value v = ASTInterpreter::execute(interpreter, start_block, starting_statement);
+
+    return v.o ? v.o : None;
+}
+
+AST_stmt* getCurrentStatementForInterpretedFrame(void* frame_ptr) {
+    ASTInterpreter* interpreter = s_interpreterMap[frame_ptr];
+    assert(interpreter);
+    return interpreter->getCurrentStatement();
+}
+
+CompiledFunction* getCFForInterpretedFrame(void* frame_ptr) {
+    ASTInterpreter* interpreter = s_interpreterMap[frame_ptr];
+    assert(interpreter);
+    return interpreter->getCF();
+}
+
+FrameInfo* getFrameInfoForInterpretedFrame(void* frame_ptr) {
+    ASTInterpreter* interpreter = s_interpreterMap[frame_ptr];
+    assert(interpreter);
+    return interpreter->getFrameInfo();
+}
+
+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()) {
+        if (only_user_visible && (l.first.str()[0] == '!' || l.first.str()[0] == '#'))
+            continue;
+
+        rtn->d[new BoxedString(l.first.str())] = l.second;
+    }
+    return rtn;
+}
+
+void gatherInterpreterRoots(GCVisitor* visitor) {
+    for (const auto& p : s_interpreterMap) {
+        p.second->gcVisit(visitor);
+    }
+}
+}

src/core/stringpool.h

@@ -19,6 +19,7 @@
 #include <cstdio>
 #include <sys/time.h>
 
+#include "llvm/ADT/DenseMapInfo.h"
 #include "llvm/ADT/Hashing.h"
 #include "llvm/ADT/StringRef.h"
 
@@ -78,6 +79,7 @@ public:
     friend class InternedStringPool;
     friend struct std::hash<InternedString>;
     friend struct std::less<InternedString>;
+    friend struct llvm::DenseMapInfo<pyston::InternedString>;
 };
 
 class InternedStringPool {
@@ -130,4 +132,18 @@ template <> struct less<pyston::InternedString> {
 };
 }
 
+template <> struct llvm::DenseMapInfo<pyston::InternedString> {
+    static inline pyston::InternedString getEmptyKey() { return pyston::InternedString(); }
+    static inline pyston::InternedString getTombstoneKey() {
+        pyston::InternedString str;
+        str._str = (const std::string*)-1;
+        return str;
+    }
+    static unsigned getHashValue(const pyston::InternedString& val) { return std::hash<pyston::InternedString>()(val); }
+    static bool isEqual(const pyston::InternedString& lhs, const pyston::InternedString& rhs) {
+        // Have to reimplement InternedString comparison otherwise asserts would trigger because of the empty keys.
+        return lhs._str == rhs._str;
+    }
+};
+
 #endif