Optimize LivenessAnalysis

We were caching certain liveness results (only crawled each BB once
looking for name use/defs), but we were still traversing the entire
CFG for every liveness query.
Now, each query does a single full CFG scan, and computes the results
for all BBs and stores them.

Reduces the time to 'import re' from about 13s to under 2s.

src/analysis/function_analysis.cpp

@@ -33,27 +33,31 @@ namespace pyston {
 
 class LivenessBBVisitor : public NoopASTVisitor {
 public:
-    typedef llvm::SmallSet<std::string, 4> StrSet;
+    enum Status {
+        NONE,
+        USED,
+        KILLED,
+    };
 
 private:
-    StrSet _loads;
-    StrSet _stores;
+    std::unordered_map<int, Status> statuses;
+    LivenessAnalysis* analysis;
 
     void _doLoad(const std::string& name) {
-        if (_stores.count(name))
-            return;
-        _loads.insert(name);
+        Status& status = statuses[analysis->getStringIndex(name)];
+        if (status == NONE)
+            status = USED;
     }
     void _doStore(const std::string& name) {
-        if (_loads.count(name))
-            return;
-        _stores.insert(name);
+        Status& status = statuses[analysis->getStringIndex(name)];
+        if (status == NONE)
+            status = KILLED;
     }
 
 public:
-    LivenessBBVisitor() {}
-    const StrSet& loads() { return _loads; }
-    const StrSet& stores() { return _stores; }
+    LivenessBBVisitor(LivenessAnalysis* analysis) : analysis(analysis) {}
+
+    Status nameStatus(int idx) { return statuses[idx]; }
 
     bool visit_classdef(AST_ClassDef* node) {
         _doStore(node->name);
@@ -94,6 +98,24 @@ public:
     }
 };
 
+int LivenessAnalysis::getStringIndex(const std::string& s) {
+    int& r = string_index_map[s];
+    if (r == 0) {
+        r = string_index_map.size(); // includes the '0' entry we just put in there
+    }
+    return r;
+}
+
+LivenessAnalysis::LivenessAnalysis(CFG* cfg) : cfg(cfg) {
+    for (CFGBlock* b : cfg->blocks) {
+        auto visitor = new LivenessBBVisitor(this); // livenessCache unique_ptr will delete it.
+        for (AST_stmt* stmt : b->body) {
+            stmt->accept(visitor);
+        }
+        liveness_cache.insert(std::make_pair(b, std::unique_ptr<LivenessBBVisitor>(visitor)));
+    }
+}
+
 bool LivenessAnalysis::isLiveAtEnd(const std::string& name, CFGBlock* block) {
     if (name[0] != '#')
         return true;
@@ -101,50 +123,43 @@ bool LivenessAnalysis::isLiveAtEnd(const std::string& name, CFGBlock* block) {
     if (block->successors.size() == 0)
         return false;
 
-    // Very inefficient liveness analysis:
-    // for each query, trace forward through all possible control flow paths.
-    // if we hit a store to the name, stop tracing that path
-    // if we hit a load to the name, return true.
-    // to improve performance we cache the liveness result of every visited BB.
-    llvm::SmallPtrSet<CFGBlock*, 1> visited;
-    std::deque<CFGBlock*> q;
-    for (CFGBlock* successor : block->successors) {
-        q.push_back(successor);
-    }
+    int idx = getStringIndex(name);
+    if (!result_cache.count(idx)) {
+        std::unordered_map<CFGBlock*, bool>& map = result_cache[idx];
+
+        // Approach:
+        // - Find all uses (blocks where the status is USED)
+        // - Trace backwards, marking all blocks as live-at-end
+        // - If we hit a block that is KILLED, stop
+        for (CFGBlock* b : cfg->blocks) {
+            auto status = liveness_cache[b]->nameStatus(idx);
 
-    while (q.size()) {
-        CFGBlock* thisblock = q.front();
-        q.pop_front();
-        if (visited.count(thisblock))
-            continue;
-
-        LivenessBBVisitor* visitor = nullptr;
-        LivenessCacheMap::iterator it = livenessCache.find(thisblock);
-        if (it != livenessCache.end()) {
-            visitor = it->second.get();
-        } else {
-            visitor = new LivenessBBVisitor; // livenessCache unique_ptr will delete it.
-            for (AST_stmt* stmt : thisblock->body) {
-                stmt->accept(visitor);
+            if (status != LivenessBBVisitor::USED)
+                continue;
+
+            std::deque<CFGBlock*> q;
+            for (CFGBlock* pred : b->predecessors) {
+                q.push_back(pred);
             }
-            livenessCache.insert(std::make_pair(thisblock, std::unique_ptr<LivenessBBVisitor>(visitor)));
-        }
-        visited.insert(thisblock);
 
-        if (visitor->loads().count(name)) {
-            assert(!visitor->stores().count(name));
-            return true;
-        }
+            while (q.size()) {
+                CFGBlock* thisblock = q.front();
+                q.pop_front();
 
-        if (!visitor->stores().count(name)) {
-            assert(!visitor->loads().count(name));
-            for (CFGBlock* successor : thisblock->successors) {
-                q.push_back(successor);
+                if (map[thisblock])
+                    continue;
+
+                map[thisblock] = true;
+                if (liveness_cache[thisblock]->nameStatus(idx) != LivenessBBVisitor::KILLED) {
+                    for (CFGBlock* pred : thisblock->predecessors) {
+                        q.push_back(pred);
+                    }
+                }
             }
         }
     }
 
-    return false;
+    return result_cache[idx][block];
 }
 
 class DefinednessBBAnalyzer : public BBAnalyzer<DefinednessAnalysis::DefinitionLevel> {
@@ -375,8 +390,8 @@ bool PhiAnalysis::isPotentiallyUndefinedAfter(const std::string& name, CFGBlock*
     return false;
 }
 
-LivenessAnalysis* computeLivenessInfo(CFG*) {
-    return new LivenessAnalysis();
+LivenessAnalysis* computeLivenessInfo(CFG* cfg) {
+    return new LivenessAnalysis(cfg);
 }
 
 PhiAnalysis* computeRequiredPhis(const SourceInfo::ArgNames& args, CFG* cfg, LivenessAnalysis* liveness,

src/analysis/function_analysis.h

@@ -31,13 +31,31 @@ class ScopeInfo;
 class LivenessBBVisitor;
 
 class LivenessAnalysis {
-public:
-    bool isLiveAtEnd(const std::string& name, CFGBlock* block);
-
 private:
+    CFG* cfg;
+
+    friend class LivenessBBVisitor;
     typedef std::unordered_map<CFGBlock*, std::unique_ptr<LivenessBBVisitor> > LivenessCacheMap;
-    LivenessCacheMap livenessCache;
+    LivenessCacheMap liveness_cache;
+
+    std::unordered_map<int, std::unordered_map<CFGBlock*, bool> > result_cache;
+
+    // Map strings to unique indices.  For a given CFG, the set of strings should be fairly small
+    // (a constant fraction max of the CFG itself), so just store all of them.  The theory is that
+    // for any particular name, we will do many lookups on it in different hash tables, and by
+    // converting to a string only once, the extra hashtable lookup will be profitable since it
+    // can make all the rest faster (int hashes vs string hashes).
+    //
+    // Haven't validated this, though.
+    std::unordered_map<std::string, int> string_index_map;
+    int getStringIndex(const std::string& s);
+
+public:
+    LivenessAnalysis(CFG* cfg);
+
+    bool isLiveAtEnd(const std::string& name, CFGBlock* block);
 };
+
 class DefinednessAnalysis {
 public:
     enum DefinitionLevel {

src/core/cfg.cpp

@@ -23,6 +23,7 @@
 #include "core/ast.h"
 #include "core/options.h"
 #include "core/types.h"
+#include "runtime/objmodel.h"
 #include "runtime/types.h"
 
 //#undef VERBOSITY
@@ -1266,8 +1267,7 @@ public:
 
     virtual bool visit_return(AST_Return* node) {
         if (root_type != AST_TYPE::FunctionDef && root_type != AST_TYPE::Lambda) {
-            fprintf(stderr, "SyntaxError: 'return' outside function\n");
-            exit(1);
+            raiseExcHelper(SyntaxError, "'return' outside function");
         }
 
         AST_expr* value = remapExpr(node->value);
@@ -1337,8 +1337,7 @@ public:
             return true;
 
         if (loops.size() == 0) {
-            fprintf(stderr, "SyntaxError: 'break' outside loop\n");
-            exit(1);
+            raiseExcHelper(SyntaxError, "'break' outside loop");
         }
 
         AST_Jump* j = makeJump();
@@ -1357,8 +1356,7 @@ public:
 
         if (loops.size() == 0) {
             // Note: error message is different than the 'break' case
-            fprintf(stderr, "SyntaxError: 'continue' not properly in loop\n");
-            exit(1);
+            raiseExcHelper(SyntaxError, "'continue' not properly in loop");
         }
 
         AST_Jump* j = makeJump();

src/jit.cpp

@@ -165,6 +165,7 @@ int main(int argc, char** argv) {
                 std::string msg = formatException(b);
                 printLastTraceback();
                 fprintf(stderr, "%s\n", msg.c_str());
+
                 exit(1);
             }
         }

src/runtime/builtin_modules/builtins.cpp

@@ -382,7 +382,7 @@ BoxedModule* builtins_module;
 // TODO looks like CPython and pypy put this into an "exceptions" module:
 BoxedClass* Exception, *AssertionError, *AttributeError, *GeneratorExit, *TypeError, *NameError, *KeyError, *IndexError,
     *IOError, *OSError, *ZeroDivisionError, *ValueError, *UnboundLocalError, *RuntimeError, *ImportError,
-    *StopIteration, *Warning;
+    *StopIteration, *Warning, *SyntaxError;
 
 const ObjectFlavor exception_flavor(&boxGCHandler, NULL);
 Box* exceptionNew1(BoxedClass* cls) {
@@ -472,6 +472,7 @@ void setupBuiltins() {
     ImportError = makeBuiltinException(Exception, "ImportError");
     StopIteration = makeBuiltinException(Exception, "StopIteration");
     Warning = makeBuiltinException(Exception, "Warning");
+    SyntaxError = makeBuiltinException(Exception, "SyntaxError");
     /*ImportWarning =*/makeBuiltinException(Warning, "ImportWarning");
     /*PendingDeprecationWarning =*/makeBuiltinException(Warning, "PendingDeprecationWarning");
     /*DeprecationWarning =*/makeBuiltinException(Warning, "DeprecationWarning");

src/runtime/types.h

@@ -356,7 +356,7 @@ Box* exceptionNew2(BoxedClass* cls, Box* message);
 
 extern BoxedClass* Exception, *AssertionError, *AttributeError, *TypeError, *NameError, *KeyError, *IndexError,
     *IOError, *OSError, *ZeroDivisionError, *ValueError, *UnboundLocalError, *RuntimeError, *ImportError,
-    *StopIteration, *GeneratorExit;
+    *StopIteration, *GeneratorExit, *SyntaxError;
 
 // cls should be obj->cls.
 // Added as parameter because it should typically be available