Bug 903754 - Remove InterpreterFrames class and use InterpreterActivation instead. r=luke
authorJan de Mooij <jdemooij@mozilla.com>
Tue, 13 Aug 2013 14:06:30 +0200
changeset 142400 cd086a74f5224e2c939bbc06c0eee0a4f410437c
parent 142399 3c0f48bd511897563fd1c0aa12d2cac82ac6c7e5
child 142401 831e2b72ad56314f6b6ed20ad326083f04e41f6d
push id25095
push userryanvm@gmail.com
push dateTue, 13 Aug 2013 19:37:08 +0000
treeherdermozilla-central@31f4b8658e3e [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersluke
bugs903754
milestone26.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 903754 - Remove InterpreterFrames class and use InterpreterActivation instead. r=luke
js/src/jsdbgapi.cpp
js/src/jsscript.cpp
js/src/vm/Interpreter.cpp
js/src/vm/Interpreter.h
js/src/vm/Runtime.cpp
js/src/vm/Runtime.h
js/src/vm/Stack-inl.h
js/src/vm/Stack.h
--- a/js/src/jsdbgapi.cpp
+++ b/js/src/jsdbgapi.cpp
@@ -262,18 +262,22 @@ JS_ClearAllTrapsForCompartment(JSContext
     cx->compartment()->clearTraps(cx->runtime()->defaultFreeOp());
 }
 
 JS_PUBLIC_API(bool)
 JS_SetInterrupt(JSRuntime *rt, JSInterruptHook hook, void *closure)
 {
     rt->debugHooks.interruptHook = hook;
     rt->debugHooks.interruptHookData = closure;
-    for (InterpreterFrames *f = rt->interpreterFrames; f; f = f->older)
-        f->enableInterruptsUnconditionally();
+
+    for (ActivationIterator iter(rt); !iter.done(); ++iter) {
+        if (iter.activation()->isInterpreter())
+            iter.activation()->asInterpreter()->enableInterruptsUnconditionally();
+    }
+
     return true;
 }
 
 JS_PUBLIC_API(bool)
 JS_ClearInterrupt(JSRuntime *rt, JSInterruptHook *hoop, void **closurep)
 {
     if (hoop)
         *hoop = rt->debugHooks.interruptHook;
--- a/js/src/jsscript.cpp
+++ b/js/src/jsscript.cpp
@@ -859,19 +859,20 @@ JSScript::initScriptCounts(JSContext *cx
         js_free(base);
         return false;
     }
     hasScriptCounts = true; // safe to set this;  we can't fail after this point
 
     JS_ASSERT(size_t(cursor - base) == bytes);
 
     /* Enable interrupts in any interpreter frames running on this script. */
-    InterpreterFrames *frames;
-    for (frames = cx->runtime()->interpreterFrames; frames; frames = frames->older)
-        frames->enableInterruptsIfRunning(this);
+    for (ActivationIterator iter(cx->runtime()); !iter.done(); ++iter) {
+        if (iter.activation()->isInterpreter())
+            iter.activation()->asInterpreter()->enableInterruptsIfRunning(this);
+    }
 
     return true;
 }
 
 static inline ScriptCountsMap::Ptr GetScriptCountsMapEntry(JSScript *script)
 {
     JS_ASSERT(script->hasScriptCounts);
     ScriptCountsMap *map = script->compartment()->scriptCountsMap;
@@ -2606,19 +2607,20 @@ JSScript::ensureHasDebugScript(JSContext
     }
     hasDebugScript = true; // safe to set this;  we can't fail after this point
 
     /*
      * Ensure that any Interpret() instances running on this script have
      * interrupts enabled. The interrupts must stay enabled until the
      * debug state is destroyed.
      */
-    InterpreterFrames *frames;
-    for (frames = cx->runtime()->interpreterFrames; frames; frames = frames->older)
-        frames->enableInterruptsIfRunning(this);
+    for (ActivationIterator iter(cx->runtime()); !iter.done(); ++iter) {
+        if (iter.activation()->isInterpreter())
+            iter.activation()->asInterpreter()->enableInterruptsIfRunning(this);
+    }
 
     return true;
 }
 
 void
 JSScript::recompileForStepMode(FreeOp *fop)
 {
 #ifdef JS_ION
--- a/js/src/vm/Interpreter.cpp
+++ b/js/src/vm/Interpreter.cpp
@@ -970,40 +970,16 @@ TryNoteIter::settle()
 #define FETCH_OBJECT(cx, n, obj)                                              \
     JS_BEGIN_MACRO                                                            \
         HandleValue val = regs.stackHandleAt(n);                              \
         obj = ToObjectFromStack(cx, (val));                                   \
         if (!obj)                                                             \
             goto error;                                                       \
     JS_END_MACRO
 
-template<typename T>
-class GenericInterruptEnabler : public InterpreterFrames::InterruptEnablerBase {
-  public:
-    GenericInterruptEnabler(T *variable, T value) : variable(variable), value(value) { }
-    void enable() const { *variable = value; }
-
-  private:
-    T *variable;
-    T value;
-};
-
-inline InterpreterFrames::InterpreterFrames(JSContext *cx, FrameRegs *regs,
-                                            const InterruptEnablerBase &enabler)
-  : context(cx), regs(regs), enabler(enabler)
-{
-    older = cx->runtime()->interpreterFrames;
-    cx->runtime()->interpreterFrames = this;
-}
-
-inline InterpreterFrames::~InterpreterFrames()
-{
-    context->runtime()->interpreterFrames = older;
-}
-
 /*
  * Ensure that the interpreter switch can close call-bytecode cases in the
  * same way as non-call bytecodes.
  */
 JS_STATIC_ASSERT(JSOP_NAME_LENGTH == JSOP_CALLNAME_LENGTH);
 JS_STATIC_ASSERT(JSOP_GETARG_LENGTH == JSOP_CALLARG_LENGTH);
 JS_STATIC_ASSERT(JSOP_GETLOCAL_LENGTH == JSOP_CALLLOCAL_LENGTH);
 
@@ -1282,20 +1258,29 @@ Interpret(JSContext *cx, RunState &state
     JSAutoResolveFlags rf(cx, RESOLVE_INFER);
 
     gc::MaybeVerifyBarriers(cx, true);
 
     JS_ASSERT(!cx->compartment()->activeAnalysis);
 
 #define CHECK_PCCOUNT_INTERRUPTS() JS_ASSERT_IF(script->hasScriptCounts, switchMask == -1)
 
+    /*
+     * When Debugger puts a script in single-step mode, all js::Interpret
+     * invocations that might be presently running that script must have
+     * interrupts enabled. It's not practical to simply check
+     * script->stepModeEnabled() at each point some callee could have changed
+     * it, because there are so many places js::Interpret could possibly cause
+     * JavaScript to run: each place an object might be coerced to a primitive
+     * or a number, for example. So instead, we expose a simple mechanism to
+     * let Debugger tweak the affected js::Interpret frames when an onStep
+     * handler is added: setting switchMask to -1 will enable interrupts.
+     */
     register int switchMask = 0;
     int switchOp;
-    typedef GenericInterruptEnabler<int> InterruptEnabler;
-    InterruptEnabler interrupts(&switchMask, -1);
 
 # define DO_OP()            goto do_op
 
 # define BEGIN_CASE(OP)     case OP:
 # define END_CASE(OP)       END_CASE_LEN(OP##_LENGTH)
 # define END_CASE_LEN(n)    END_CASE_LENX(n)
 # define END_CASE_LENX(n)   END_CASE_LEN##n
 
@@ -1340,17 +1325,17 @@ Interpret(JSContext *cx, RunState &state
             goto check_backedge;                                              \
         DO_OP();                                                              \
     JS_END_MACRO
 
 #define SET_SCRIPT(s)                                                         \
     JS_BEGIN_MACRO                                                            \
         script = (s);                                                         \
         if (script->hasAnyBreakpointsOrStepMode() || script->hasScriptCounts) \
-            interrupts.enable();                                              \
+            switchMask = -1; /* Enable interrupts. */                         \
     JS_END_MACRO
 
     FrameRegs regs;
     FrameGuard fg(state, regs);
 
     StackFrame *entryFrame = state.pushInterpreterFrame(cx, &fg);
     if (!entryFrame)
         return false;
@@ -1359,23 +1344,17 @@ Interpret(JSContext *cx, RunState &state
         regs.prepareToRun(*entryFrame, state.script());
         JS_ASSERT(regs.pc == state.script()->code);
     } else {
         regs = state.asGenerator()->gen()->regs;
     }
 
     JS_ASSERT_IF(entryFrame->isEvalFrame(), state.script()->isActiveEval);
 
-    InterpreterActivation activation(cx, entryFrame, regs);
-
-    /*
-     * Help Debugger find frames running scripts that it has put in
-     * single-step mode.
-     */
-    InterpreterFrames interpreterFrame(cx, &regs, interrupts);
+    InterpreterActivation activation(cx, entryFrame, regs, &switchMask);
 
     /* Copy in hot values that change infrequently. */
     JSRuntime *const rt = cx->runtime();
     RootedScript script(cx);
     SET_SCRIPT(regs.fp()->script());
 
 #if JS_TRACE_LOGGING
     TraceLogging::defaultLogger()->log(TraceLogging::SCRIPT_START, script);
@@ -1445,17 +1424,17 @@ Interpret(JSContext *cx, RunState &state
      * processing of an opcode while looping. We rely on |advanceAndDoOp:| to
      * manage "op" correctly in all other cases.
      */
     JSOp op;
     int32_t len;
     len = 0;
 
     if (rt->profilingScripts || cx->runtime()->debugHooks.interruptHook)
-        interrupts.enable();
+        switchMask = -1; /* Enable interrupts. */
 
     goto advanceAndDoOp;
 
     for (;;) {
       advance_pc_by_one:
         JS_ASSERT(js_CodeSpec[op].length == 1);
         len = 1;
       advanceAndDoOp:
--- a/js/src/vm/Interpreter.h
+++ b/js/src/vm/Interpreter.h
@@ -310,64 +310,16 @@ extern bool
 SameValue(JSContext *cx, const Value &v1, const Value &v2, bool *same);
 
 extern JSType
 TypeOfValue(JSContext *cx, const Value &v);
 
 extern bool
 HasInstance(JSContext *cx, HandleObject obj, HandleValue v, bool *bp);
 
-/*
- * A linked list of the |FrameRegs regs;| variables belonging to all
- * js::Interpret C++ frames on this thread's stack.
- *
- * Note that this is *not* a list of all JS frames running under the
- * interpreter; that would include inlined frames, whose FrameRegs are
- * saved in various pieces in various places. Rather, this lists each
- * js::Interpret call's live 'regs'; when control returns to that call, it
- * will resume execution with this 'regs' instance.
- *
- * When Debugger puts a script in single-step mode, all js::Interpret
- * invocations that might be presently running that script must have
- * interrupts enabled. It's not practical to simply check
- * script->stepModeEnabled() at each point some callee could have changed
- * it, because there are so many places js::Interpret could possibly cause
- * JavaScript to run: each place an object might be coerced to a primitive
- * or a number, for example. So instead, we simply expose a list of the
- * 'regs' those frames are using, and let Debugger tweak the affected
- * js::Interpret frames when an onStep handler is established.
- *
- * Elements of this list are allocated within the js::Interpret stack
- * frames themselves; the list is headed by this thread's js::ThreadData.
- */
-class InterpreterFrames {
-  public:
-    class InterruptEnablerBase {
-      public:
-        virtual void enable() const = 0;
-    };
-
-    InterpreterFrames(JSContext *cx, FrameRegs *regs, const InterruptEnablerBase &enabler);
-    ~InterpreterFrames();
-
-    /* If this js::Interpret frame is running |script|, enable interrupts. */
-    void enableInterruptsIfRunning(JSScript *script) {
-        if (regs->fp()->script() == script)
-            enabler.enable();
-    }
-    void enableInterruptsUnconditionally() { enabler.enable(); }
-
-    InterpreterFrames *older;
-
-  private:
-    JSContext *context;
-    FrameRegs *regs;
-    const InterruptEnablerBase &enabler;
-};
-
 /* Unwind block and scope chains to match the given depth. */
 extern void
 UnwindScope(JSContext *cx, AbstractFramePtr frame, uint32_t stackDepth);
 
 /*
  * Unwind for an uncatchable exception. This means not running finalizers, etc;
  * just preserving the basic engine stack invariants.
  */
--- a/js/src/vm/Runtime.cpp
+++ b/js/src/vm/Runtime.cpp
@@ -122,17 +122,16 @@ JSRuntime::JSRuntime(JSUseHelperThreads 
     tempLifoAlloc(TEMP_LIFO_ALLOC_PRIMARY_CHUNK_SIZE),
     freeLifoAlloc(TEMP_LIFO_ALLOC_PRIMARY_CHUNK_SIZE),
     execAlloc_(NULL),
     bumpAlloc_(NULL),
     ionRuntime_(NULL),
     selfHostingGlobal_(NULL),
     nativeStackBase(0),
     nativeStackQuota(0),
-    interpreterFrames(NULL),
     cxCallback(NULL),
     destroyCompartmentCallback(NULL),
     compartmentNameCallback(NULL),
     activityCallback(NULL),
     activityCallbackArg(NULL),
 #ifdef JS_THREADSAFE
     requestDepth(0),
 # ifdef DEBUG
--- a/js/src/vm/Runtime.h
+++ b/js/src/vm/Runtime.h
@@ -71,17 +71,16 @@ namespace WTF { class BumpPointerAllocat
 
 namespace js {
 
 typedef Rooted<JSLinearString*> RootedLinearString;
 
 class Activation;
 class ActivationIterator;
 class AsmJSActivation;
-class InterpreterFrames;
 class MathCache;
 class WorkerThreadState;
 
 namespace ion {
 class IonRuntime;
 class JitActivation;
 struct PcScriptCache;
 }
@@ -912,22 +911,16 @@ struct JSRuntime : public JS::shadow::Ru
     void setDefaultVersion(JSVersion v) { defaultVersion_ = v; }
 
     /* Base address of the native stack for the current thread. */
     uintptr_t           nativeStackBase;
 
     /* The native stack size limit that runtime should not exceed. */
     size_t              nativeStackQuota;
 
-    /*
-     * Frames currently running in js::Interpret. See InterpreterFrames for
-     * details.
-     */
-    js::InterpreterFrames *interpreterFrames;
-
     /* Context create/destroy callback. */
     JSContextCallback   cxCallback;
     void               *cxCallbackData;
 
     /* Compartment destroy callback. */
     JSDestroyCompartmentCallback destroyCompartmentCallback;
 
     /* Call this to get the name of a compartment. */
--- a/js/src/vm/Stack-inl.h
+++ b/js/src/vm/Stack-inl.h
@@ -828,54 +828,52 @@ Activation::Activation(JSContext *cx, Ki
 }
 
 Activation::~Activation()
 {
     JS_ASSERT(cx_->mainThread().activation_ == this);
     cx_->mainThread().activation_ = prev_;
 }
 
-InterpreterActivation::InterpreterActivation(JSContext *cx, StackFrame *entry, FrameRegs &regs)
+InterpreterActivation::InterpreterActivation(JSContext *cx, StackFrame *entry, FrameRegs &regs,
+                                             int *const switchMask)
   : Activation(cx, Interpreter),
     entry_(entry),
-    current_(entry),
-    regs_(regs)
+    regs_(regs),
+    switchMask_(switchMask)
 #ifdef DEBUG
   , oldFrameCount_(cx_->runtime()->interpreterStack().frameCount_)
 #endif
 {}
 
 InterpreterActivation::~InterpreterActivation()
 {
     // Pop all inline frames.
-    while (current_ != entry_)
-        popInlineFrame(current_);
+    while (regs_.fp() != entry_)
+        popInlineFrame(regs_.fp());
 
     JS_ASSERT(oldFrameCount_ == cx_->runtime()->interpreterStack().frameCount_);
     JS_ASSERT_IF(oldFrameCount_ == 0, cx_->runtime()->interpreterStack().allocator_.used() == 0);
 }
 
 inline bool
 InterpreterActivation::pushInlineFrame(const CallArgs &args, HandleScript script,
                                        InitialFrameFlags initial)
 {
     if (!cx_->runtime()->interpreterStack().pushInlineFrame(cx_, regs_, args, script, initial))
         return false;
     JS_ASSERT(regs_.fp()->script()->compartment() == compartment_);
-    current_ = regs_.fp();
     return true;
 }
 
 inline void
 InterpreterActivation::popInlineFrame(StackFrame *frame)
 {
-    JS_ASSERT(current_ == frame);
-    JS_ASSERT(current_ != entry_);
-
-    current_ = frame->prev();
-    JS_ASSERT(current_);
+    (void)frame; // Quell compiler warning.
+    JS_ASSERT(regs_.fp() == frame);
+    JS_ASSERT(regs_.fp() != entry_);
 
     cx_->runtime()->interpreterStack().popInlineFrame(regs_);
 }
 
 } /* namespace js */
 
 #endif /* vm_Stack_inl_h */
--- a/js/src/vm/Stack.h
+++ b/js/src/vm/Stack.h
@@ -1221,38 +1221,47 @@ class Activation
 
 class InterpreterFrameIterator;
 
 class InterpreterActivation : public Activation
 {
     friend class js::InterpreterFrameIterator;
 
     StackFrame *const entry_; // Entry frame for this activation.
-    StackFrame *current_;     // The most recent frame.
     FrameRegs &regs_;
+    int *const switchMask_; // For debugger interrupts, see js::Interpret.
 
 #ifdef DEBUG
     size_t oldFrameCount_;
 #endif
 
   public:
-    inline InterpreterActivation(JSContext *cx, StackFrame *entry, FrameRegs &regs);
+    inline InterpreterActivation(JSContext *cx, StackFrame *entry, FrameRegs &regs,
+                                 int *const switchMask);
     inline ~InterpreterActivation();
 
     inline bool pushInlineFrame(const CallArgs &args, HandleScript script,
                                 InitialFrameFlags initial);
     inline void popInlineFrame(StackFrame *frame);
 
     StackFrame *current() const {
-        JS_ASSERT(current_);
-        return current_;
+        return regs_.fp();
     }
     FrameRegs &regs() const {
         return regs_;
     }
+
+    // If this js::Interpret frame is running |script|, enable interrupts.
+    void enableInterruptsIfRunning(JSScript *script) {
+        if (regs_.fp()->script() == script)
+            enableInterruptsUnconditionally();
+    }
+    void enableInterruptsUnconditionally() {
+        *switchMask_ = -1;
+    }
 };
 
 // Iterates over a runtime's activation list.
 class ActivationIterator
 {
     uint8_t *jitTop_;
 
   protected: