Bug 1426124 - Discard JIT frames from native stacks in the profiler; r=jandem,mstange draft
authorGreg Tatum <gtatum@mozilla.com>
Wed, 25 Apr 2018 10:08:38 -0500
changeset 794240 ec4e2a5180f191b9c0946143f6e01eaf25028ce7
parent 794191 4303d49c53931385892231969e40048f096b4d4c
push id109622
push usergtatum@mozilla.com
push dateFri, 11 May 2018 16:06:46 +0000
reviewersjandem, mstange
bugs1426124
milestone62.0a1
Bug 1426124 - Discard JIT frames from native stacks in the profiler; r=jandem,mstange MozReview-Commit-ID: 9O92eRm5adW
js/public/ProfilingFrameIterator.h
js/src/vm/Stack.cpp
js/src/vm/Stack.h
tools/profiler/core/platform.cpp
--- a/js/public/ProfilingFrameIterator.h
+++ b/js/public/ProfilingFrameIterator.h
@@ -116,16 +116,17 @@ class MOZ_NON_PARAM JS_PUBLIC_API(Profil
     };
 
     struct Frame
     {
         FrameKind kind;
         void* stackAddress;
         void* returnAddress;
         void* activation;
+        void* endStackAddress;
         const char* label;
     } JS_HAZ_GC_INVALIDATED;
 
     bool isWasm() const;
     bool isJSJit() const;
 
     uint32_t extractStack(Frame* frames, uint32_t offset, uint32_t end) const;
 
--- a/js/src/vm/Stack.cpp
+++ b/js/src/vm/Stack.cpp
@@ -1980,16 +1980,17 @@ JS::ProfilingFrameIterator::getPhysicalF
 
     if (isWasm()) {
         Frame frame;
         frame.kind = Frame_Wasm;
         frame.stackAddress = stackAddr;
         frame.returnAddress = nullptr;
         frame.activation = activation_;
         frame.label = nullptr;
+        frame.endStackAddress = activation_->asJit()->jsOrWasmExitFP();
         return mozilla::Some(frame);
     }
 
     MOZ_ASSERT(isJSJit());
 
     // Look up an entry for the return address.
     void* returnAddr = jsJitIter().returnAddressToFp();
     jit::JitcodeGlobalTable* table = cx_->runtime()->jitRuntime()->getJitcodeGlobalTable();
@@ -2006,16 +2007,17 @@ JS::ProfilingFrameIterator::getPhysicalF
         return mozilla::Nothing();
 
     Frame frame;
     frame.kind = entry->isBaseline() ? Frame_Baseline : Frame_Ion;
     frame.stackAddress = stackAddr;
     frame.returnAddress = returnAddr;
     frame.activation = activation_;
     frame.label = nullptr;
+    frame.endStackAddress = activation_->asJit()->jsOrWasmExitFP();
     return mozilla::Some(frame);
 }
 
 uint32_t
 JS::ProfilingFrameIterator::extractStack(Frame* frames, uint32_t offset, uint32_t end) const
 {
     if (offset >= end)
         return 0;
--- a/js/src/vm/Stack.h
+++ b/js/src/vm/Stack.h
@@ -1690,16 +1690,19 @@ class JitActivation : public Activation
     }
     static size_t offsetOfPrevJitActivation() {
         return offsetof(JitActivation, prevJitActivation_);
     }
 
     bool hasExitFP() const {
         return !!packedExitFP_;
     }
+    uint8_t* jsOrWasmExitFP() const {
+        return (uint8_t*)(uintptr_t(packedExitFP_) & ~ExitFpWasmBit);
+    }
     static size_t offsetOfPackedExitFP() {
         return offsetof(JitActivation, packedExitFP_);
     }
 
     bool hasJSExitFP() const {
         return !(uintptr_t(packedExitFP_) & ExitFpWasmBit);
     }
     uint8_t* jsExitFP() const {
--- a/tools/profiler/core/platform.cpp
+++ b/tools/profiler/core/platform.cpp
@@ -907,23 +907,25 @@ MergeStacks(uint32_t aFeatures, bool aIs
   // oldest-to-youngest. Thus, iterate over the pseudo-stack forwards and JS
   // and native arrays backwards. Note: this means the terminating condition
   // jsIndex and nativeIndex is being < 0.
   uint32_t pseudoIndex = 0;
   int32_t jsIndex = jsCount - 1;
   int32_t nativeIndex = aNativeStack.mCount - 1;
 
   uint8_t* lastPseudoCppStackAddr = nullptr;
+  uint8_t* jitEndStackAddr = nullptr;
 
   // Iterate as long as there is at least one frame remaining.
   while (pseudoIndex != pseudoCount || jsIndex >= 0 || nativeIndex >= 0) {
     // There are 1 to 3 frames available. Find and add the oldest.
     uint8_t* pseudoStackAddr = nullptr;
     uint8_t* jsStackAddr = nullptr;
     uint8_t* nativeStackAddr = nullptr;
+    uint8_t* jsActivationAddr = nullptr;
 
     if (pseudoIndex != pseudoCount) {
       const js::ProfileEntry& pseudoEntry = pseudoEntries[pseudoIndex];
 
       if (pseudoEntry.isCpp()) {
         lastPseudoCppStackAddr = (uint8_t*) pseudoEntry.stackAddress();
       }
 
@@ -938,16 +940,17 @@ MergeStacks(uint32_t aFeatures, bool aIs
       }
 
       MOZ_ASSERT(lastPseudoCppStackAddr);
       pseudoStackAddr = lastPseudoCppStackAddr;
     }
 
     if (jsIndex >= 0) {
       jsStackAddr = (uint8_t*) jsFrames[jsIndex].stackAddress;
+      jsActivationAddr = (uint8_t*) jsFrames[jsIndex].activation;
     }
 
     if (nativeIndex >= 0) {
       nativeStackAddr = (uint8_t*) aNativeStack.mSPs[nativeIndex];
     }
 
     // If there's a native stack entry which has the same SP as a pseudo stack
     // entry, pretend we didn't see the native stack entry.  Ditto for a native
@@ -984,17 +987,17 @@ MergeStacks(uint32_t aFeatures, bool aIs
       pseudoIndex++;
       continue;
     }
 
     // Check to see if JS jit stack frame is top-most
     if (jsStackAddr > nativeStackAddr) {
       MOZ_ASSERT(jsIndex >= 0);
       const JS::ProfilingFrameIterator::Frame& jsFrame = jsFrames[jsIndex];
-
+      jitEndStackAddr = (uint8_t*) jsFrame.endStackAddress;
       // Stringifying non-wasm JIT frames is delayed until streaming time. To
       // re-lookup the entry in the JitcodeGlobalTable, we need to store the
       // JIT code address (OptInfoAddr) in the circular buffer.
       //
       // Note that we cannot do this when we are sychronously sampling the
       // current thread; that is, when called from profiler_get_backtrace. The
       // captured backtrace is usually externally stored for an indeterminate
       // amount of time, such as in nsRefreshDriver. Problematically, the
@@ -1013,17 +1016,26 @@ MergeStacks(uint32_t aFeatures, bool aIs
       }
 
       jsIndex--;
       continue;
     }
 
     // If we reach here, there must be a native stack entry and it must be the
     // greatest entry.
-    if (nativeStackAddr) {
+    if (nativeStackAddr &&
+        // If the latest JS frame was JIT, this could be the native frame that
+        // corresponds to it. In that case, skip the native frame, because there's
+        // no need for the same frame to be present twice in the stack. The JS
+        // frame can be considered the symbolicated version of the native frame.
+        (!jitEndStackAddr || nativeStackAddr < jitEndStackAddr ) &&
+        // This might still be a JIT operation, check to make sure that is not in range
+        // of the NEXT JavaScript's stacks' activation address.
+        (!jsActivationAddr || nativeStackAddr > jsActivationAddr)
+      ) {
       MOZ_ASSERT(nativeIndex >= 0);
       void* addr = (void*)aNativeStack.mPCs[nativeIndex];
       aCollector.CollectNativeLeafAddr(addr);
     }
     if (nativeIndex >= 0) {
       nativeIndex--;
     }
   }