Bug 1582741 - Add values to the native allocation payload; r=gerald

This commit adds the memory address of the allocation and the thread id
of the allocation to the payload. These both are required for properly
processing the balanced allocations on the front-end. All of the native
allocation payloads are now stored on the main thread, and so are
disassociated from the thread where they were generated.

Differential Revision: https://phabricator.services.mozilla.com/D51938

--HG--
extra : moz-landing-system : lando

tools/profiler/core/ProfilerMarkerPayload.cpp

@@ -940,7 +940,7 @@ void JsAllocationMarkerPayload::StreamPayload(
 BlocksRingBuffer::Length
 NativeAllocationMarkerPayload::TagAndSerializationBytes() const {
   return CommonPropsTagAndSerializationBytes() +
-         BlocksRingBuffer::SumBytes(mSize);
+         BlocksRingBuffer::SumBytes(mSize, mThreadId, mMemoryAddress);
 }
 
 void NativeAllocationMarkerPayload::SerializeTagAndPayload(
@@ -948,6 +948,8 @@ void NativeAllocationMarkerPayload::SerializeTagAndPayload(
   static const DeserializerTag tag = TagForDeserializer(Deserialize);
   SerializeTagAndCommonProps(tag, aEntryWriter);
   aEntryWriter.WriteObject(mSize);
+  aEntryWriter.WriteObject(mMemoryAddress);
+  aEntryWriter.WriteObject(mThreadId);
 }
 
 // static
@@ -956,8 +958,10 @@ UniquePtr<ProfilerMarkerPayload> NativeAllocationMarkerPayload::Deserialize(
   ProfilerMarkerPayload::CommonProps props =
       DeserializeCommonProps(aEntryReader);
   auto size = aEntryReader.ReadObject<int64_t>();
-  return UniquePtr<ProfilerMarkerPayload>(
-      new NativeAllocationMarkerPayload(std::move(props), size));
+  auto memoryAddress = aEntryReader.ReadObject<uintptr_t>();
+  auto threadId = aEntryReader.ReadObject<int>();
+  return UniquePtr<ProfilerMarkerPayload>(new NativeAllocationMarkerPayload(
+      std::move(props), size, memoryAddress, threadId));
 }
 
 void NativeAllocationMarkerPayload::StreamPayload(
@@ -966,6 +970,8 @@ void NativeAllocationMarkerPayload::StreamPayload(
   StreamCommonProps("Native allocation", aWriter, aProcessStartTime,
                     aUniqueStacks);
   aWriter.IntProperty("size", mSize);
+  aWriter.IntProperty("memoryAddress", static_cast<int64_t>(mMemoryAddress));
+  aWriter.IntProperty("threadId", mThreadId);
 }
 
 BlocksRingBuffer::Length IPCMarkerPayload::TagAndSerializationBytes() const {

tools/profiler/core/memory_hooks.cpp

@@ -398,8 +398,9 @@ static void AllocCallback(void* aPtr, size_t aReqSize) {
       // First perform the Bernoulli trial.
       gBernoulli->trial(actualSize) &&
       // Second, attempt to add a marker if the Bernoulli trial passed.
-      profiler_add_native_allocation_marker(ThreadIntercept::MainThreadId(),
-                                            static_cast<int64_t>(actualSize))) {
+      profiler_add_native_allocation_marker(
+          ThreadIntercept::MainThreadId(), static_cast<int64_t>(actualSize),
+          reinterpret_cast<uintptr_t>(aPtr))) {
     MOZ_ASSERT(gAllocationTracker,
                "gAllocationTracker must be properly installed for the memory "
                "hooks.");
@@ -418,7 +419,7 @@ static void FreeCallback(void* aPtr) {
 
   // The first part of this function does not allocate.
   size_t unsignedSize = MallocSizeOf(aPtr);
-  int64_t signedSize = -((int64_t)unsignedSize);
+  int64_t signedSize = -(static_cast<int64_t>(unsignedSize));
   sCounter->Add(signedSize);
 
   auto threadIntercept = ThreadIntercept::MaybeGet();
@@ -442,7 +443,8 @@ static void FreeCallback(void* aPtr) {
   if (gAllocationTracker->RemoveMemoryAddressIfFound(aPtr)) {
     // This size here is negative, indicating a deallocation.
     profiler_add_native_allocation_marker(ThreadIntercept::MainThreadId(),
-                                          signedSize);
+                                          signedSize,
+                                          reinterpret_cast<uintptr_t>(aPtr));
   }
 }
 

tools/profiler/core/platform.cpp

@@ -4654,15 +4654,17 @@ bool profiler_is_locked_on_current_thread() {
   return gPSMutex.IsLockedOnCurrentThread();
 }
 
-bool profiler_add_native_allocation_marker(int aMainThreadId, int64_t aSize) {
+bool profiler_add_native_allocation_marker(int aMainThreadId, int64_t aSize,
+                                           uintptr_t aMemoryAddress) {
   if (!profiler_can_accept_markers()) {
     return false;
   }
   AUTO_PROFILER_STATS(add_marker_with_NativeAllocationMarkerPayload);
   profiler_add_marker_for_thread(
       aMainThreadId, JS::ProfilingCategoryPair::OTHER, "Native allocation",
-      MakeUnique<NativeAllocationMarkerPayload>(TimeStamp::Now(), aSize,
-                                                profiler_get_backtrace()));
+      MakeUnique<NativeAllocationMarkerPayload>(
+          TimeStamp::Now(), aSize, aMemoryAddress, profiler_current_thread_id(),
+          profiler_get_backtrace()));
   return true;
 }
 

tools/profiler/public/GeckoProfiler.h

@@ -778,7 +778,8 @@ void profiler_add_js_allocation_marker(JS::RecordAllocationInfo&& info);
 
 // Returns true or or false depending on whether the marker was actually added
 // or not.
-bool profiler_add_native_allocation_marker(int aMainThreadId, int64_t aSize);
+bool profiler_add_native_allocation_marker(int aMainThreadId, int64_t aSize,
+                                           uintptr_t aMemorySize);
 
 // Returns true if the profiler lock is currently held *on the current thread*.
 // This may be used by re-entrant code that may call profiler functions while

tools/profiler/public/ProfilerMarkerPayload.h

@@ -688,20 +688,31 @@ class JsAllocationMarkerPayload : public ProfilerMarkerPayload {
 class NativeAllocationMarkerPayload : public ProfilerMarkerPayload {
  public:
   NativeAllocationMarkerPayload(const mozilla::TimeStamp& aStartTime,
-                                const int64_t aSize,
-                                UniqueProfilerBacktrace aStack)
+                                int64_t aSize, uintptr_t aMemoryAddress,
+                                int aThreadId, UniqueProfilerBacktrace aStack)
       : ProfilerMarkerPayload(aStartTime, aStartTime, mozilla::Nothing(),
                               std::move(aStack)),
-        mSize(aSize) {}
+        mSize(aSize),
+        mMemoryAddress(aMemoryAddress),
+        mThreadId(aThreadId) {}
 
   DECL_STREAM_PAYLOAD
 
  private:
-  NativeAllocationMarkerPayload(CommonProps&& aCommonProps, int64_t aSize)
-      : ProfilerMarkerPayload(std::move(aCommonProps)), mSize(aSize) {}
+  NativeAllocationMarkerPayload(CommonProps&& aCommonProps, int64_t aSize,
+                                uintptr_t aMemoryAddress, int aThreadId)
+      : ProfilerMarkerPayload(std::move(aCommonProps)),
+        mSize(aSize),
+        mMemoryAddress(aMemoryAddress),
+        mThreadId(aThreadId) {}
 
-  // The size in bytes of the allocation or de-allocation.
+  // The size in bytes of the allocation. If the number is negative then it
+  // represents a de-allocation.
   int64_t mSize;
+  // The memory address of the allocation or de-allocation.
+  uintptr_t mMemoryAddress;
+
+  int mThreadId;
 };
 
 class IPCMarkerPayload : public ProfilerMarkerPayload {

tools/profiler/tests/gtest/GeckoProfiler.cpp

@@ -708,7 +708,7 @@ TEST(GeckoProfiler, Markers)
 
   PROFILER_ADD_MARKER_WITH_PAYLOAD("NativeAllocationMarkerPayload marker",
                                    OTHER, NativeAllocationMarkerPayload,
-                                   (ts1, 9876543210, nullptr));
+                                   (ts1, 9876543210, 1234, 5678, nullptr));
 
   PROFILER_ADD_MARKER_WITH_PAYLOAD(
       "PrefMarkerPayload marker", OTHER, PrefMarkerPayload,
@@ -1137,6 +1137,8 @@ TEST(GeckoProfiler, Markers)
                 EXPECT_EQ_JSON(payload["endTime"], Double, ts1Double);
                 EXPECT_TRUE(payload["stack"].isNull());
                 EXPECT_EQ_JSON(payload["size"], Int64, 9876543210);
+                EXPECT_EQ_JSON(payload["memoryAddress"], Int64, 1234);
+                EXPECT_EQ_JSON(payload["threadId"], Int64, 5678);
 
               } else if (nameString == "PrefMarkerPayload marker") {
                 EXPECT_EQ(state, S_PrefMarkerPayload);