Bug 1348959 - Make profiler_get_buffer_info() return information in a struct instead of using outparams. r=mystor,njn

MozReview-Commit-ID: 1iJ05NxOdou

--HG--
extra : rebase_source : 1c4e654340630b0f21c165b41b8fc1be30743c48
extra : source : 64b1a4108f6c3319cfc74c1246bdd2e895e93e1a

tools/profiler/core/platform.cpp

@@ -2684,25 +2684,23 @@ profiler_get_available_features()
   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

tools/profiler/gecko/nsProfiler.cpp

@@ -494,7 +494,16 @@ nsProfiler::GetBufferInfo(uint32_t* aCurrentPosition, uint32_t* aTotalSize,
   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;
 }
 

tools/profiler/public/GeckoProfiler.h

@@ -347,30 +347,21 @@ using UniqueProfilerBacktrace =
 // 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();

tools/profiler/tests/gtest/GeckoProfiler.cpp

@@ -193,9 +193,8 @@ TEST(GeckoProfiler, EnsureStarted)
     // 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!
@@ -207,17 +206,16 @@ TEST(GeckoProfiler, EnsureStarted)
 
     // 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
@@ -230,10 +228,10 @@ TEST(GeckoProfiler, EnsureStarted)
     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);
   }
 
   {
@@ -382,24 +380,21 @@ TEST(GeckoProfiler, Pause)
 
   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();