Bug 1348959 - Make profiler_get_buffer_info() return information in a struct instead of using outparams. r=mystor,njn
authorMarkus Stange <mstange@themasta.com>
Thu, 18 Jan 2018 17:54:57 -0500
changeset 402629 5ca04ee79f78722b7ef44f496f3cb4a2b7b7079a
parent 402628 0f792f685c04216a43409145f5abdaf60c64aa41
child 402630 1406e2ac322a6631536cf80e931bd465fccea141
push id33394
push userrgurzau@mozilla.com
push dateWed, 07 Feb 2018 00:14:43 +0000
treeherdermozilla-central@f033d62a90ad [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersmystor, njn
bugs1348959
milestone60.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 1348959 - Make profiler_get_buffer_info() return information in a struct instead of using outparams. r=mystor,njn MozReview-Commit-ID: 1iJ05NxOdou
tools/profiler/core/platform.cpp
tools/profiler/gecko/nsProfiler.cpp
tools/profiler/public/GeckoProfiler.h
tools/profiler/tests/gtest/GeckoProfiler.cpp
--- a/tools/profiler/core/platform.cpp
+++ b/tools/profiler/core/platform.cpp
@@ -2679,35 +2679,33 @@ profiler_get_available_features()
 #endif
 #if !defined(MOZ_TASK_TRACER)
   ProfilerFeature::ClearTaskTracer(features);
 #endif
 
   return features;
 }
 
-void
-profiler_get_buffer_info_helper(uint32_t* aCurrentPosition,
-                                uint32_t* aEntries,
-                                uint32_t* aGeneration)
+Maybe<ProfilerBufferInfo>
+profiler_get_buffer_info()
 {
-  // This function is called by profiler_get_buffer_info(), which has already
-  // zeroed the outparams.
-
   MOZ_RELEASE_ASSERT(CorePS::Exists());
 
   PSAutoLock lock(gPSMutex);
 
   if (!ActivePS::Exists(lock)) {
-    return;
+    return Nothing();
   }
 
-  *aCurrentPosition = ActivePS::Buffer(lock).mWritePos;
-  *aEntries = ActivePS::Entries(lock);
-  *aGeneration = ActivePS::Buffer(lock).mGeneration;
+  return Some(ProfilerBufferInfo {
+    ActivePS::Buffer(lock).mWritePos,
+    ActivePS::Buffer(lock).mReadPos,
+    ActivePS::Buffer(lock).mGeneration,
+    ActivePS::Entries(lock)
+  });
 }
 
 static void
 PollJSSamplingForCurrentThread()
 {
   MOZ_RELEASE_ASSERT(CorePS::Exists());
 
   PSAutoLock lock(gPSMutex);
--- a/tools/profiler/gecko/nsProfiler.cpp
+++ b/tools/profiler/gecko/nsProfiler.cpp
@@ -489,17 +489,26 @@ nsProfiler::GetStartParams(nsIProfilerSt
 
 NS_IMETHODIMP
 nsProfiler::GetBufferInfo(uint32_t* aCurrentPosition, uint32_t* aTotalSize,
                           uint32_t* aGeneration)
 {
   MOZ_ASSERT(aCurrentPosition);
   MOZ_ASSERT(aTotalSize);
   MOZ_ASSERT(aGeneration);
-  profiler_get_buffer_info(aCurrentPosition, aTotalSize, aGeneration);
+  Maybe<ProfilerBufferInfo> info = profiler_get_buffer_info();
+  if (info) {
+    *aCurrentPosition = info->mWritePosition;
+    *aTotalSize = info->mEntryCount;
+    *aGeneration = info->mGeneration;
+  } else {
+    *aCurrentPosition = 0;
+    *aTotalSize = 0;
+    *aGeneration = 0;
+  }
   return NS_OK;
 }
 
 void
 nsProfiler::GatheredOOPProfile(const nsACString& aProfile)
 {
   MOZ_RELEASE_ASSERT(NS_IsMainThread());
 
--- a/tools/profiler/public/GeckoProfiler.h
+++ b/tools/profiler/public/GeckoProfiler.h
@@ -342,40 +342,31 @@ struct ProfilerBacktraceDestructor
 
 using UniqueProfilerBacktrace =
   mozilla::UniquePtr<ProfilerBacktrace, ProfilerBacktraceDestructor>;
 
 // Immediately capture the current thread's call stack and return it. A no-op
 // if the profiler is inactive or in privacy mode.
 UniqueProfilerBacktrace profiler_get_backtrace();
 
-// Get information about the current buffer status. A no-op when the profiler
-// is inactive. Do not call this function; call profiler_get_buffer_info()
-// instead.
-void profiler_get_buffer_info_helper(uint32_t* aCurrentPosition,
-                                     uint32_t* aEntries,
-                                     uint32_t* aGeneration);
+struct ProfilerBufferInfo
+{
+  uint32_t mWritePosition;
+  uint32_t mReadPosition;
+  uint32_t mGeneration;
+  uint32_t mEntryCount;
+};
 
-// Get information about the current buffer status. Returns (via outparams) the
-// current write position in the buffer, the total size of the buffer, and the
-// generation of the buffer. Returns zeroes if the profiler is inactive.
+// Get information about the current buffer status.
+// Returns Nothing() if the profiler is inactive.
 //
 // This information may be useful to a user-interface displaying the current
 // status of the profiler, allowing the user to get a sense for how fast the
 // buffer is being written to, and how much data is visible.
-static inline void profiler_get_buffer_info(uint32_t* aCurrentPosition,
-                                            uint32_t* aEntries,
-                                            uint32_t* aGeneration)
-{
-  *aCurrentPosition = 0;
-  *aEntries = 0;
-  *aGeneration = 0;
-
-  profiler_get_buffer_info_helper(aCurrentPosition, aEntries, aGeneration);
-}
+mozilla::Maybe<ProfilerBufferInfo> profiler_get_buffer_info();
 
 // Get the current thread's PseudoStack.
 PseudoStack* profiler_get_pseudo_stack();
 
 //---------------------------------------------------------------------------
 // Put profiling data into the profiler (labels and markers)
 //---------------------------------------------------------------------------
 
--- a/tools/profiler/tests/gtest/GeckoProfiler.cpp
+++ b/tools/profiler/tests/gtest/GeckoProfiler.cpp
@@ -188,57 +188,55 @@ TEST(GeckoProfiler, EnsureStarted)
   }
 
   {
     // Active -> Active with same settings
 
     // First, write some samples into the buffer.
     PR_Sleep(PR_MillisecondsToInterval(500));
 
-    uint32_t currPos1, entries1, generation1;
-    profiler_get_buffer_info(&currPos1, &entries1, &generation1);
-    ASSERT_TRUE(generation1 > 0 || currPos1 > 0);
+    Maybe<ProfilerBufferInfo> info1 = profiler_get_buffer_info();
+    ASSERT_TRUE(info1->mGeneration > 0 || info1->mWritePosition > 0);
 
     // Call profiler_ensure_started with the same settings as before.
     // This operation must not clear our buffer!
     profiler_ensure_started(PROFILER_DEFAULT_ENTRIES, PROFILER_DEFAULT_INTERVAL,
                             features, filters, MOZ_ARRAY_LENGTH(filters));
 
     ActiveParamsCheck(PROFILER_DEFAULT_ENTRIES, PROFILER_DEFAULT_INTERVAL,
                       features, filters, MOZ_ARRAY_LENGTH(filters));
 
     // Check that our position in the buffer stayed the same or advanced.
     // In particular, it shouldn't have reverted to the start.
-    uint32_t currPos2, entries2, generation2;
-    profiler_get_buffer_info(&currPos2, &entries2, &generation2);
-    ASSERT_TRUE(generation2 >= generation1);
-    ASSERT_TRUE(generation2 > generation1 || currPos2 >= currPos1);
+    Maybe<ProfilerBufferInfo> info2 = profiler_get_buffer_info();
+    ASSERT_TRUE(info2->mGeneration >= info1->mGeneration);
+    ASSERT_TRUE(info2->mGeneration > info1->mGeneration ||
+                info2->mWritePosition >= info1->mWritePosition);
   }
 
   {
     // Active -> Active with *different* settings
 
-    uint32_t currPos1, entries1, generation1;
-    profiler_get_buffer_info(&currPos1, &entries1, &generation1);
+    Maybe<ProfilerBufferInfo> info1 = profiler_get_buffer_info();
 
     // Call profiler_ensure_started with a different feature set than the one it's
     // currently running with. This is supposed to stop and restart the
     // profiler, thereby discarding the buffer contents.
     uint32_t differentFeatures = features | ProfilerFeature::Leaf;
     profiler_ensure_started(PROFILER_DEFAULT_ENTRIES, PROFILER_DEFAULT_INTERVAL,
                             differentFeatures,
                             filters, MOZ_ARRAY_LENGTH(filters));
 
     ActiveParamsCheck(PROFILER_DEFAULT_ENTRIES, PROFILER_DEFAULT_INTERVAL,
                       differentFeatures, filters, MOZ_ARRAY_LENGTH(filters));
 
-    uint32_t currPos2, entries2, generation2;
-    profiler_get_buffer_info(&currPos2, &entries2, &generation2);
-    ASSERT_TRUE(generation2 <= generation1);
-    ASSERT_TRUE(generation2 < generation1 || currPos2 < currPos1);
+    Maybe<ProfilerBufferInfo> info2 = profiler_get_buffer_info();
+    ASSERT_TRUE(info2->mGeneration <= info1->mGeneration);
+    ASSERT_TRUE(info2->mGeneration < info1->mGeneration ||
+                info2->mWritePosition < info1->mWritePosition);
   }
 
   {
     // Active -> Inactive
 
     profiler_stop();
 
     InactiveFeaturesAndParamsCheck();
@@ -377,34 +375,31 @@ TEST(GeckoProfiler, Pause)
 
   ASSERT_TRUE(!profiler_is_paused());
 
   profiler_start(PROFILER_DEFAULT_ENTRIES, PROFILER_DEFAULT_INTERVAL,
                  features, filters, MOZ_ARRAY_LENGTH(filters));
 
   ASSERT_TRUE(!profiler_is_paused());
 
-  uint32_t currPos1, entries1, generation1;
-  uint32_t currPos2, entries2, generation2;
-
   // Check that we are writing samples while not paused.
-  profiler_get_buffer_info(&currPos1, &entries1, &generation1);
+  Maybe<ProfilerBufferInfo> info1 = profiler_get_buffer_info();
   PR_Sleep(PR_MillisecondsToInterval(500));
-  profiler_get_buffer_info(&currPos2, &entries2, &generation2);
-  ASSERT_TRUE(currPos1 != currPos2);
+  Maybe<ProfilerBufferInfo> info2 = profiler_get_buffer_info();
+  ASSERT_TRUE(info1->mWritePosition != info2->mWritePosition);
 
   profiler_pause();
 
   ASSERT_TRUE(profiler_is_paused());
 
   // Check that we are not writing samples while paused.
-  profiler_get_buffer_info(&currPos1, &entries1, &generation1);
+  info1 = profiler_get_buffer_info();
   PR_Sleep(PR_MillisecondsToInterval(500));
-  profiler_get_buffer_info(&currPos2, &entries2, &generation2);
-  ASSERT_TRUE(currPos1 == currPos2);
+  info2 = profiler_get_buffer_info();
+  ASSERT_TRUE(info1->mWritePosition == info2->mWritePosition);
 
   profiler_resume();
 
   ASSERT_TRUE(!profiler_is_paused());
 
   profiler_stop();
 
   ASSERT_TRUE(!profiler_is_paused());