Improve JS stack profiling

Summary:
This is a follow-up to the previous diff(D37889426).

Diff 1: we change the plumbing to pass a pointer to the SamplingProfiler in the callback, so that when we collect the stack trace we know which profiler to collect from.

Diff 2: code clean up to make more code shared between Android and Apple platforms, for the collectStackForLoom() function.

Diff 3: Notify loom when the SamplingProfiler is destroyed, such that it should no longer use the dangling pointer.

Diff 4: Move the enableForLoomCollection() to the point when loom starts, and move the disableForLoomCollection() to the point when loom ends. Add APIs only, these APIs are not used yet in this diff.

Diff5: Split SamplingProfiler::GlobalProfiler::sampleStacks() into two function, the added function does the work of one iteration in sampleStacks().

Diff6: Use the newly added APIs in Diff 4.

Reviewed By: jpporto

Differential Revision: D38745848

fbshipit-source-id: 771a95da1f0d04b21cad365ca8fb63336e53dfb4

include/hermes/VM/Profiler/SamplingProfilerPosix.h

@@ -396,6 +396,16 @@ class SamplingProfiler {
   /// Resumes the sample profiling. There must have been a previous call to
   /// suspend() that hansn't been resume()d yet.
   void resume();
+
+#if (defined(__ANDROID__) || defined(__APPLE__)) && \
+    defined(HERMES_FACEBOOK_BUILD)
+  // Common code that is shared by the collectStackForLoom(), for both the
+  // Android and Apple versions.
+  void collectStackForLoomCommon(
+      const StackFrame &frame,
+      int64_t *frames,
+      uint32_t index);
+#endif
 };
 
 /// An RAII class for temporarily suspending (and auto-resuming) the sampling

lib/VM/Profiler/SamplingProfilerPosix.cpp

@@ -362,6 +362,45 @@ bool SamplingProfiler::GlobalProfiler::enabled() {
   return enabled_;
 }
 
+#if (defined(__ANDROID__) || defined(__APPLE__)) && \
+    defined(HERMES_FACEBOOK_BUILD)
+void SamplingProfiler::collectStackForLoomCommon(
+    const StackFrame &frame,
+    int64_t *frames,
+    uint32_t index) {
+  constexpr uint64_t kNativeFrameMask = ((uint64_t)1 << 63);
+  switch (frame.kind) {
+    case StackFrame::FrameKind::JSFunction: {
+      auto *bcProvider = frame.jsFrame.module->getBytecode();
+      uint32_t virtualOffset =
+          bcProvider->getVirtualOffsetForFunction(frame.jsFrame.functionId) +
+          frame.jsFrame.offset;
+      uint32_t segmentID = bcProvider->getSegmentID();
+      uint64_t frameAddress = ((uint64_t)segmentID << 32) + virtualOffset;
+      assert(
+          (frameAddress & kNativeFrameMask) == 0 &&
+          "Module id should take less than 32 bits");
+      frames[(index)] = static_cast<int64_t>(frameAddress);
+      break;
+    }
+
+    case StackFrame::FrameKind::NativeFunction:
+    case StackFrame::FrameKind::FinalizableNativeFunction: {
+#if defined(__ANDROID__)
+      NativeFunctionPtr nativeFrame = frame.nativeFunctionPtrForLoom;
+#else
+      NativeFunctionPtr nativeFrame = getNativeFunctionPtr(frame);
+#endif
+      frames[(index)] = ((uint64_t)nativeFrame | kNativeFrameMask);
+      break;
+    }
+
+    default:
+      llvm_unreachable("Loom: unknown frame kind");
+  }
+}
+#endif
+
 #if defined(__ANDROID__) && defined(HERMES_FACEBOOK_BUILD)
 /*static*/ StackCollectionRetcode SamplingProfiler::collectStackForLoom(
     ucontext_t *ucontext,
@@ -405,34 +444,8 @@ bool SamplingProfiler::GlobalProfiler::enabled() {
   // TODO: enhance this when supporting more frame types.
   sampledStackDepth = std::min(sampledStackDepth, (uint32_t)max_depth);
   for (uint32_t i = 0; i < sampledStackDepth; ++i) {
-    constexpr uint64_t kNativeFrameMask = ((uint64_t)1 << 63);
     const StackFrame &stackFrame = profilerInstance->sampleStorage_.stack[i];
-    switch (stackFrame.kind) {
-      case StackFrame::FrameKind::JSFunction: {
-        auto *bcProvider = stackFrame.jsFrame.module->getBytecode();
-        uint32_t virtualOffset = bcProvider->getVirtualOffsetForFunction(
-                                     stackFrame.jsFrame.functionId) +
-            stackFrame.jsFrame.offset;
-
-        uint32_t segmentID = bcProvider->getSegmentID();
-        uint64_t frameAddress = ((uint64_t)segmentID << 32) + virtualOffset;
-        assert(
-            (frameAddress & kNativeFrameMask) == 0 &&
-            "Module id should take less than 32 bits");
-        frames[i] = frameAddress;
-        break;
-      }
-
-      case StackFrame::FrameKind::NativeFunction:
-      case StackFrame::FrameKind::FinalizableNativeFunction: {
-        NativeFunctionPtr nativeFrame = stackFrame.nativeFunctionPtrForLoom;
-        frames[i] = ((uint64_t)nativeFrame | kNativeFrameMask);
-        break;
-      }
-
-      default:
-        llvm_unreachable("Loom: unknown frame kind");
-    }
+    localProfiler->collectStackForLoomCommon(stackFrame, frames, i);
   }
   *depth = sampledStackDepth;
   if (*depth == 0) {
@@ -460,39 +473,16 @@ bool SamplingProfiler::GlobalProfiler::enabled() {
   }
   std::lock_guard<std::mutex> lk(profilerInstance->profilerLock_);
   *depth = 0;
-  int index = 0;
+  uint32_t index = 0;
   auto *localProfiler = reinterpret_cast<SamplingProfiler *>(profiler);
-  constexpr uint64_t kNativeFrameMask = ((uint64_t)1 << 63);
   for (unsigned i = 0; i < localProfiler->sampledStacks_.size(); ++i) {
     auto &sample = localProfiler->sampledStacks_[i];
     for (auto iter = sample.stack.rbegin(); iter != sample.stack.rend();
          ++iter) {
       const StackFrame &frame = *iter;
-      switch (frame.kind) {
-        case StackFrame::FrameKind::JSFunction: {
-          auto *bcProvider = frame.jsFrame.module->getBytecode();
-          uint32_t virtualOffset = bcProvider->getVirtualOffsetForFunction(
-                                       frame.jsFrame.functionId) +
-              frame.jsFrame.offset;
-          uint32_t segmentID = bcProvider->getSegmentID();
-          uint64_t frameAddress = ((uint64_t)segmentID << 32) + virtualOffset;
-          frames[index++] = static_cast<int64_t>(frameAddress);
-          (*depth)++;
-          break;
-        }
-
-        case StackFrame::FrameKind::NativeFunction:
-        case StackFrame::FrameKind::FinalizableNativeFunction: {
-          NativeFunctionPtr nativeFrame =
-              localProfiler->getNativeFunctionPtr(frame);
-          frames[index++] = ((uint64_t)nativeFrame | kNativeFrameMask);
-          (*depth)++;
-          break;
-        }
-
-        default:
-          llvm_unreachable("Loom: unknown frame kind");
-      }
+      localProfiler->collectStackForLoomCommon(frame, frames, index);
+      (*depth)++;
+      index++;
     }
   }
   localProfiler->clear();