Bug 1310155 - IonMonkey, part 1.1: Use a separate allocator for all control flow graphs, r=jandem
authorHannes Verschore <hv1989@gmail.com>
Thu, 08 Dec 2016 13:53:07 -1000
changeset 325459 2f96ca59484d38b4b6d2f3aca283001bc7f01029
parent 325458 3daa33144b64225d2a36c4eb579a682a9ecfc6b3
child 325460 8faf0055fb54eb3b5394b5a92e7fbc121a0897b5
push id24
push usermaklebus@msu.edu
push dateTue, 20 Dec 2016 03:11:33 +0000
reviewersjandem
bugs1310155
milestone53.0a1
Bug 1310155 - IonMonkey, part 1.1: Use a separate allocator for all control flow graphs, r=jandem
js/src/gc/Zone.cpp
js/src/jit/BaselineJIT.cpp
js/src/jit/IonBuilder.cpp
js/src/jit/IonControlFlow.cpp
js/src/jit/IonControlFlow.h
js/src/jit/JitCompartment.h
--- a/js/src/gc/Zone.cpp
+++ b/js/src/gc/Zone.cpp
@@ -238,28 +238,42 @@ Zone::discardJitCode(FreeOp* fop, bool d
                 jit::FinishDiscardBaselineScript(fop, script);
 
             /*
              * Warm-up counter for scripts are reset on GC. After discarding code we
              * need to let it warm back up to get information such as which
              * opcodes are setting array holes or accessing getter properties.
              */
             script->resetWarmUpCounter();
+
+            /*
+             * Make it impossible to use the control flow graphs cached on the
+             * BaselineScript. They get deleted.
+             */
+            if (script->hasBaselineScript())
+                script->baselineScript()->setControlFlowGraph(nullptr);
         }
 
         /*
          * When scripts contains pointers to nursery things, the store buffer
          * can contain entries that point into the optimized stub space. Since
          * this method can be called outside the context of a GC, this situation
          * could result in us trying to mark invalid store buffer entries.
          *
          * Defer freeing any allocated blocks until after the next minor GC.
          */
         if (discardBaselineCode)
             jitZone()->optimizedStubSpace()->freeAllAfterMinorGC(fop->runtime());
+
+        /*
+         * Free all control flow graphs that are cached on BaselineScripts.
+         * Assuming this happens on the mainthread and all control flow
+         * graph reads happen on the mainthread, this is save.
+         */
+        jitZone()->cfgSpace()->lifoAlloc().freeAll();
     }
 }
 
 #ifdef JSGC_HASH_TABLE_CHECKS
 void
 JS::Zone::checkUniqueIdTableAfterMovingGC()
 {
     for (UniqueIdMap::Enum e(uniqueIds_); !e.empty(); e.popFront())
--- a/js/src/jit/BaselineJIT.cpp
+++ b/js/src/jit/BaselineJIT.cpp
@@ -496,18 +496,16 @@ BaselineScript::Trace(JSTracer* trc, Bas
     script->trace(trc);
 }
 
 void
 BaselineScript::Destroy(FreeOp* fop, BaselineScript* script)
 {
 
     MOZ_ASSERT(!script->hasPendingIonBuilder());
-    fop->delete_(script->controlFlowGraph_);
-    script->controlFlowGraph_ = nullptr;
 
     script->unlinkDependentWasmImports(fop);
 
     /*
      * When the script contains pointers to nursery things, the store buffer can
      * contain entries that point into the fallback stub space. Since we can
      * destroy scripts outside the context of a GC, this situation could result
      * in us trying to mark invalid store buffer entries.
--- a/js/src/jit/IonBuilder.cpp
+++ b/js/src/jit/IonBuilder.cpp
@@ -1357,79 +1357,66 @@ IonBuilder::maybeAddOsrTypeBarriers()
 }
 
 enum class CFGState : uint32_t {
     Alloc = 0,
     Abort = 1,
     Success = 2
 };
 
-// Keeps track of the lifetime of the created CFG.
-// If possible it will forward the lifetime to the baselineScript,
-// which will take care of removing the memory if needed.
-// Else this class will remove it at destruction.
-class AutoControlFlowGraph
-{
-    TempAllocator& alloc;
-    JSScript* script;
-    ControlFlowGraph* cfg_;
-
-  public:
-    AutoControlFlowGraph(TempAllocator& alloc, JSScript* script)
-      : alloc(alloc),
-        script(script),
-        cfg_(nullptr)
-    { }
-
-    CFGState getOrCreate(const ControlFlowGraph** cfg_out) {
-        if (script->hasBaselineScript() && script->baselineScript()->controlFlowGraph()) {
-            *cfg_out = script->baselineScript()->controlFlowGraph();
-            return CFGState::Success;
-        }
-
-        ControlFlowGenerator cfgenerator(alloc, script);
-        if (!cfgenerator.init())
-            return CFGState::Alloc;
-
-        if (!cfgenerator.traverseBytecode()) {
-            if (cfgenerator.aborted())
-                return CFGState::Abort;
+static CFGState
+GetOrCreateControlFlowGraph(TempAllocator& tempAlloc, JSScript* script,
+                            const ControlFlowGraph** cfgOut)
+{
+    if (script->hasBaselineScript() && script->baselineScript()->controlFlowGraph()) {
+        *cfgOut = script->baselineScript()->controlFlowGraph();
+        return CFGState::Success;
+    }
+
+    ControlFlowGenerator cfgenerator(tempAlloc, script);
+    if (!cfgenerator.init())
+        return CFGState::Alloc;
+
+    if (!cfgenerator.traverseBytecode()) {
+        if (cfgenerator.aborted())
+            return CFGState::Abort;
+        return CFGState::Alloc;
+    }
+
+    // If possible cache the control flow graph on the baseline script.
+    TempAllocator* graphAlloc = nullptr;
+    if (script->hasBaselineScript()) {
+        LifoAlloc& lifoAlloc = script->zone()->jitZone()->cfgSpace()->lifoAlloc();
+        LifoAlloc::AutoFallibleScope fallibleAllocator(&lifoAlloc);
+        graphAlloc = lifoAlloc.new_<TempAllocator>(&lifoAlloc);
+        if (!graphAlloc)
             return CFGState::Alloc;
-        }
-
-        cfg_ = cfgenerator.getGraph();
-        if (!cfg_)
-            return CFGState::Alloc;
-
-        if (script->hasBaselineScript())
-            script->baselineScript()->setControlFlowGraph(cfg_);
-
-        if (JitSpewEnabled(JitSpew_CFG)) {
-            JitSpew(JitSpew_CFG, "Generating graph for %s:%" PRIuSIZE,
-                                 script->filename(), script->lineno());
-            Fprinter& print = JitSpewPrinter();
-            cfg_->dump(print, script);
-        }
-
-        *cfg_out = cfg_;
-        return CFGState::Success;
-    }
-
-    ~AutoControlFlowGraph() {
-        if (!cfg_)
-            return;
-
-        if (script->hasBaselineScript()) {
-            MOZ_ASSERT(script->baselineScript()->controlFlowGraph() == cfg_);
-            return;
-        }
-
-        js_delete(cfg_);
-    }
-};
+    } else {
+        graphAlloc = &tempAlloc;
+    }
+
+    ControlFlowGraph* cfg = cfgenerator.getGraph(*graphAlloc);
+    if (!cfg)
+        return CFGState::Alloc;
+
+    if (script->hasBaselineScript()) {
+        MOZ_ASSERT(!script->baselineScript()->controlFlowGraph());
+        script->baselineScript()->setControlFlowGraph(cfg);
+    }
+
+    if (JitSpewEnabled(JitSpew_CFG)) {
+        JitSpew(JitSpew_CFG, "Generating graph for %s:%" PRIuSIZE,
+                             script->filename(), script->lineno());
+        Fprinter& print = JitSpewPrinter();
+        cfg->dump(print, script);
+    }
+
+    *cfgOut = cfg;
+    return CFGState::Success;
+}
 
 // We traverse the bytecode using the control flow graph. This structure contains
 // a graph of CFGBlocks in RPO order.
 //
 // Per CFGBlock we take the corresponding MBasicBlock and start iterating the
 // bytecode of that CFGBlock. Each basic block has a mapping of local slots to
 // instructions, as well as a stack depth. As we encounter instructions we
 // mutate this mapping in the current block.
@@ -1440,18 +1427,19 @@ class AutoControlFlowGraph
 // around loops ...
 //
 // We keep a link between a CFGBlock and the entry MBasicBlock (in blockWorklist).
 // That vector only contains the MBasicBlocks that correspond with a CFGBlock.
 // We can create new MBasicBlocks that don't correspond to a CFGBlock.
 bool
 IonBuilder::traverseBytecode()
 {
-    AutoControlFlowGraph autoCFG(alloc(), info().script());
-    CFGState state = autoCFG.getOrCreate(&cfg);
+    CFGState state = GetOrCreateControlFlowGraph(alloc(), info().script(), &cfg);
+    MOZ_ASSERT_IF(cfg && info().script()->hasBaselineScript(),
+                  info().script()->baselineScript()->controlFlowGraph() == cfg);
     if (state == CFGState::Alloc) {
         abortReason_ = AbortReason_Alloc;
         return false;
     }
     if (state == CFGState::Abort)
         return abort("Couldn't create the CFG of script");
 
     if (!blockWorklist.growBy(cfg->numBlocks())) {
--- a/js/src/jit/IonControlFlow.cpp
+++ b/js/src/jit/IonControlFlow.cpp
@@ -73,93 +73,93 @@ ControlFlowGraph::dump(GenericPrinter& p
                 print.printf(", ");
             print.printf("%" PRIuSIZE, blocks_[i].stopIns()->getSuccessor(j)->id());
         }
         print.printf("]\n\n");
     }
 }
 
 bool
-ControlFlowGraph::init(const CFGBlockVector& blocks)
+ControlFlowGraph::init(TempAllocator& alloc, const CFGBlockVector& blocks)
 {
     if (!blocks_.reserve(blocks.length()))
         return false;
 
     for (size_t i = 0; i < blocks.length(); i++) {
         MOZ_ASSERT(blocks[i]->id() == i);
         CFGBlock block(blocks[i]->startPc());
 
         block.setStopPc(blocks[i]->stopPc());
         block.setId(i);
         blocks_.infallibleAppend(mozilla::Move(block));
     }
 
     for (size_t i = 0; i < blocks.length(); i++) {
-        if (!alloc().ensureBallast())
+        if (!alloc.ensureBallast())
             return false;
 
         CFGControlInstruction* copy = nullptr;
         CFGControlInstruction* ins = blocks[i]->stopIns();
         switch (ins->type()) {
           case CFGControlInstruction::Type_Goto: {
             CFGBlock* successor = &blocks_[ins->getSuccessor(0)->id()];
-            copy = CFGGoto::New(alloc(), successor, ins->toGoto()->popAmount());
+            copy = CFGGoto::New(alloc, successor, ins->toGoto()->popAmount());
             break;
           }
           case CFGControlInstruction::Type_BackEdge: {
             CFGBlock* successor = &blocks_[ins->getSuccessor(0)->id()];
-            copy = CFGBackEdge::New(alloc(), successor);
+            copy = CFGBackEdge::New(alloc, successor);
             break;
           }
           case CFGControlInstruction::Type_LoopEntry: {
             CFGLoopEntry* old = ins->toLoopEntry();
             CFGBlock* successor = &blocks_[ins->getSuccessor(0)->id()];
-            copy = CFGLoopEntry::New(alloc(), successor, old->canOsr(), old->stackPhiCount(),
+            copy = CFGLoopEntry::New(alloc, successor, old->canOsr(), old->stackPhiCount(),
                                      old->loopStopPc());
             break;
           }
           case CFGControlInstruction::Type_Throw: {
-            copy = CFGThrow::New(alloc());
+            copy = CFGThrow::New(alloc);
             break;
           }
           case CFGControlInstruction::Type_Test: {
             CFGTest* old = ins->toTest();
             CFGBlock* trueBranch = &blocks_[old->trueBranch()->id()];
             CFGBlock* falseBranch = &blocks_[old->falseBranch()->id()];
-            copy = CFGTest::New(alloc(), trueBranch, falseBranch, old->mustKeepCondition());
+            copy = CFGTest::New(alloc, trueBranch, falseBranch, old->mustKeepCondition());
             break;
           }
           case CFGControlInstruction::Type_Compare: {
             CFGCompare* old = ins->toCompare();
             CFGBlock* trueBranch = &blocks_[old->trueBranch()->id()];
             CFGBlock* falseBranch = &blocks_[old->falseBranch()->id()];
-            copy = CFGCompare::New(alloc(), trueBranch, falseBranch);
+            copy = CFGCompare::New(alloc, trueBranch, falseBranch);
             break;
           }
           case CFGControlInstruction::Type_Return: {
-            copy = CFGReturn::New(alloc());
+            copy = CFGReturn::New(alloc);
             break;
           }
           case CFGControlInstruction::Type_RetRVal: {
-            copy = CFGRetRVal::New(alloc());
+            copy = CFGRetRVal::New(alloc);
             break;
           }
           case CFGControlInstruction::Type_Try: {
             CFGTry* old = ins->toTry();
             CFGBlock* tryBlock = &blocks_[old->tryBlock()->id()];
             CFGBlock* merge = nullptr;
             if (old->numSuccessors() == 2)
                 merge = &blocks_[old->afterTryCatchBlock()->id()];
-            copy = CFGTry::New(alloc(), tryBlock, old->catchStartPc(), merge);
+            copy = CFGTry::New(alloc, tryBlock, old->catchStartPc(), merge);
             break;
           }
           case CFGControlInstruction::Type_TableSwitch: {
             CFGTableSwitch* old = ins->toTableSwitch();
             CFGTableSwitch* tableSwitch =
-                CFGTableSwitch::New(alloc(), old->low(), old->high());
+                CFGTableSwitch::New(alloc, old->low(), old->high());
             if (!tableSwitch->addDefault(&blocks_[old->defaultCase()->id()]))
                 return false;
             for (size_t i = 0; i < ins->numSuccessors() - 1; i++) {
                 if (!tableSwitch->addCase(&blocks_[old->getCase(i)->id()]))
                     return false;
             }
             copy = tableSwitch;
             break;
--- a/js/src/jit/IonControlFlow.h
+++ b/js/src/jit/IonControlFlow.h
@@ -30,16 +30,37 @@ class CFGControlInstruction;
         return new(alloc) CFGThisOpcode(mozilla::Forward<Args>(args)...);     \
     }                                                                         \
     template <typename... Args>                                               \
     static CFGThisOpcode* New(TempAllocator::Fallible alloc, Args&&... args)  \
     {                                                                         \
         return new(alloc) CFGThisOpcode(mozilla::Forward<Args>(args)...);     \
     }
 
+class CFGSpace
+{
+    static const size_t DEFAULT_CHUNK_SIZE = 4096;
+
+  protected:
+    LifoAlloc allocator_;
+  public:
+
+    explicit CFGSpace()
+      : allocator_(DEFAULT_CHUNK_SIZE)
+    {}
+
+    LifoAlloc& lifoAlloc() {
+        return allocator_;
+    }
+
+    size_t sizeOfExcludingThis(mozilla::MallocSizeOf mallocSizeOf) const {
+        return allocator_.sizeOfExcludingThis(mallocSizeOf);
+    }
+};
+
 class CFGBlock : public TempObject
 {
     size_t id_;
     jsbytecode* start;
     jsbytecode* stop;
     CFGControlInstruction* end;
     bool inWorkList;
 
@@ -513,42 +534,36 @@ class CFGLoopEntry : public CFGUnaryCont
 
     void setLoopStopPc(jsbytecode* loopStopPc) {
         loopStopPc_ = loopStopPc;
     }
 };
 
 typedef Vector<CFGBlock*, 4, JitAllocPolicy> CFGBlockVector;
 
-class ControlFlowGraph
+class ControlFlowGraph : public TempObject
 {
-    // Keeps the data alive until destruction.
-    LifoAlloc lifoAlloc_;
-    TempAllocator alloc_;
-
     // A list of blocks in RPO, containing per block a pc-range and
     // a control instruction.
     Vector<CFGBlock, 4, JitAllocPolicy> blocks_;
 
+    explicit ControlFlowGraph(TempAllocator& alloc)
+      : blocks_(alloc)
+    {}
+
   public:
-    ControlFlowGraph()
-      : lifoAlloc_(TempAllocator::PreferredLifoChunkSize),
-        alloc_(&lifoAlloc_),
-        blocks_(alloc_)
-    {}
+    static ControlFlowGraph* New(TempAllocator& alloc) {
+        return new(alloc) ControlFlowGraph(alloc);
+    }
 
     ControlFlowGraph(const ControlFlowGraph&) = delete;
     void operator=(const ControlFlowGraph&) = delete;
 
     void dump(GenericPrinter& print, JSScript* script);
-    bool init(const CFGBlockVector& blocks);
-
-    TempAllocator& alloc() {
-        return alloc_;
-    }
+    bool init(TempAllocator& alloc, const CFGBlockVector& blocks);
 
     const CFGBlock* block(size_t i) const {
         return &blocks_[i];
     }
 
     size_t numBlocks() const {
         return blocks_.length();
     }
@@ -730,24 +745,22 @@ class ControlFlowGenerator
 
   public:
     ControlFlowGenerator(TempAllocator& alloc, JSScript* script);
 
     MOZ_MUST_USE bool init();
 
     MOZ_MUST_USE bool traverseBytecode();
     MOZ_MUST_USE bool addBlock(CFGBlock* block);
-    ControlFlowGraph* getGraph() {
-        ControlFlowGraph* cfg = js_new<ControlFlowGraph>();
+    ControlFlowGraph* getGraph(TempAllocator& alloc) {
+        ControlFlowGraph* cfg = ControlFlowGraph::New(alloc);
         if (!cfg)
             return nullptr;
-        if (!cfg->init(blocks_)) {
-            js_delete(cfg);
+        if (!cfg->init(alloc, blocks_))
             return nullptr;
-        }
         return cfg;
     }
 
     bool aborted() const {
         return aborted_;
     }
 
   private:
--- a/js/src/jit/JitCompartment.h
+++ b/js/src/jit/JitCompartment.h
@@ -10,16 +10,17 @@
 #include "mozilla/Array.h"
 #include "mozilla/DebugOnly.h"
 #include "mozilla/MemoryReporting.h"
 
 #include "builtin/TypedObject.h"
 #include "jit/CompileInfo.h"
 #include "jit/ICStubSpace.h"
 #include "jit/IonCode.h"
+#include "jit/IonControlFlow.h"
 #include "jit/JitFrames.h"
 #include "jit/shared/Assembler-shared.h"
 #include "js/GCHashTable.h"
 #include "js/Value.h"
 #include "vm/Stack.h"
 
 namespace js {
 namespace jit {
@@ -379,21 +380,26 @@ class JitRuntime
         return isProfilerInstrumentationEnabled(rt);
     }
 };
 
 class JitZone
 {
     // Allocated space for optimized baseline stubs.
     OptimizedICStubSpace optimizedStubSpace_;
+    // Allocated space for cached cfg.
+    CFGSpace cfgSpace_;
 
   public:
     OptimizedICStubSpace* optimizedStubSpace() {
         return &optimizedStubSpace_;
     }
+    CFGSpace* cfgSpace() {
+        return &cfgSpace_;
+    }
 };
 
 enum class CacheKind : uint8_t;
 class CacheIRStubInfo;
 
 enum class ICStubEngine : uint8_t {
     Baseline = 0,
     IonMonkey