Bug 1464829 - Remove recover instruction results after bailouts. r=jandem a=lizzard
authorNicolas B. Pierron <nicolas.b.pierron@gmail.com>
Mon, 28 May 2018 15:39:43 +0000
changeset 478013 1f9ce3126115f82fa8440a311e175e0771183425
parent 478012 6aedd84a5388b511ba15f64e6efeaaec09079c58
child 478014 0e7242cbb1ce425e8e8caf0457defc0e01881220
push id9499
push userarchaeopteryx@coole-files.de
push dateThu, 19 Jul 2018 06:52:33 +0000
treeherdermozilla-beta@ce18b96ec82b [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersjandem, lizzard
bugs1464829
milestone62.0
Bug 1464829 - Remove recover instruction results after bailouts. r=jandem a=lizzard
js/src/jit/BaselineBailouts.cpp
js/src/jit/JSJitFrameIter.h
--- a/js/src/jit/BaselineBailouts.cpp
+++ b/js/src/jit/BaselineBailouts.cpp
@@ -417,51 +417,16 @@ struct BaselineStackBuilder
 #endif
     }
 
     void setCheckGlobalDeclarationConflicts() {
         header_->checkGlobalDeclarationConflicts = true;
     }
 };
 
-// Ensure that all value locations are readable from the SnapshotIterator.
-// Remove RInstructionResults from the JitActivation if the frame got recovered
-// ahead of the bailout.
-class SnapshotIteratorForBailout : public SnapshotIterator
-{
-    JitActivation* activation_;
-    const JSJitFrameIter& iter_;
-
-  public:
-    SnapshotIteratorForBailout(JitActivation* activation, const JSJitFrameIter& iter)
-      : SnapshotIterator(iter, activation->bailoutData()->machineState()),
-        activation_(activation),
-        iter_(iter)
-    {
-        MOZ_ASSERT(iter.isBailoutJS());
-    }
-
-    ~SnapshotIteratorForBailout() {
-        // The bailout is complete, we no longer need the recover instruction
-        // results.
-        activation_->removeIonFrameRecovery(fp_);
-    }
-
-    // Take previously computed result out of the activation, or compute the
-    // results of all recover instructions contained in the snapshot.
-    MOZ_MUST_USE bool init(JSContext* cx) {
-
-        // Under a bailout, there is no need to invalidate the frame after
-        // evaluating the recover instruction, as the invalidation is only
-        // needed to cause of the frame which has been introspected.
-        MaybeReadFallback recoverBailout(cx, activation_, &iter_, MaybeReadFallback::Fallback_DoNothing);
-        return initInstructionResults(recoverBailout);
-    }
-};
-
 #ifdef DEBUG
 static inline bool
 IsInlinableFallback(ICFallbackStub* icEntry)
 {
     return icEntry->isCall_Fallback() || icEntry->isGetProp_Fallback() ||
            icEntry->isSetProp_Fallback();
 }
 #endif
@@ -1510,28 +1475,35 @@ InitFromBailout(JSContext* cx, size_t fr
 uint32_t
 jit::BailoutIonToBaseline(JSContext* cx, JitActivation* activation,
                           const JSJitFrameIter& iter, bool invalidate,
                           BaselineBailoutInfo** bailoutInfo,
                           const ExceptionBailoutInfo* excInfo)
 {
     MOZ_ASSERT(bailoutInfo != nullptr);
     MOZ_ASSERT(*bailoutInfo == nullptr);
+    MOZ_ASSERT(iter.isBailoutJS());
 
     TraceLoggerThread* logger = TraceLoggerForCurrentThread(cx);
     TraceLogStopEvent(logger, TraceLogger_IonMonkey);
     TraceLogStartEvent(logger, TraceLogger_Baseline);
 
     // Ion bailout can fail due to overrecursion and OOM. In such cases we
     // cannot honor any further Debugger hooks on the frame, and need to
     // ensure that its Debugger.Frame entry is cleaned up.
     auto guardRemoveRematerializedFramesFromDebugger = mozilla::MakeScopeExit([&] {
         activation->removeRematerializedFramesFromDebugger(cx, iter.fp());
     });
 
+    // Always remove the RInstructionResults from the JitActivation, even in
+    // case of failures as the stack frame is going away after the bailout.
+    auto removeIonFrameRecovery = mozilla::MakeScopeExit([&] {
+        activation->removeIonFrameRecovery(iter.jsFrame());
+    });
+
     // The caller of the top frame must be one of the following:
     //      IonJS - Ion calling into Ion.
     //      BaselineStub - Baseline calling into Ion.
     //      Entry / WasmToJSJit - Interpreter or other (wasm) calling into Ion.
     //      Rectifier - Arguments rectifier calling into Ion.
     MOZ_ASSERT(iter.isBailoutJS());
 #if defined(DEBUG) || defined(JS_JITSPEW)
     FrameType prevFrameType = iter.prevType();
@@ -1595,18 +1567,26 @@ jit::BailoutIonToBaseline(JSContext* cx,
     // Allocate buffer to hold stack replacement data.
     BaselineStackBuilder builder(iter, 1024);
     if (!builder.init()) {
         ReportOutOfMemory(cx);
         return BAILOUT_RETURN_FATAL_ERROR;
     }
     JitSpew(JitSpew_BaselineBailouts, "  Incoming frame ptr = %p", builder.startFrame());
 
-    SnapshotIteratorForBailout snapIter(activation, iter);
-    if (!snapIter.init(cx)) {
+    // Under a bailout, there is no need to invalidate the frame after
+    // evaluating the recover instruction, as the invalidation is only needed in
+    // cases where the frame is introspected ahead of the bailout.
+    MaybeReadFallback recoverBailout(cx, activation, &iter, MaybeReadFallback::Fallback_DoNothing);
+
+    // Ensure that all value locations are readable from the SnapshotIterator.
+    // Get the RInstructionResults from the JitActivation if the frame got
+    // recovered ahead of the bailout.
+    SnapshotIterator snapIter(iter, activation->bailoutData()->machineState());
+    if (!snapIter.initInstructionResults(recoverBailout)) {
         ReportOutOfMemory(cx);
         return BAILOUT_RETURN_FATAL_ERROR;
     }
 
 #ifdef TRACK_SNAPSHOTS
     snapIter.spewBailingFrom();
 #endif
 
--- a/js/src/jit/JSJitFrameIter.h
+++ b/js/src/jit/JSJitFrameIter.h
@@ -528,23 +528,23 @@ class SnapshotIterator
     // Skip an Instruction by walking to the next instruction and by skipping
     // all the allocations corresponding to this instruction.
     void skipInstruction();
 
     inline bool moreInstructions() const {
         return recover_.moreInstructions();
     }
 
-  protected:
     // Register a vector used for storing the results of the evaluation of
     // recover instructions. This vector should be registered before the
     // beginning of the iteration. This function is in charge of allocating
     // enough space for all instructions results, and return false iff it fails.
     MOZ_MUST_USE bool initInstructionResults(MaybeReadFallback& fallback);
 
+  protected:
     // This function is used internally for computing the result of the recover
     // instructions.
     MOZ_MUST_USE bool computeInstructionResults(JSContext* cx, RInstructionResults* results) const;
 
   public:
     // Handle iterating over frames of the snapshots.
     void nextFrame();
     void settleOnFrame();