Bug 1582741 - Add values to the native allocation payload; r=gerald
authorGreg Tatum <gtatum@mozilla.com>
Wed, 13 Nov 2019 16:19:16 +0000
changeset 501803 e2b8a34e2aac27d21773cb2d3da1278f47550e4f
parent 501802 439f620efb11ff3e421a3ffdab1deb6b76b79281
child 501804 9a4d7144206139470df276d6d0a26a99068aa0ab
push id114172
push userdluca@mozilla.com
push dateTue, 19 Nov 2019 11:31:10 +0000
treeherdermozilla-inbound@b5c5ba07d3db [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersgerald
bugs1582741
milestone72.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 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
tools/profiler/core/ProfilerMarkerPayload.cpp
tools/profiler/core/memory_hooks.cpp
tools/profiler/core/platform.cpp
tools/profiler/public/GeckoProfiler.h
tools/profiler/public/ProfilerMarkerPayload.h
tools/profiler/tests/gtest/GeckoProfiler.cpp
--- a/tools/profiler/core/ProfilerMarkerPayload.cpp
+++ b/tools/profiler/core/ProfilerMarkerPayload.cpp
@@ -935,42 +935,48 @@ void JsAllocationMarkerPayload::StreamPa
   aWriter.StringProperty("coarseType", mCoarseType);
   aWriter.IntProperty("size", mSize);
   aWriter.BoolProperty("inNursery", mInNursery);
 }
 
 BlocksRingBuffer::Length
 NativeAllocationMarkerPayload::TagAndSerializationBytes() const {
   return CommonPropsTagAndSerializationBytes() +
-         BlocksRingBuffer::SumBytes(mSize);
+         BlocksRingBuffer::SumBytes(mSize, mThreadId, mMemoryAddress);
 }
 
 void NativeAllocationMarkerPayload::SerializeTagAndPayload(
     BlocksRingBuffer::EntryWriter& aEntryWriter) const {
   static const DeserializerTag tag = TagForDeserializer(Deserialize);
   SerializeTagAndCommonProps(tag, aEntryWriter);
   aEntryWriter.WriteObject(mSize);
+  aEntryWriter.WriteObject(mMemoryAddress);
+  aEntryWriter.WriteObject(mThreadId);
 }
 
 // static
 UniquePtr<ProfilerMarkerPayload> NativeAllocationMarkerPayload::Deserialize(
     BlocksRingBuffer::EntryReader& aEntryReader) {
   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(
     SpliceableJSONWriter& aWriter, const TimeStamp& aProcessStartTime,
     UniqueStacks& aUniqueStacks) const {
   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 {
   return CommonPropsTagAndSerializationBytes() +
          BlocksRingBuffer::SumBytes(mOtherPid, mMessageSeqno, mMessageType,
                                     mSide, mDirection, mSync);
 }
 
--- a/tools/profiler/core/memory_hooks.cpp
+++ b/tools/profiler/core/memory_hooks.cpp
@@ -393,18 +393,19 @@ static void AllocCallback(void* aPtr, si
   // configured probability. It takes into account the byte size so that
   // larger allocations are weighted heavier than smaller allocations.
   MOZ_ASSERT(gBernoulli,
              "gBernoulli must be properly installed for the memory hooks.");
   if (
       // 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.");
     // Only track the memory if the allocation marker was actually added to the
     // profiler.
     gAllocationTracker->AddMemoryAddress(aPtr);
   }
 
@@ -413,17 +414,17 @@ static void AllocCallback(void* aPtr, si
 
 static void FreeCallback(void* aPtr) {
   if (!aPtr) {
     return;
   }
 
   // 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();
   if (threadIntercept.isNothing()) {
     // Either the native allocations feature is not turned on, or we  may be
     // recursing into a memory hook, return. We'll still collect counter
     // information about this allocation, but no stack.
     return;
@@ -437,17 +438,18 @@ static void FreeCallback(void* aPtr) {
   // configured probability. It takes into account the byte size so that
   // larger allocations are weighted heavier than smaller allocations.
   MOZ_ASSERT(
       gAllocationTracker,
       "gAllocationTracker must be properly installed for the memory hooks.");
   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));
   }
 }
 
 }  // namespace profiler
 }  // namespace mozilla
 
 //---------------------------------------------------------------------------
 // malloc/free interception
--- a/tools/profiler/core/platform.cpp
+++ b/tools/profiler/core/platform.cpp
@@ -4649,25 +4649,27 @@ void profiler_add_js_allocation_marker(J
       JsAllocationMarkerPayload(TimeStamp::Now(), std::move(info),
                                 profiler_get_backtrace()));
 }
 
 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;
 }
 
 void profiler_add_network_marker(
     nsIURI* aURI, int32_t aPriority, uint64_t aChannelId, NetworkLoadType aType,
     mozilla::TimeStamp aStart, mozilla::TimeStamp aEnd, int64_t aCount,
     mozilla::net::CacheDisposition aCacheDisposition,
     const mozilla::net::TimingStruct* aTimings, nsIURI* aRedirectURI,
--- a/tools/profiler/public/GeckoProfiler.h
+++ b/tools/profiler/public/GeckoProfiler.h
@@ -773,17 +773,18 @@ void profiler_add_marker(const char* aMa
                          JS::ProfilingCategoryPair aCategoryPair,
                          const ProfilerMarkerPayload& aPayload);
 
 void profiler_add_js_marker(const char* aMarkerName);
 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
 // the profiler already has the lock (which would deadlock).
 bool profiler_is_locked_on_current_thread();
 
 // Insert a marker in the profile timeline for a specified thread.
 void profiler_add_marker_for_thread(
--- a/tools/profiler/public/ProfilerMarkerPayload.h
+++ b/tools/profiler/public/ProfilerMarkerPayload.h
@@ -683,30 +683,41 @@ class JsAllocationMarkerPayload : public
 };
 
 // This payload is for collecting information about native allocations. There is
 // a memory hook into malloc and other memory functions that can sample a subset
 // of the allocations. This information is then stored in this payload.
 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 {
  public:
   IPCMarkerPayload(int32_t aOtherPid, int32_t aMessageSeqno,
                    IPC::Message::msgid_t aMessageType, mozilla::ipc::Side aSide,
                    mozilla::ipc::MessageDirection aDirection, bool aSync,
                    const mozilla::TimeStamp& aStartTime)
--- a/tools/profiler/tests/gtest/GeckoProfiler.cpp
+++ b/tools/profiler/tests/gtest/GeckoProfiler.cpp
@@ -703,17 +703,17 @@ TEST(GeckoProfiler, Markers)
   PROFILER_ADD_MARKER_WITH_PAYLOAD("LogMarkerPayload marker", OTHER,
                                    LogMarkerPayload, ("module", "text", ts1));
 
   PROFILER_ADD_MARKER_WITH_PAYLOAD("LongTaskMarkerPayload marker", OTHER,
                                    LongTaskMarkerPayload, (ts1, ts2));
 
   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,
       ("preference name", mozilla::Nothing(), mozilla::Nothing(),
        NS_LITERAL_CSTRING("preference value"), ts1));
 
   nsCString screenshotURL = NS_LITERAL_CSTRING("url");
   PROFILER_ADD_MARKER_WITH_PAYLOAD(
@@ -1132,16 +1132,18 @@ TEST(GeckoProfiler, Markers)
                 state = State(S_NativeAllocationMarkerPayload + 1);
                 EXPECT_EQ(typeString, "Native allocation");
                 EXPECT_EQ_JSON(payload["startTime"], Double, ts1Double);
                 // Start timestamp is also stored in marker outside of payload.
                 EXPECT_EQ_JSON(marker[1], Double, ts1Double);
                 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);
                 state = State(S_PrefMarkerPayload + 1);
                 EXPECT_EQ(typeString, "PreferenceRead");
                 EXPECT_EQ_JSON(payload["startTime"], Double, ts1Double);
                 // Start timestamp is also stored in marker outside of payload.
                 EXPECT_EQ_JSON(marker[1], Double, ts1Double);