Backed out changeset 3923ce220df3 (bug 1380286) for hazard failures
authorCarsten "Tomcat" Book <cbook@mozilla.com>
Tue, 25 Jul 2017 08:44:13 +0200
changeset 370795 18b80915d07365596301f88d6a9b4bbb65eb5a93
parent 370794 50f76943b7cef407c6a2b2da994f91354a1d1d23
child 370796 07484bfdb96bc7297c404e377eea93f1d8ca4442
child 370849 e017c81d94ded55fa27d60d9e2426815f5dacdb4
push id32232
push usercbook@mozilla.com
push dateTue, 25 Jul 2017 12:27:52 +0000
treeherdermozilla-central@07484bfdb96b [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
bugs1380286
milestone56.0a1
backs out3923ce220df31c0a1fb589158f1d2a3f40a93aef
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
Backed out changeset 3923ce220df3 (bug 1380286) for hazard failures
tools/profiler/core/ProfileBuffer.cpp
tools/profiler/core/ProfileBuffer.h
tools/profiler/core/platform.cpp
tools/profiler/moz.build
tools/profiler/public/GeckoProfiler.h
tools/profiler/tests/gtest/GeckoProfiler.cpp
--- a/tools/profiler/core/ProfileBuffer.cpp
+++ b/tools/profiler/core/ProfileBuffer.cpp
@@ -3,18 +3,16 @@
 /* This Source Code Form is subject to the terms of the Mozilla Public
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 #include "ProfileBuffer.h"
 
 #include "ProfilerMarker.h"
 
-using namespace mozilla;
-
 ProfileBuffer::ProfileBuffer(int aEntrySize)
   : mEntries(mozilla::MakeUnique<ProfileBufferEntry[]>(aEntrySize))
   , mWritePos(0)
   , mReadPos(0)
   , mEntrySize(aEntrySize)
   , mGeneration(0)
 {
 }
@@ -53,68 +51,41 @@ ProfileBuffer::AddThreadIdEntry(int aThr
     // This is the start of a sample, so make a note of its location in |aLS|.
     aLS->mGeneration = mGeneration;
     aLS->mPos = mWritePos;
   }
   AddEntry(ProfileBufferEntry::ThreadId(aThreadId));
 }
 
 void
+ProfileBuffer::AddDynamicStringEntry(const char* aStr)
+{
+  size_t strLen = strlen(aStr) + 1;   // +1 for the null terminator
+  for (size_t j = 0; j < strLen; ) {
+    // Store up to kNumChars characters in the entry.
+    char chars[ProfileBufferEntry::kNumChars];
+    size_t len = ProfileBufferEntry::kNumChars;
+    if (j + len >= strLen) {
+      len = strLen - j;
+    }
+    memcpy(chars, &aStr[j], len);
+    j += ProfileBufferEntry::kNumChars;
+
+    AddEntry(ProfileBufferEntry::DynamicStringFragment(chars));
+  }
+}
+
+void
 ProfileBuffer::AddStoredMarker(ProfilerMarker *aStoredMarker)
 {
   aStoredMarker->SetGeneration(mGeneration);
   mStoredMarkers.insert(aStoredMarker);
 }
 
 void
-ProfileBuffer::CollectNativeLeafAddr(void* aAddr)
-{
-  AddEntry(ProfileBufferEntry::NativeLeafAddr(aAddr));
-}
-
-void
-ProfileBuffer::CollectJitReturnAddr(void* aAddr)
-{
-  AddEntry(ProfileBufferEntry::JitReturnAddr(aAddr));
-}
-
-void
-ProfileBuffer::CollectCodeLocation(
-  const char* aLabel, const char* aStr, int aLineNumber,
-  const Maybe<js::ProfileEntry::Category>& aCategory)
-{
-  AddEntry(ProfileBufferEntry::Label(aLabel));
-
-  if (aStr) {
-    // Store the string using one or more DynamicStringFragment entries.
-    size_t strLen = strlen(aStr) + 1;   // +1 for the null terminator
-    for (size_t j = 0; j < strLen; ) {
-      // Store up to kNumChars characters in the entry.
-      char chars[ProfileBufferEntry::kNumChars];
-      size_t len = ProfileBufferEntry::kNumChars;
-      if (j + len >= strLen) {
-        len = strLen - j;
-      }
-      memcpy(chars, &aStr[j], len);
-      j += ProfileBufferEntry::kNumChars;
-
-      AddEntry(ProfileBufferEntry::DynamicStringFragment(chars));
-    }
-  }
-
-  if (aLineNumber != -1) {
-    AddEntry(ProfileBufferEntry::LineNumber(aLineNumber));
-  }
-
-  if (aCategory.isSome()) {
-    AddEntry(ProfileBufferEntry::Category(int(*aCategory)));
-  }
-}
-
-void
 ProfileBuffer::DeleteExpiredStoredMarkers()
 {
   // Delete markers of samples that have been overwritten due to circular
   // buffer wraparound.
   uint32_t generation = mGeneration;
   while (mStoredMarkers.peek() &&
          mStoredMarkers.peek()->HasExpired(generation)) {
     delete mStoredMarkers.popHead();
--- a/tools/profiler/core/ProfileBuffer.h
+++ b/tools/profiler/core/ProfileBuffer.h
@@ -8,17 +8,17 @@
 
 #include "platform.h"
 #include "ProfileBufferEntry.h"
 #include "ProfilerMarker.h"
 #include "ProfileJSONWriter.h"
 #include "mozilla/RefPtr.h"
 #include "mozilla/RefCounted.h"
 
-class ProfileBuffer final : public ProfilerStackCollector
+class ProfileBuffer final
 {
 public:
   explicit ProfileBuffer(int aEntrySize);
 
   ~ProfileBuffer();
 
   // LastSample is used to record the buffer location of the most recent
   // sample for each thread. It is used for periodic samples stored in the
@@ -37,26 +37,19 @@ public:
 
   // Add |aEntry| to the buffer, ignoring what kind of entry it is.
   void AddEntry(const ProfileBufferEntry& aEntry);
 
   // Add to the buffer a sample start (ThreadId) entry for aThreadId. Also,
   // record the resulting generation and index in |aLS| if it's non-null.
   void AddThreadIdEntry(int aThreadId, LastSample* aLS = nullptr);
 
-  virtual mozilla::Maybe<uint32_t> Generation() override
-  {
-    return mozilla::Some(mGeneration);
-  }
-
-  virtual void CollectNativeLeafAddr(void* aAddr) override;
-  virtual void CollectJitReturnAddr(void* aAddr) override;
-  virtual void CollectCodeLocation(
-    const char* aLabel, const char* aStr, int aLineNumber,
-    const mozilla::Maybe<js::ProfileEntry::Category>& aCategory) override;
+  // Add to the buffer a dynamic string. It'll be spread across one or more
+  // DynamicStringFragment entries.
+  void AddDynamicStringEntry(const char* aStr);
 
   // Maximum size of a frameKey string that we'll handle.
   static const size_t kMaxFrameKeyLength = 512;
 
   void StreamSamplesToJSON(SpliceableJSONWriter& aWriter, int aThreadId,
                            double aSinceTime, JSContext* cx,
                            UniqueStacks& aUniqueStacks) const;
   void StreamMarkersToJSON(SpliceableJSONWriter& aWriter, int aThreadId,
--- a/tools/profiler/core/platform.cpp
+++ b/tools/profiler/core/platform.cpp
@@ -17,17 +17,17 @@
 //   call (profiler_get_backtrace()). It involves writing a stack trace and
 //   little else into a temporary ProfileBuffer, and wrapping that up in a
 //   ProfilerBacktrace that can be subsequently used in a marker. The sampling
 //   is done on-thread, and so Registers::SyncPopulate() is used to get the
 //   register values.
 //
 // - A "backtrace" sample is the simplest kind. It is done in response to an
 //   API call (profiler_suspend_and_sample_thread()). It involves getting a
-//   stack trace via a ProfilerStackCollector; it does not write to a
+//   stack trace and passing it to a callback function; it does not write to a
 //   ProfileBuffer. The sampling is done from off-thread, and so uses
 //   SuspendAndSampleAndResumeThread() to get the register values.
 
 #include <algorithm>
 #include <ostream>
 #include <fstream>
 #include <sstream>
 #include <errno.h>
@@ -49,19 +49,17 @@
 #include "mozilla/StaticPtr.h"
 #include "ThreadInfo.h"
 #include "nsIHttpProtocolHandler.h"
 #include "nsIObserverService.h"
 #include "nsIXULAppInfo.h"
 #include "nsIXULRuntime.h"
 #include "nsDirectoryServiceUtils.h"
 #include "nsDirectoryServiceDefs.h"
-#include "nsJSPrincipals.h"
 #include "nsMemoryReporterManager.h"
-#include "nsScriptSecurityManager.h"
 #include "nsXULAppAPI.h"
 #include "nsProfilerStartParams.h"
 #include "ProfilerParent.h"
 #include "mozilla/Services.h"
 #include "nsThreadUtils.h"
 #include "ProfilerMarkerPayload.h"
 #include "shared-libraries.h"
 #include "prdtoa.h"
@@ -654,81 +652,69 @@ public:
   Address mLR;    // ARM link register.
 #if defined(GP_OS_linux) || defined(GP_OS_android)
   // This contains all the registers, which means it duplicates the four fields
   // above. This is ok.
   ucontext_t* mContext; // The context from the signal handler.
 #endif
 };
 
-static bool
-IsChromeJSScript(JSScript* aScript)
+static void
+AddPseudoEntry(PSLockRef aLock, NotNull<RacyThreadInfo*> aRacyInfo,
+               const js::ProfileEntry& entry, ProfileBuffer& aBuffer)
 {
   // WARNING: this function runs within the profiler's "critical section".
 
-  nsIScriptSecurityManager* const secman =
-    nsScriptSecurityManager::GetScriptSecurityManager();
-  NS_ENSURE_TRUE(secman, false);
-
-  JSPrincipals* const principals = JS_GetScriptPrincipals(aScript);
-  return secman->IsSystemPrincipal(nsJSPrincipals::get(principals));
-}
-
-static void
-AddPseudoEntry(uint32_t aFeatures, NotNull<RacyThreadInfo*> aRacyInfo,
-               const js::ProfileEntry& entry,
-               ProfilerStackCollector& aCollector)
-{
-  // WARNING: this function runs within the profiler's "critical section".
-  // WARNING: this function might be called while the profiler is inactive, and
-  //          cannot rely on ActivePS.
-
   MOZ_ASSERT(entry.kind() == js::ProfileEntry::Kind::CPP_NORMAL ||
              entry.kind() == js::ProfileEntry::Kind::JS_NORMAL);
 
+  aBuffer.AddEntry(ProfileBufferEntry::Label(entry.label()));
+
   const char* dynamicString = entry.dynamicString();
   int lineno = -1;
 
   // XXX: it's unclear why the computation of lineno should depend on
   // |dynamicString|. Perhaps it shouldn't?
 
   if (dynamicString) {
-    bool isChromeJSEntry = false;
+    // Adjust the dynamic string as necessary.
+    if (ActivePS::FeaturePrivacy(aLock)) {
+      dynamicString = "(private)";
+    } else if (strlen(dynamicString) >= ProfileBuffer::kMaxFrameKeyLength) {
+      dynamicString = "(too long)";
+    }
+
+    // Store the string using one or more DynamicStringFragment entries.
+    aBuffer.AddDynamicStringEntry(dynamicString);
     if (entry.isJs()) {
       JSScript* script = entry.script();
       if (script) {
-        isChromeJSEntry = IsChromeJSScript(script);
         if (!entry.pc()) {
           // The JIT only allows the top-most entry to have a nullptr pc.
           MOZ_ASSERT(&entry == &aRacyInfo->entries[aRacyInfo->stackSize() - 1]);
         } else {
           lineno = JS_PCToLineNumber(script, entry.pc());
         }
       }
     } else {
       lineno = entry.line();
     }
-
-    // Adjust the dynamic string as necessary.
-    if (ProfilerFeature::HasPrivacy(aFeatures) && !isChromeJSEntry) {
-      dynamicString = "(private)";
-    } else if (strlen(dynamicString) >= ProfileBuffer::kMaxFrameKeyLength) {
-      dynamicString = "(too long)";
-    }
-
   } else {
     // XXX: Bug 1010578. Don't assume a CPP entry and try to get the line for
     // js entries as well.
     if (entry.isCpp()) {
       lineno = entry.line();
     }
   }
 
-  aCollector.CollectCodeLocation(entry.label(), dynamicString, lineno,
-                                 Some(entry.category()));
+  if (lineno != -1) {
+    aBuffer.AddEntry(ProfileBufferEntry::LineNumber(lineno));
+  }
+
+  aBuffer.AddEntry(ProfileBufferEntry::Category(int(entry.category())));
 }
 
 // Setting MAX_NATIVE_FRAMES too high risks the unwinder wasting a lot of time
 // looping on corrupted stacks.
 //
 // The PseudoStack frame size is found in PseudoStack::MaxEntries.
 static const size_t MAX_NATIVE_FRAMES = 1024;
 static const size_t MAX_JS_FRAMES     = 1024;
@@ -756,45 +742,40 @@ struct AutoWalkJSStack
 
   ~AutoWalkJSStack() {
     if (walkAllowed) {
       WALKING_JS_STACK = false;
     }
   }
 };
 
-// Merges the pseudo-stack, native stack, and JS stack, outputting the details
-// to aCollector.
 static void
-MergeStacks(uint32_t aFeatures, bool aIsSynchronous,
-            const ThreadInfo& aThreadInfo, const Registers& aRegs,
-            const NativeStack& aNativeStack,
-            ProfilerStackCollector& aCollector)
+MergeStacksIntoProfile(PSLockRef aLock, bool aIsSynchronous,
+                       const ThreadInfo& aThreadInfo, const Registers& aRegs,
+                       const NativeStack& aNativeStack, ProfileBuffer& aBuffer)
 {
   // WARNING: this function runs within the profiler's "critical section".
-  // WARNING: this function might be called while the profiler is inactive, and
-  //          cannot rely on ActivePS.
 
   NotNull<RacyThreadInfo*> racyInfo = aThreadInfo.RacyInfo();
   js::ProfileEntry* pseudoEntries = racyInfo->entries;
   uint32_t pseudoCount = racyInfo->stackSize();
   JSContext* context = aThreadInfo.mContext;
 
   // Make a copy of the JS stack into a JSFrame array. This is necessary since,
   // like the native stack, the JS stack is iterated youngest-to-oldest and we
   // need to iterate oldest-to-youngest when adding entries to aInfo.
 
   // Synchronous sampling reports an invalid buffer generation to
   // ProfilingFrameIterator to avoid incorrectly resetting the generation of
   // sampled JIT entries inside the JS engine. See note below concerning 'J'
   // entries.
-  uint32_t startBufferGen = UINT32_MAX;
-  if (!aIsSynchronous && aCollector.Generation().isSome()) {
-    startBufferGen = *aCollector.Generation();
-  }
+  uint32_t startBufferGen;
+  startBufferGen = aIsSynchronous
+                 ? UINT32_MAX
+                 : aBuffer.mGeneration;
   uint32_t jsCount = 0;
   JS::ProfilingFrameIterator::Frame jsFrames[MAX_JS_FRAMES];
 
   // Only walk jit stack if profiling frame iterator is turned on.
   if (context && JS::IsProfilingEnabledForContext(context)) {
     AutoWalkJSStack autoWalkJSStack;
     const uint32_t maxFrames = ArrayLength(jsFrames);
 
@@ -896,17 +877,17 @@ MergeStacks(uint32_t aFeatures, bool aIs
     // Check to see if pseudoStack frame is top-most.
     if (pseudoStackAddr > jsStackAddr && pseudoStackAddr > nativeStackAddr) {
       MOZ_ASSERT(pseudoIndex < pseudoCount);
       js::ProfileEntry& pseudoEntry = pseudoEntries[pseudoIndex];
 
       // Pseudo-frames with the CPP_MARKER_FOR_JS kind are just annotations and
       // should not be recorded in the profile.
       if (pseudoEntry.kind() != js::ProfileEntry::Kind::CPP_MARKER_FOR_JS) {
-        AddPseudoEntry(aFeatures, racyInfo, pseudoEntry, aCollector);
+        AddPseudoEntry(aLock, racyInfo, pseudoEntry, aBuffer);
       }
       pseudoIndex++;
       continue;
     }
 
     // Check to see if JS jit stack frame is top-most
     if (jsStackAddr > nativeStackAddr) {
       MOZ_ASSERT(jsIndex >= 0);
@@ -922,48 +903,49 @@ MergeStacks(uint32_t aFeatures, bool aIs
       // amount of time, such as in nsRefreshDriver. Problematically, the
       // stored backtrace may be alive across a GC during which the profiler
       // itself is disabled. In that case, the JS engine is free to discard its
       // JIT code. This means that if we inserted such OptInfoAddr entries into
       // the buffer, nsRefreshDriver would now be holding on to a backtrace
       // with stale JIT code return addresses.
       if (aIsSynchronous ||
           jsFrame.kind == JS::ProfilingFrameIterator::Frame_Wasm) {
-        aCollector.CollectCodeLocation("", jsFrame.label, -1, Nothing());
+        aBuffer.AddEntry(ProfileBufferEntry::Label(""));
+        aBuffer.AddDynamicStringEntry(jsFrame.label);
       } else {
         MOZ_ASSERT(jsFrame.kind == JS::ProfilingFrameIterator::Frame_Ion ||
                    jsFrame.kind == JS::ProfilingFrameIterator::Frame_Baseline);
-        aCollector.CollectJitReturnAddr(jsFrames[jsIndex].returnAddress);
+        aBuffer.AddEntry(
+          ProfileBufferEntry::JitReturnAddr(jsFrames[jsIndex].returnAddress));
       }
 
       jsIndex--;
       continue;
     }
 
     // If we reach here, there must be a native stack entry and it must be the
     // greatest entry.
     if (nativeStackAddr) {
       MOZ_ASSERT(nativeIndex >= 0);
       void* addr = (void*)aNativeStack.mPCs[nativeIndex];
-      aCollector.CollectNativeLeafAddr(addr);
+      aBuffer.AddEntry(ProfileBufferEntry::NativeLeafAddr(addr));
     }
     if (nativeIndex >= 0) {
       nativeIndex--;
     }
   }
 
   // Update the JS context with the current profile sample buffer generation.
   //
   // Do not do this for synchronous samples, which use their own
   // ProfileBuffers instead of the global one in CorePS.
-  if (!aIsSynchronous && context && aCollector.Generation().isSome()) {
-    MOZ_ASSERT(*aCollector.Generation() >= startBufferGen);
-    uint32_t lapCount = *aCollector.Generation() - startBufferGen;
-    JS::UpdateJSContextProfilerSampleBufferGen(context,
-                                               *aCollector.Generation(),
+  if (!aIsSynchronous && context) {
+    MOZ_ASSERT(aBuffer.mGeneration >= startBufferGen);
+    uint32_t lapCount = aBuffer.mGeneration - startBufferGen;
+    JS::UpdateJSContextProfilerSampleBufferGen(context, aBuffer.mGeneration,
                                                lapCount);
   }
 }
 
 #if defined(GP_OS_windows)
 static uintptr_t GetThreadHandle(PlatformData* aData);
 #endif
 
@@ -978,18 +960,16 @@ StackWalkCallback(uint32_t aFrameNumber,
   nativeStack->mCount++;
 }
 
 static void
 DoNativeBacktrace(PSLockRef aLock, const ThreadInfo& aThreadInfo,
                   const Registers& aRegs, NativeStack& aNativeStack)
 {
   // WARNING: this function runs within the profiler's "critical section".
-  // WARNING: this function might be called while the profiler is inactive, and
-  //          cannot rely on ActivePS.
 
   // Start with the current function. We use 0 as the frame number here because
   // the FramePointerStackWalk() and MozStackWalk() calls below will use 1..N.
   // This is a bit weird but it doesn't matter because StackWalkCallback()
   // doesn't use the frame number argument.
   StackWalkCallback(/* frameNum */ 0, aRegs.mPC, aRegs.mSP, &aNativeStack);
 
   uint32_t maxFrames = uint32_t(MAX_NATIVE_FRAMES - aNativeStack.mCount);
@@ -1013,18 +993,16 @@ DoNativeBacktrace(PSLockRef aLock, const
 #endif
 
 #ifdef USE_EHABI_STACKWALK
 static void
 DoNativeBacktrace(PSLockRef aLock, const ThreadInfo& aThreadInfo,
                   const Registers& aRegs, NativeStack& aNativeStack)
 {
   // WARNING: this function runs within the profiler's "critical section".
-  // WARNING: this function might be called while the profiler is inactive, and
-  //          cannot rely on ActivePS.
 
   const mcontext_t* mcontext = &aRegs.mContext->uc_mcontext;
   mcontext_t savedContext;
   NotNull<RacyThreadInfo*> racyInfo = aThreadInfo.RacyInfo();
 
   // The pseudostack contains an "EnterJIT" frame whenever we enter
   // JIT code with profiling enabled; the stack pointer value points
   // the saved registers.  We use this to unwind resume unwinding
@@ -1094,18 +1072,16 @@ ASAN_memcpy(void* aDst, const void* aSrc
 }
 #endif
 
 static void
 DoNativeBacktrace(PSLockRef aLock, const ThreadInfo& aThreadInfo,
                   const Registers& aRegs, NativeStack& aNativeStack)
 {
   // WARNING: this function runs within the profiler's "critical section".
-  // WARNING: this function might be called while the profiler is inactive, and
-  //          cannot rely on ActivePS.
 
   const mcontext_t* mc = &aRegs.mContext->uc_mcontext;
 
   lul::UnwindRegs startRegs;
   memset(&startRegs, 0, sizeof(startRegs));
 
 #if defined(GP_PLAT_amd64_linux)
   startRegs.xip = lul::TaggedUWord(mc->gregs[REG_RIP]);
@@ -1241,23 +1217,23 @@ DoSharedSample(PSLockRef aLock, bool aIs
   TimeDuration delta = aNow - CorePS::ProcessStartTime();
   aBuffer.AddEntry(ProfileBufferEntry::Time(delta.ToMilliseconds()));
 
   NativeStack nativeStack;
 #if defined(HAVE_NATIVE_UNWIND)
   if (ActivePS::FeatureStackWalk(aLock)) {
     DoNativeBacktrace(aLock, aThreadInfo, aRegs, nativeStack);
 
-    MergeStacks(ActivePS::Features(aLock), aIsSynchronous, aThreadInfo, aRegs,
-                nativeStack, aBuffer);
+    MergeStacksIntoProfile(aLock, aIsSynchronous, aThreadInfo, aRegs,
+                           nativeStack, aBuffer);
   } else
 #endif
   {
-    MergeStacks(ActivePS::Features(aLock), aIsSynchronous, aThreadInfo, aRegs,
-                nativeStack, aBuffer);
+    MergeStacksIntoProfile(aLock, aIsSynchronous, aThreadInfo, aRegs,
+                           nativeStack, aBuffer);
 
     if (ActivePS::FeatureLeaf(aLock)) {
       aBuffer.AddEntry(ProfileBufferEntry::NativeLeafAddr((void*)aRegs.mPC));
     }
   }
 }
 
 // Writes the components of a synchronous sample to the given ProfileBuffer.
@@ -3065,67 +3041,10 @@ profiler_suspend_and_sample_thread(
       // NOTE: Make sure to disable the sampler before it is destroyed, in case
       // the profiler is running at the same time.
       sampler.Disable(lock);
       break;
     }
   }
 }
 
-// NOTE: aCollector's methods will be called while the target thread is paused.
-// Doing things in those methods like allocating -- which may try to claim
-// locks -- is a surefire way to deadlock.
-void
-profiler_suspend_and_sample_thread(int aThreadId,
-                                   uint32_t aFeatures,
-                                   ProfilerStackCollector& aCollector,
-                                   bool aSampleNative /* = true */)
-{
-  // Lock the profiler mutex
-  PSAutoLock lock(gPSMutex);
-
-  const CorePS::ThreadVector& liveThreads = CorePS::LiveThreads(lock);
-  for (uint32_t i = 0; i < liveThreads.size(); i++) {
-    ThreadInfo* info = liveThreads.at(i);
-
-    if (info->ThreadId() == aThreadId) {
-      if (info->IsMainThread()) {
-        aCollector.SetIsMainThread();
-      }
-
-      // Allocate the space for the native stack
-      NativeStack nativeStack;
-
-      // Suspend, sample, and then resume the target thread.
-      Sampler sampler(lock);
-      sampler.SuspendAndSampleAndResumeThread(lock, *info,
-                                              [&](const Registers& aRegs) {
-        // The target thread is now suspended. Collect a native backtrace, and
-        // call the callback.
-        bool isSynchronous = false;
-#if defined(HAVE_NATIVE_UNWIND)
-        if (aSampleNative) {
-          DoNativeBacktrace(lock, *info, aRegs, nativeStack);
-
-          MergeStacks(aFeatures, isSynchronous, *info, aRegs, nativeStack,
-                      aCollector);
-        } else
-#endif
-        {
-          MergeStacks(aFeatures, isSynchronous, *info, aRegs, nativeStack,
-                      aCollector);
-
-          if (ProfilerFeature::HasLeaf(aFeatures)) {
-            aCollector.CollectNativeLeafAddr((void*)aRegs.mPC);
-          }
-        }
-      });
-
-      // NOTE: Make sure to disable the sampler before it is destroyed, in case
-      // the profiler is running at the same time.
-      sampler.Disable(lock);
-      break;
-    }
-  }
-}
-
 // END externally visible functions
 ////////////////////////////////////////////////////////////////////////
--- a/tools/profiler/moz.build
+++ b/tools/profiler/moz.build
@@ -76,17 +76,16 @@ if CONFIG['MOZ_GECKO_PROFILER']:
             'core/shared-libraries-macos.cc',
         ]
     elif CONFIG['OS_TARGET'] == 'WINNT':
         SOURCES += [
             'core/shared-libraries-win32.cc',
         ]
 
     LOCAL_INCLUDES += [
-        '/caps',
         '/docshell/base',
         '/ipc/chromium/src',
         '/mozglue/linker',
         '/toolkit/crashreporter/google-breakpad/src',
         '/tools/profiler/core/',
         '/tools/profiler/gecko/',
         '/xpcom/base',
     ]
--- a/tools/profiler/public/GeckoProfiler.h
+++ b/tools/profiler/public/GeckoProfiler.h
@@ -20,17 +20,16 @@
 #include <signal.h>
 #include <stdarg.h>
 #include <stdint.h>
 #include <stdlib.h>
 
 #include "mozilla/Assertions.h"
 #include "mozilla/Attributes.h"
 #include "mozilla/GuardObjects.h"
-#include "mozilla/Maybe.h"
 #include "mozilla/Sprintf.h"
 #include "mozilla/ThreadLocal.h"
 #include "mozilla/UniquePtr.h"
 #include "js/TypeDecls.h"
 #include "js/ProfilingStack.h"
 #include "nscore.h"
 
 // Make sure that we can use std::min here without the Windows headers messing
@@ -263,62 +262,21 @@ PROFILER_FUNC(int profiler_current_threa
 typedef void ProfilerStackCallback(void** aPCs, size_t aCount, bool aIsMainThread);
 
 // This method suspends the thread identified by aThreadId, optionally samples
 // it for its native stack, and then calls the callback.
 //
 // WARNING: The target thread is suspended during the callback. Do not try to
 // allocate or acquire any locks, or you could deadlock. The target thread will
 // have resumed by the time this function returns.
-//
-// XXX: this function is in the process of being replaced with the other profiler_suspend_and_sample_thread() function.
 PROFILER_FUNC_VOID(
   profiler_suspend_and_sample_thread(int aThreadId,
                                      const std::function<ProfilerStackCallback>& aCallback,
                                      bool aSampleNative = true))
 
-// An object of this class is passed to profiler_suspend_and_sample_thread().
-// For each stack frame, one of the Collect methods will be called.
-class ProfilerStackCollector
-{
-public:
-  // Some collectors need to worry about possibly overwriting previous
-  // generations of data. If that's not an issue, this can return Nothing,
-  // which is the default behaviour.
-  virtual mozilla::Maybe<uint32_t> Generation() { return mozilla::Nothing(); }
-
-  // This method will be called once if the thread being suspended is the main
-  // thread. Default behaviour is to do nothing.
-  virtual void SetIsMainThread() {}
-
-  // WARNING: The target thread is suspended when the Collect methods are
-  // called. Do not try to allocate or acquire any locks, or you could
-  // deadlock. The target thread will have resumed by the time this function
-  // returns.
-
-  virtual void CollectNativeLeafAddr(void* aAddr) = 0;
-
-  virtual void CollectJitReturnAddr(void* aAddr) = 0;
-
-  // aLabel is static and never null. aStr may be null. aLineNumber may be -1.
-  virtual void CollectCodeLocation(
-    const char* aLabel, const char* aStr, int aLineNumber,
-    const mozilla::Maybe<js::ProfileEntry::Category>& aCategory) = 0;
-};
-
-// This method suspends the thread identified by aThreadId, samples its
-// pseudo-stack, JS stack, and (optionally) native stack, passing the collected
-// frames into aCollector. aFeatures dictates which compiler features are used.
-// |Privacy| and |Leaf| are the only relevant ones.
-PROFILER_FUNC_VOID(
-  profiler_suspend_and_sample_thread(int aThreadId,
-                                     uint32_t aFeatures,
-                                     ProfilerStackCollector& aCollector,
-                                     bool aSampleNative = true))
-
 struct ProfilerBacktraceDestructor
 {
 #ifdef MOZ_GECKO_PROFILER
   void operator()(ProfilerBacktrace*);
 #else
   void operator()(ProfilerBacktrace*) {}
 #endif
 };
--- a/tools/profiler/tests/gtest/GeckoProfiler.cpp
+++ b/tools/profiler/tests/gtest/GeckoProfiler.cpp
@@ -659,75 +659,8 @@ TEST(GeckoProfiler, Bug1355807)
 
   // In bug 1355807 this caused an assertion failure in StopJSSampling().
   profiler_start(PROFILER_DEFAULT_ENTRIES, PROFILER_DEFAULT_INTERVAL,
                  features,
                  fewThreadsFilter, MOZ_ARRAY_LENGTH(fewThreadsFilter));
 
   profiler_stop();
 }
-
-class GTestStackCollector final : public ProfilerStackCollector
-{
-public:
-  GTestStackCollector()
-    : mSetIsMainThread(0)
-    , mFrames(0)
-  {}
-
-  virtual void SetIsMainThread() { mSetIsMainThread++; }
-
-  virtual void CollectNativeLeafAddr(void* aAddr) { mFrames++; }
-  virtual void CollectJitReturnAddr(void* aAddr) { mFrames++; }
-  virtual void CollectCodeLocation(
-    const char* aLabel, const char* aStr, int aLineNumber,
-    const mozilla::Maybe<js::ProfileEntry::Category>& aCategory) { mFrames++; }
-
-  int mSetIsMainThread;
-  int mFrames;
-};
-
-void DoSuspendAndSample(int aTid, nsIThread* aThread)
-{
-  aThread->Dispatch(
-    NS_NewRunnableFunction(
-      "GeckoProfiler_SuspendAndSample_Test::TestBody",
-      [&]() {
-        uint32_t features = ProfilerFeature::Leaf;
-        GTestStackCollector collector;
-        profiler_suspend_and_sample_thread(aTid, features, collector,
-                                           /* sampleNative = */ true);
-
-        ASSERT_TRUE(collector.mSetIsMainThread == 1);
-        ASSERT_TRUE(collector.mFrames > 5); // approximate; must be > 0
-      }),
-    NS_DISPATCH_SYNC);
-}
-
-TEST(GeckoProfiler, SuspendAndSample)
-{
-  nsCOMPtr<nsIThread> thread;
-  nsresult rv = NS_NewNamedThread("GeckoProfGTest", getter_AddRefs(thread));
-  ASSERT_TRUE(NS_SUCCEEDED(rv));
-
-  int tid = Thread::GetCurrentId();
-
-  ASSERT_TRUE(!profiler_is_active());
-
-  // Suspend and sample while the profiler is inactive.
-  DoSuspendAndSample(tid, thread);
-
-  uint32_t features = ProfilerFeature::JS | ProfilerFeature::Threads;
-  const char* filters[] = { "GeckoMain", "Compositor" };
-
-  profiler_start(PROFILER_DEFAULT_ENTRIES, PROFILER_DEFAULT_INTERVAL,
-                 features, filters, MOZ_ARRAY_LENGTH(filters));
-
-  ASSERT_TRUE(profiler_is_active());
-
-  // Suspend and sample while the profiler is active.
-  DoSuspendAndSample(tid, thread);
-
-  profiler_stop();
-
-  ASSERT_TRUE(!profiler_is_active());
-}
-