Bug 1348959 - Make ProfileBuffer fields uint32_t. r?mystor, r?njn draft
authorMarkus Stange <mstange@themasta.com>
Thu, 18 Jan 2018 17:54:33 -0500
changeset 751650 7df86065dfd180ed5d4181ba506349cb898c54b3
parent 751641 c6852f03a2f32029c04735687a4bc13c3d467ab5
child 751651 442a79190178ff156146fe1281c0f75be225409b
push id98034
push userbmo:mstange@themasta.com
push dateTue, 06 Feb 2018 21:39:48 +0000
reviewersmystor, njn
bugs1348959
milestone60.0a1
Bug 1348959 - Make ProfileBuffer fields uint32_t. r?mystor, r?njn MozReview-Commit-ID: veIGhEQyK5
tools/profiler/core/ProfileBuffer.cpp
tools/profiler/core/ProfileBuffer.h
tools/profiler/core/ProfileBufferEntry.cpp
tools/profiler/core/platform.cpp
tools/profiler/public/GeckoProfiler.h
tools/profiler/tests/gtest/ThreadProfileTest.cpp
--- a/tools/profiler/core/ProfileBuffer.cpp
+++ b/tools/profiler/core/ProfileBuffer.cpp
@@ -8,17 +8,17 @@
 
 #include "ProfilerMarker.h"
 #include "jsfriendapi.h"
 #include "nsScriptSecurityManager.h"
 #include "nsJSPrincipals.h"
 
 using namespace mozilla;
 
-ProfileBuffer::ProfileBuffer(int aEntrySize)
+ProfileBuffer::ProfileBuffer(uint32_t aEntrySize)
   : mEntries(mozilla::MakeUnique<ProfileBufferEntry[]>(aEntrySize))
   , mWritePos(0)
   , mReadPos(0)
   , mEntrySize(aEntrySize)
   , mGeneration(0)
 {
 }
 
@@ -50,17 +50,17 @@ ProfileBuffer::AddEntry(const ProfileBuf
 }
 
 void
 ProfileBuffer::AddThreadIdEntry(int aThreadId, LastSample* aLS)
 {
   if (aLS) {
     // This is the start of a sample, so make a note of its location in |aLS|.
     aLS->mGeneration = mGeneration;
-    aLS->mPos = mWritePos;
+    aLS->mPos = Some(mWritePos);
   }
   AddEntry(ProfileBufferEntry::ThreadId(aThreadId));
 }
 
 void
 ProfileBuffer::AddStoredMarker(ProfilerMarker *aStoredMarker)
 {
   aStoredMarker->SetGeneration(mGeneration);
--- a/tools/profiler/core/ProfileBuffer.h
+++ b/tools/profiler/core/ProfileBuffer.h
@@ -11,33 +11,33 @@
 #include "ProfilerMarker.h"
 #include "ProfileJSONWriter.h"
 #include "mozilla/RefPtr.h"
 #include "mozilla/RefCounted.h"
 
 class ProfileBuffer final
 {
 public:
-  explicit ProfileBuffer(int aEntrySize);
+  explicit ProfileBuffer(uint32_t 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
   // global ProfileBuffer, but *not* for synchronous samples.
   struct LastSample {
     LastSample()
       : mGeneration(0)
-      , mPos(-1)
+      , mPos()
     {}
 
     // The profiler-buffer generation number at which the sample was created.
     uint32_t mGeneration;
-    // And its position in the buffer, or -1 meaning "invalid".
-    int mPos;
+    // And its position in the buffer.
+    mozilla::Maybe<uint32_t> mPos;
   };
 
   // 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);
@@ -70,31 +70,31 @@ public:
 
   // The following two methods are not signal safe! They delete markers.
   void DeleteExpiredStoredMarkers();
   void Reset();
 
   size_t SizeOfIncludingThis(mozilla::MallocSizeOf aMallocSizeOf) const;
 
 private:
-  int FindLastSampleOfThread(int aThreadId, const LastSample& aLS) const;
+  mozilla::Maybe<uint32_t> FindLastSampleOfThread(int aThreadId, const LastSample& aLS) const;
 
 public:
   // Circular buffer 'Keep One Slot Open' implementation for simplicity
   mozilla::UniquePtr<ProfileBufferEntry[]> mEntries;
 
   // Points to the next entry we will write to, which is also the one at which
   // we need to stop reading.
-  int mWritePos;
+  uint32_t mWritePos;
 
   // Points to the entry at which we can start reading.
-  int mReadPos;
+  uint32_t mReadPos;
 
   // The number of entries in our buffer.
-  int mEntrySize;
+  uint32_t mEntrySize;
 
   // How many times mWritePos has wrapped around.
   uint32_t mGeneration;
 
   // Markers that marker entries in the buffer might refer to.
   ProfilerMarkerLinkedList mStoredMarkers;
 };
 
--- a/tools/profiler/core/ProfileBufferEntry.cpp
+++ b/tools/profiler/core/ProfileBufferEntry.cpp
@@ -581,19 +581,19 @@ public:
   {}
 
   bool Has() const { return mReadPos != mWritePos; }
   const ProfileBufferEntry& Get() const { return mEntries[mReadPos]; }
   void Next() { mReadPos = (mReadPos + 1) % mEntrySize; }
 
 private:
   const ProfileBufferEntry* const mEntries;
-  int mReadPos;
-  const int mWritePos;
-  const int mEntrySize;
+  uint32_t mReadPos;
+  const uint32_t mWritePos;
+  const uint32_t mEntrySize;
 };
 
 // The following grammar shows legal sequences of profile buffer entries.
 // The sequences beginning with a ThreadId entry are known as "samples".
 //
 // (
 //   (
 //     ThreadId
@@ -995,65 +995,68 @@ ProfileBuffer::StreamPausedRangesToJSON(
                    currentPauseStartTime, Nothing());
   }
   if (currentCollectionStartTime) {
     AddPausedRange(aWriter, "collecting",
                    currentCollectionStartTime, Nothing());
   }
 }
 
-int
+Maybe<uint32_t>
 ProfileBuffer::FindLastSampleOfThread(int aThreadId, const LastSample& aLS)
   const
 {
   // |aLS| has a valid generation number if either it matches the buffer's
   // generation, or is one behind the buffer's generation, since the buffer's
   // generation is incremented on wraparound.  There's no ambiguity relative to
   // ProfileBuffer::reset, since that increments mGeneration by two.
   if (aLS.mGeneration == mGeneration ||
       (mGeneration > 0 && aLS.mGeneration == mGeneration - 1)) {
-    int ix = aLS.mPos;
-
-    if (ix == -1) {
+    if (!aLS.mPos) {
       // There's no record of |aLS|'s thread ever having recorded a sample in
       // the buffer.
-      return -1;
+      return Nothing();
     }
 
+    uint32_t ix = *aLS.mPos;
+
     // It might be that the sample has since been overwritten, so check that it
     // is still valid.
     MOZ_RELEASE_ASSERT(0 <= ix && ix < mEntrySize);
     ProfileBufferEntry& entry = mEntries[ix];
     bool isStillValid = entry.IsThreadId() && entry.u.mInt == aThreadId;
-    return isStillValid ? ix : -1;
+    return isStillValid ? Some(ix) : Nothing();
   }
 
   // |aLS| denotes a sample which is older than either two wraparounds or one
   // call to ProfileBuffer::reset.  In either case it is no longer valid.
   MOZ_ASSERT(aLS.mGeneration <= mGeneration - 2);
-  return -1;
+  return Nothing();
 }
 
 bool
 ProfileBuffer::DuplicateLastSample(int aThreadId,
                                    const TimeStamp& aProcessStartTime,
                                    LastSample& aLS)
 {
-  int lastSampleStartPos = FindLastSampleOfThread(aThreadId, aLS);
-  if (lastSampleStartPos == -1) {
+  Maybe<uint32_t> maybeLastSampleStartPos =
+    FindLastSampleOfThread(aThreadId, aLS);
+  if (!maybeLastSampleStartPos) {
     return false;
   }
 
+  uint32_t lastSampleStartPos = *maybeLastSampleStartPos;
+
   MOZ_ASSERT(mEntries[lastSampleStartPos].IsThreadId() &&
              mEntries[lastSampleStartPos].u.mInt == aThreadId);
 
   AddThreadIdEntry(aThreadId, &aLS);
 
   // Go through the whole entry and duplicate it, until we find the next one.
-  for (int readPos = (lastSampleStartPos + 1) % mEntrySize;
+  for (uint32_t readPos = (lastSampleStartPos + 1) % mEntrySize;
        readPos != mWritePos;
        readPos = (readPos + 1) % mEntrySize) {
     switch (mEntries[readPos].GetKind()) {
       case ProfileBufferEntry::Kind::Pause:
       case ProfileBufferEntry::Kind::Resume:
       case ProfileBufferEntry::Kind::CollectionStart:
       case ProfileBufferEntry::Kind::CollectionEnd:
       case ProfileBufferEntry::Kind::ThreadId:
--- a/tools/profiler/core/platform.cpp
+++ b/tools/profiler/core/platform.cpp
@@ -344,17 +344,17 @@ private:
     // explicitly specify ProfilerFeature::Threads.
     if (aFilterCount > 0) {
       aFeatures |= ProfilerFeature::Threads;
     }
 
     return aFeatures;
   }
 
-  ActivePS(PSLockRef aLock, int aEntries, double aInterval,
+  ActivePS(PSLockRef aLock, uint32_t aEntries, double aInterval,
            uint32_t aFeatures, const char** aFilters, uint32_t aFilterCount)
     : mGeneration(sNextGeneration++)
     , mEntries(aEntries)
     , mInterval(aInterval)
     , mFeatures(AdjustFeatures(aFeatures, aFilterCount))
     , mBuffer(MakeUnique<ProfileBuffer>(aEntries))
       // The new sampler thread doesn't start sampling immediately because the
       // main loop within Run() is blocked until this function's caller unlocks
@@ -427,17 +427,17 @@ private:
         return true;
       }
     }
 
     return false;
   }
 
 public:
-  static void Create(PSLockRef aLock, int aEntries, double aInterval,
+  static void Create(PSLockRef aLock, uint32_t aEntries, double aInterval,
                      uint32_t aFeatures,
                      const char** aFilters, uint32_t aFilterCount)
   {
     sInstance = new ActivePS(aLock, aEntries, aInterval, aFeatures,
                              aFilters, aFilterCount);
   }
 
   static MOZ_MUST_USE SamplerThread* Destroy(PSLockRef aLock)
@@ -447,17 +447,17 @@ public:
     sInstance = nullptr;
 
     return samplerThread;
   }
 
   static bool Exists(PSLockRef) { return !!sInstance; }
 
   static bool Equals(PSLockRef,
-                     int aEntries, double aInterval, uint32_t aFeatures,
+                     uint32_t aEntries, double aInterval, uint32_t aFeatures,
                      const char** aFilters, uint32_t aFilterCount)
   {
     if (sInstance->mEntries != aEntries ||
         sInstance->mInterval != aInterval ||
         sInstance->mFeatures != aFeatures ||
         sInstance->mFilters.length() != aFilterCount) {
       return false;
     }
@@ -484,17 +484,17 @@ public:
     MOZ_RELEASE_ASSERT(sInstance);
 
     return ((aInfo->IsMainThread() || FeatureThreads(aLock)) &&
             sInstance->ThreadSelected(aInfo->Name()));
   }
 
   PS_GET(uint32_t, Generation)
 
-  PS_GET(int, Entries)
+  PS_GET(uint32_t, Entries)
 
   PS_GET(double, Interval)
 
   PS_GET(uint32_t, Features)
 
   #define PS_GET_FEATURE(n_, str_, Name_) \
     static bool Feature##Name_(PSLockRef) \
     { \
@@ -535,17 +535,17 @@ private:
   //
   // By checking ActivePS *and* the generation, we can avoid this scenario.
   // sNextGeneration is used to track the next generation number; it is static
   // because it must persist across different ActivePS instantiations.
   const uint32_t mGeneration;
   static uint32_t sNextGeneration;
 
   // The number of entries in mBuffer.
-  const int mEntries;
+  const uint32_t mEntries;
 
   // The interval between samples, measured in milliseconds.
   const double mInterval;
 
   // The profile features that are enabled.
   const uint32_t mFeatures;
 
   // Substrings of names of threads we want to profile.
@@ -2238,17 +2238,17 @@ NotifyProfilerStarted(const int aEntries
   nsCOMPtr<nsIProfilerStartParams> params =
     new nsProfilerStartParams(aEntries, aInterval, aFeatures, filtersArray);
 
   ProfilerParent::ProfilerStarted(params);
   NotifyObservers("profiler-started", params);
 }
 
 static void
-locked_profiler_start(PSLockRef aLock, const int aEntries, double aInterval,
+locked_profiler_start(PSLockRef aLock, uint32_t aEntries, double aInterval,
                       uint32_t aFeatures,
                       const char** aFilters, uint32_t aFilterCount);
 
 // This basically duplicates AutoProfilerLabel's constructor.
 PseudoStack*
 MozGlueLabelEnter(const char* aLabel, const char* aDynamicString, void* aSp,
                   uint32_t aLine)
 {
@@ -2741,17 +2741,17 @@ TriggerPollJSSamplingOnMainThread()
       NS_NewRunnableFunction("TriggerPollJSSamplingOnMainThread", []() {
         PollJSSamplingForCurrentThread();
       });
     SystemGroup::Dispatch(TaskCategory::Other, task.forget());
   }
 }
 
 static void
-locked_profiler_start(PSLockRef aLock, int aEntries, double aInterval,
+locked_profiler_start(PSLockRef aLock, uint32_t aEntries, double aInterval,
                       uint32_t aFeatures,
                       const char** aFilters, uint32_t aFilterCount)
 {
   if (LOG_TEST) {
     LOG("locked_profiler_start");
     LOG("- entries  = %d", aEntries);
     LOG("- interval = %.2f", aInterval);
 
@@ -2771,17 +2771,17 @@ locked_profiler_start(PSLockRef aLock, i
 
   MOZ_RELEASE_ASSERT(CorePS::Exists() && !ActivePS::Exists(aLock));
 
 #if defined(GP_PLAT_amd64_windows)
   InitializeWin64ProfilerHooks();
 #endif
 
   // Fall back to the default values if the passed-in values are unreasonable.
-  int entries = aEntries > 0 ? aEntries : PROFILER_DEFAULT_ENTRIES;
+  uint32_t entries = aEntries > 0 ? aEntries : PROFILER_DEFAULT_ENTRIES;
   double interval = aInterval > 0 ? aInterval : PROFILER_DEFAULT_INTERVAL;
 
   ActivePS::Create(aLock, entries, interval, aFeatures, aFilters, aFilterCount);
 
   // Set up profiling for each registered thread, if appropriate.
   int tid = Thread::GetCurrentId();
   const CorePS::ThreadVector& liveThreads = CorePS::LiveThreads(aLock);
   for (uint32_t i = 0; i < liveThreads.size(); i++) {
@@ -2827,17 +2827,17 @@ locked_profiler_start(PSLockRef aLock, i
   }
 #endif
 
   // At the very end, set up RacyFeatures.
   RacyFeatures::SetActive(ActivePS::Features(aLock));
 }
 
 void
-profiler_start(int aEntries, double aInterval, uint32_t aFeatures,
+profiler_start(uint32_t aEntries, double aInterval, uint32_t aFeatures,
                const char** aFilters, uint32_t aFilterCount)
 {
   LOG("profiler_start");
 
 
   SamplerThread* samplerThread = nullptr;
   {
     PSAutoLock lock(gPSMutex);
@@ -2863,17 +2863,17 @@ profiler_start(int aEntries, double aInt
     NotifyObservers("profiler-stopped");
     delete samplerThread;
   }
   NotifyProfilerStarted(aEntries, aInterval, aFeatures,
                         aFilters, aFilterCount);
 }
 
 void
-profiler_ensure_started(int aEntries, double aInterval, uint32_t aFeatures,
+profiler_ensure_started(uint32_t aEntries, double aInterval, uint32_t aFeatures,
                         const char** aFilters, uint32_t aFilterCount)
 {
   LOG("profiler_ensure_started");
 
   bool startedProfiler = false;
   SamplerThread* samplerThread = nullptr;
   {
     PSAutoLock lock(gPSMutex);
--- a/tools/profiler/public/GeckoProfiler.h
+++ b/tools/profiler/public/GeckoProfiler.h
@@ -171,29 +171,29 @@ void profiler_shutdown();
 // Start the profiler -- initializing it first if necessary -- with the
 // selected options. Stops and restarts the profiler if it is already active.
 // After starting the profiler is "active". The samples will be recorded in a
 // circular buffer.
 //   "aEntries" is the number of entries in the profiler's circular buffer.
 //   "aInterval" the sampling interval, measured in millseconds.
 //   "aFeatures" is the feature set. Features unsupported by this
 //               platform/configuration are ignored.
-void profiler_start(int aEntries, double aInterval, uint32_t aFeatures,
+void profiler_start(uint32_t aEntries, double aInterval, uint32_t aFeatures,
                     const char** aFilters, uint32_t aFilterCount);
 
 // Stop the profiler and discard the profile without saving it. A no-op if the
 // profiler is inactive. After stopping the profiler is "inactive".
 void profiler_stop();
 
 // If the profiler is inactive, start it. If it's already active, restart it if
 // the requested settings differ from the current settings. Both the check and
 // the state change are performed while the profiler state is locked.
 // The only difference to profiler_start is that the current buffer contents are
 // not discarded if the profiler is already running with the requested settings.
-void profiler_ensure_started(int aEntries, double aInterval,
+void profiler_ensure_started(uint32_t aEntries, double aInterval,
                              uint32_t aFeatures, const char** aFilters,
                              uint32_t aFilterCount);
 
 //---------------------------------------------------------------------------
 // Control the profiler
 //---------------------------------------------------------------------------
 
 // Register/unregister threads with the profiler. Both functions operate the
--- a/tools/profiler/tests/gtest/ThreadProfileTest.cpp
+++ b/tools/profiler/tests/gtest/ThreadProfileTest.cpp
@@ -38,17 +38,17 @@ TEST(ThreadProfile, InsertEntriesNoWrap)
   NS_GetMainThread(getter_AddRefs(mainThread));
   ThreadInfo info("testThread", tid, true, mainThread, nullptr);
   auto pb = MakeUnique<ProfileBuffer>(100);
   int test_size = 50;
   for (int i = 0; i < test_size; i++) {
     pb->AddEntry(ProfileBufferEntry::Time(i));
   }
   ASSERT_TRUE(pb->mEntries != nullptr);
-  int readPos = pb->mReadPos;
+  uint32_t readPos = pb->mReadPos;
   while (readPos != pb->mWritePos) {
     ASSERT_TRUE(pb->mEntries[readPos].IsTime());
     ASSERT_TRUE(pb->mEntries[readPos].u.mDouble == readPos);
     readPos = (readPos + 1) % pb->mEntrySize;
   }
 }
 
 // See if wrapping works as it should in the basic case
@@ -61,17 +61,17 @@ TEST(ThreadProfile, InsertEntriesWrap) {
   NS_GetMainThread(getter_AddRefs(mainThread));
   ThreadInfo info("testThread", tid, true, mainThread, nullptr);
   auto pb = MakeUnique<ProfileBuffer>(buffer_size);
   int test_size = 43;
   for (int i = 0; i < test_size; i++) {
     pb->AddEntry(ProfileBufferEntry::Time(i));
   }
   ASSERT_TRUE(pb->mEntries != nullptr);
-  int readPos = pb->mReadPos;
+  uint32_t readPos = pb->mReadPos;
   int ctr = 0;
   while (readPos != pb->mWritePos) {
     ASSERT_TRUE(pb->mEntries[readPos].IsTime());
     // the first few entries were discarded when we wrapped
     ASSERT_TRUE(pb->mEntries[readPos].u.mDouble == ctr + (test_size - entries));
     ctr++;
     readPos = (readPos + 1) % pb->mEntrySize;
   }