From ab7dd1effd2f2e240269dbdda5fdf9c5b1902bee Mon Sep 17 00:00:00 2001
From: Gerald Squelart <gsquelart@mozilla.com>
Date: Thu, 13 Aug 2020 03:29:06 +0000
Subject: [PATCH] Bug 1658232 - ProfileBuffer::UnderlyingChunkedBuffer() -
 r=gregtatum

Let `ProfilerBuffer` expose its underlying `ProfileChunkedBuffer`, this will be useful when `ProfilerBacktrace` will only be given a `ProfileBuffer`, and to perform some safety checks.

As a bonus from this change, `StoreMarker()` can be made non-generic -- It was relying on both `ProfileBuffer` and `ProfileChunkedBuffer` to have the same function `PutObjects()`. Consequently, we don't need `ProfileBuffer::PutObjects()` anymore, this removes this clunky pass-through (but useful and the best solution at the time).

Differential Revision: https://phabricator.services.mozilla.com/D86510
---
 mozglue/baseprofiler/core/ProfileBuffer.h |  2 ++
 tools/profiler/core/ProfileBuffer.h       | 12 ++++--------
 tools/profiler/core/platform.cpp          | 21 ++++++++++++---------
 3 files changed, 18 insertions(+), 17 deletions(-)

diff --git a/mozglue/baseprofiler/core/ProfileBuffer.h b/mozglue/baseprofiler/core/ProfileBuffer.h
index 82b3a543e48c..12b43b4e1c8f 100644
--- a/mozglue/baseprofiler/core/ProfileBuffer.h
+++ b/mozglue/baseprofiler/core/ProfileBuffer.h
@@ -27,6 +27,8 @@ class ProfileBuffer final {
   // manager.
   explicit ProfileBuffer(ProfileChunkedBuffer& aBuffer);
 
+  ProfileChunkedBuffer& UnderlyingChunkedBuffer() const { return mEntries; }
+
   bool IsThreadSafe() const { return mEntries.IsThreadSafe(); }
 
   // Add |aEntry| to the buffer, ignoring what kind of entry it is.
diff --git a/tools/profiler/core/ProfileBuffer.h b/tools/profiler/core/ProfileBuffer.h
index 12ed1a4e5602..70fbb1a438bb 100644
--- a/tools/profiler/core/ProfileBuffer.h
+++ b/tools/profiler/core/ProfileBuffer.h
@@ -25,6 +25,10 @@ class ProfileBuffer final {
   // manager.
   explicit ProfileBuffer(mozilla::ProfileChunkedBuffer& aBuffer);
 
+  mozilla::ProfileChunkedBuffer& UnderlyingChunkedBuffer() const {
+    return mEntries;
+  }
+
   bool IsThreadSafe() const { return mEntries.IsThreadSafe(); }
 
   // Add |aEntry| to the buffer, ignoring what kind of entry it is.
@@ -34,14 +38,6 @@ class ProfileBuffer final {
   // Returns the position of the entry.
   uint64_t AddThreadIdEntry(int aThreadId);
 
-  // Add a new single entry with *all* given object (using a Serializer for
-  // each), return block index.
-  template <typename... Ts>
-  mozilla::ProfileBufferBlockIndex PutObjects(
-      const ProfileBufferEntry::Kind aKind, const Ts&... aTs) {
-    return mEntries.PutObjects(aKind, aTs...);
-  }
-
   void CollectCodeLocation(
       const char* aLabel, const char* aStr, uint32_t aFrameFlags,
       uint64_t aInnerWindowID, const mozilla::Maybe<uint32_t>& aLineNumber,
diff --git a/tools/profiler/core/platform.cpp b/tools/profiler/core/platform.cpp
index 1d0a280cee12..f309d7dddae5 100644
--- a/tools/profiler/core/platform.cpp
+++ b/tools/profiler/core/platform.cpp
@@ -1660,16 +1660,17 @@ MarkerTiming get_marker_timing_from_payload(
 
 // Add the marker to the given buffer with the given information.
 // This is a unified insertion point for all the markers.
-template <typename Buffer>
-static void StoreMarker(Buffer& aBuffer, int aThreadId, const char* aMarkerName,
+static void StoreMarker(ProfileChunkedBuffer& aChunkedBuffer, int aThreadId,
+                        const char* aMarkerName,
                         const MarkerTiming& aMarkerTiming,
                         JS::ProfilingCategoryPair aCategoryPair,
                         const ProfilerMarkerPayload* aPayload) {
-  aBuffer.PutObjects(ProfileBufferEntry::Kind::MarkerData, aThreadId,
-                     WrapProfileBufferUnownedCString(aMarkerName),
-                     aMarkerTiming.GetStartTime(), aMarkerTiming.GetEndTime(),
-                     aMarkerTiming.GetMarkerPhase(),
-                     static_cast<uint32_t>(aCategoryPair), aPayload);
+  aChunkedBuffer.PutObjects(ProfileBufferEntry::Kind::MarkerData, aThreadId,
+                            WrapProfileBufferUnownedCString(aMarkerName),
+                            aMarkerTiming.GetStartTime(),
+                            aMarkerTiming.GetEndTime(),
+                            aMarkerTiming.GetMarkerPhase(),
+                            static_cast<uint32_t>(aCategoryPair), aPayload);
 }
 
 ////////////////////////////////////////////////////////////////////////
@@ -2764,7 +2765,8 @@ static void CollectJavaThreadProfileData(ProfileBuffer& aProfileBuffer) {
 
     if (!text) {
       // This marker doesn't have a text.
-      StoreMarker(aProfileBuffer, threadId, markerName.get(), timing,
+      StoreMarker(aProfileBuffer.UnderlyingChunkedBuffer(), threadId,
+                  markerName.get(), timing,
                   JS::ProfilingCategoryPair::JAVA_ANDROID, nullptr);
     } else {
       // This marker has a text.
@@ -2773,7 +2775,8 @@ static void CollectJavaThreadProfileData(ProfileBuffer& aProfileBuffer) {
                                       nullptr);
 
       // Put the marker inside the buffer.
-      StoreMarker(aProfileBuffer, threadId, markerName.get(), timing,
+      StoreMarker(aProfileBuffer.UnderlyingChunkedBuffer(), threadId,
+                  markerName.get(), timing,
                   JS::ProfilingCategoryPair::JAVA_ANDROID, &payload);
     }
   }