Bug 1817163 - Add a profiler feature for memory tracking to enable the memory hooks r=profiler-reviewers,perftest-reviewers,android-reviewers,kshampur,aabh,gl

This feature is added as "recommended", and added to every presets. This will
allow us to disable it if we really want to.

If the "native allocations" feature is enabled, we still force the installation
of the memory hooks even if we don't have this feature to not break the native
allocations feature.

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

devtools/client/performance-new/shared/background.sys.mjs

@@ -124,7 +124,7 @@ export const presets = {
   "web-developer": {
     entries: 128 * 1024 * 1024,
     interval: 1,
-    features: ["screenshots", "js", "cpu"],
+    features: ["screenshots", "js", "cpu", "memory"],
     threads: ["GeckoMain", "Compositor", "Renderer", "DOM Worker"],
     duration: 0,
     profilerViewMode: "active-tab",
@@ -142,7 +142,15 @@ export const presets = {
   "firefox-platform": {
     entries: 128 * 1024 * 1024,
     interval: 1,
-    features: ["screenshots", "js", "stackwalk", "cpu", "java", "processcpu"],
+    features: [
+      "screenshots",
+      "js",
+      "stackwalk",
+      "cpu",
+      "java",
+      "processcpu",
+      "memory",
+    ],
     threads: [
       "GeckoMain",
       "Compositor",
@@ -165,7 +173,7 @@ export const presets = {
   graphics: {
     entries: 128 * 1024 * 1024,
     interval: 1,
-    features: ["stackwalk", "js", "cpu", "java", "processcpu"],
+    features: ["stackwalk", "js", "cpu", "java", "processcpu", "memory"],
     threads: [
       "GeckoMain",
       "Compositor",
@@ -199,6 +207,7 @@ export const presets = {
       "audiocallbacktracing",
       "ipcmessages",
       "processcpu",
+      "memory",
     ],
     threads: [
       "cubeb",
@@ -248,6 +257,7 @@ export const presets = {
       "java",
       "processcpu",
       "bandwidth",
+      "memory",
     ],
     threads: [
       "Compositor",
@@ -286,6 +296,7 @@ export const presets = {
       "markersallthreads",
       "power",
       "bandwidth",
+      "memory",
     ],
     threads: ["GeckoMain", "Renderer"],
     duration: 0,

devtools/client/performance-new/shared/utils.js

@@ -414,6 +414,13 @@ const featureDescriptions = [
       "Record how much CPU has been used between samples by each profiled thread.",
     recommended: true,
   },
+  {
+    name: "Memory Tracking",
+    value: "memory",
+    title:
+      "Track the memory allocations and deallocations per process over time.",
+    recommended: true,
+  },
   {
     name: "Java",
     value: "java",

devtools/client/performance-new/test/browser/browser_interaction-between-interfaces.js

@@ -294,19 +294,21 @@ add_task(async function test_change_in_about_profiling() {
         "The new value should have the same count of threads as the old value, please double check the test code."
       );
       setThreadInputValue(newThreadValue);
-      checkDevtoolsCustomPresetContent(
-        devtoolsDocument,
-        `
-          Interval: 2 ms
-          Threads: GeckoMain, Dummy
-          JavaScript
-          Native Stacks
-          CPU Utilization
-          Audio Callback Tracing
-          IPC Messages
-          Process CPU Utilization
-        `
-      );
+
+      let presetContents = `
+        Interval: 2 ms
+        Threads: GeckoMain, Dummy
+        JavaScript
+        Native Stacks
+        CPU Utilization
+        Audio Callback Tracing
+        IPC Messages
+        Process CPU Utilization
+      `;
+      if (Services.profiler.GetFeatures().includes("memory")) {
+        presetContents += "Memory Tracking";
+      }
+      checkDevtoolsCustomPresetContent(devtoolsDocument, presetContents);
     }
   );
 });

devtools/server/actors/perf.js

@@ -58,6 +58,7 @@ exports.PerfActor = class PerfActor extends Actor {
         "stackwalk",
         "cpu",
         "responsiveness",
+        "memory",
       ],
       threads: options.threads || ["GeckoMain", "Compositor"],
       activeTabID: RecordingUtils.getActiveBrowserID(),

mobile/android/fenix/app/src/main/java/org/mozilla/fenix/perf/ProfilerUtils.kt

@@ -34,6 +34,7 @@ private val firefox_features = arrayOf(
     "java",
     "processcpu",
     "ipcmessages",
+    "memory",
 )
 private val firefox_threads = arrayOf(
     "GeckoMain",
@@ -43,7 +44,8 @@ private val firefox_threads = arrayOf(
     "DOM Worker",
 )
 
-private val graphics_features = arrayOf("stackwalk", "js", "cpu", "java", "processcpu", "ipcmessages")
+private val graphics_features =
+    arrayOf("stackwalk", "js", "cpu", "java", "processcpu", "ipcmessages", "memory")
 private val graphics_threads = arrayOf(
     "GeckoMain",
     "Compositor",
@@ -64,6 +66,7 @@ private val media_features = arrayOf(
     "ipcmessages",
     "processcpu",
     "java",
+    "memory",
 )
 private val media_threads = arrayOf(
     "cubeb", "audio", "BackgroundThreadPool", "camera", "capture", "Compositor", "decoder", "GeckoMain", "gmp",
@@ -81,6 +84,7 @@ private val networking_features = arrayOf(
     "processcpu",
     "bandwidth",
     "ipcmessages",
+    "memory",
 )
 
 private val networking_threads = arrayOf(

mobile/android/fenix/tools/setup-startup-profiling.py

@@ -23,7 +23,7 @@ PRODUCTS = [PROD_FENIX, PROD_GVE]
 GV_CONFIG = b"""env:
   MOZ_PROFILER_STARTUP: 1
   MOZ_PROFILER_STARTUP_INTERVAL: 5
-  MOZ_PROFILER_STARTUP_FEATURES: js,stackwalk,leaf,screenshots,ipcmessages,java,cpu
+  MOZ_PROFILER_STARTUP_FEATURES: js,stackwalk,leaf,screenshots,ipcmessages,java,cpu,memory
   MOZ_PROFILER_STARTUP_FILTERS: GeckoMain,Compositor,Renderer,IPDL Background
 """
 

modules/libpref/init/all.js

@@ -720,8 +720,8 @@ pref("devtools.performance.recording.duration.remote", 0);
 // explanations. Remote profiling also includes the java feature by default.
 // If the remote debuggee isn't an Android phone, then this feature will
 // be ignored.
-pref("devtools.performance.recording.features", "[\"js\",\"stackwalk\",\"cpu\",\"screenshots\"]");
-pref("devtools.performance.recording.features.remote", "[\"js\",\"stackwalk\",\"cpu\",\"screenshots\",\"java\"]");
+pref("devtools.performance.recording.features", "[\"js\",\"stackwalk\",\"cpu\",\"screenshots\",\"memory\"]");
+pref("devtools.performance.recording.features.remote", "[\"js\",\"stackwalk\",\"cpu\",\"screenshots\",\"memory\",\"java\"]");
 // Threads to be captured by the profiler.
 pref("devtools.performance.recording.threads", "[\"GeckoMain\",\"Compositor\",\"Renderer\"]");
 pref("devtools.performance.recording.threads.remote", "[\"GeckoMain\",\"Compositor\",\"Renderer\"]");

mozglue/baseprofiler/public/BaseProfilerState.h

@@ -238,7 +238,10 @@ class MOZ_RAII AutoProfilerStats {
           "every CPU core for every profiler sample.")                       \
                                                                              \
     MACRO(23, "bandwidth", Bandwidth,                                        \
-          "Record the network bandwidth used for every profiler sample.")
+          "Record the network bandwidth used for every profiler sample.")    \
+    MACRO(24, "memory", Memory,                                              \
+          "Track the memory allocations and deallocations per process over " \
+          "time.")
 // *** Synchronize with lists in ProfilerState.h and geckoProfiler.json ***
 
 struct ProfilerFeature {

testing/raptor/raptor/browsertime/base.py

@@ -630,7 +630,7 @@ class Browsertime(Perftest):
             (
                 "gecko_profile_features",
                 "--firefox.geckoProfilerParams.features",
-                "js,stackwalk,cpu,screenshots",
+                "js,stackwalk,cpu,screenshots,memory",
             ),
             (
                 "gecko_profile_threads",

testing/raptor/raptor/tests/benchmarks/motionmark-1-3-desktop.toml

@@ -2,7 +2,7 @@
 alert_threshold = 2.0
 apps = "firefox, chrome, safari, custom-car"
 gecko_profile_interval = 1
-gecko_profile_features = "stackwalk,js,cpu,java,processcpu"
+gecko_profile_features = "stackwalk,js,cpu,java,processcpu,memory"
 gecko_profile_threads = "GeckoMain,Compositor,Renderer,SwComposite,RenderBackend,SceneBuilder,WrWorker,CanvasWorkers,TextureUpdate"
 expose_browser_profiler = true
 expose_chrome_trace = true

testing/raptor/raptor/tests/benchmarks/motionmark-1-3-mobile.toml

@@ -2,7 +2,7 @@
 alert_threshold = 2.0
 apps = "fenix, geckoview, chrome-m, cstm-car-m"
 gecko_profile_interval = 1
-gecko_profile_features = "stackwalk,js,cpu,java,processcpu"
+gecko_profile_features = "stackwalk,js,cpu,java,processcpu,memory"
 gecko_profile_threads = "GeckoMain,Compositor,Renderer,SwComposite,RenderBackend,SceneBuilder,WrWorker,CanvasWorkers,TextureUpdate"
 expose_browser_profiler = true
 lower_is_better = false

testing/raptor/raptor/tests/benchmarks/motionmark-animometer-desktop.toml

@@ -2,7 +2,7 @@
 alert_threshold = 2.0
 apps = "firefox, chrome, safari"
 gecko_profile_interval = 1
-gecko_profile_features = "stackwalk,js,cpu,java,processcpu"
+gecko_profile_features = "stackwalk,js,cpu,java,processcpu,memory"
 gecko_profile_threads = "GeckoMain,Compositor,Renderer,SwComposite,RenderBackend,SceneBuilder,WrWorker,CanvasWorkers,TextureUpdate"
 expose_browser_profiler = true
 lower_is_better = false

testing/raptor/raptor/tests/benchmarks/motionmark-animometer-mobile.toml

@@ -2,7 +2,7 @@
 alert_threshold = 2.0
 apps = "fenix, chrome-m, geckoview"
 gecko_profile_interval = 1
-gecko_profile_features = "stackwalk,js,cpu,java,processcpu"
+gecko_profile_features = "stackwalk,js,cpu,java,processcpu,memory"
 gecko_profile_threads = "GeckoMain,Compositor,Renderer,SwComposite,RenderBackend,SceneBuilder,WrWorker,CanvasWorkers,TextureUpdate"
 expose_browser_profiler = true
 lower_is_better = false

testing/raptor/raptor/tests/benchmarks/motionmark-htmlsuite-desktop.toml

@@ -2,7 +2,7 @@
 alert_threshold = 2.0
 apps = "firefox, chrome, safari"
 gecko_profile_interval = 1
-gecko_profile_features = "stackwalk,js,cpu,java,processcpu"
+gecko_profile_features = "stackwalk,js,cpu,java,processcpu,memory"
 gecko_profile_threads = "GeckoMain,Compositor,Renderer,SwComposite,RenderBackend,SceneBuilder,WrWorker,CanvasWorkers,TextureUpdate"
 expose_gecko_profiler = true
 expose_chrome_trace = true

testing/raptor/raptor/tests/benchmarks/motionmark-htmlsuite-mobile.toml

@@ -2,7 +2,7 @@
 alert_threshold = 2.0
 apps = "fenix, chrome-m, geckoview"
 gecko_profile_interval = 1
-gecko_profile_features = "stackwalk,js,cpu,java,processcpu"
+gecko_profile_features = "stackwalk,js,cpu,java,processcpu,memory"
 gecko_profile_threads = "GeckoMain,Compositor,Renderer,SwComposite,RenderBackend,SceneBuilder,WrWorker,CanvasWorkers,TextureUpdate"
 expose_gecko_profiler = true
 expose_chrome_trace = true

testing/raptor/raptor/tests/custom/browsertime-indexeddb.toml

@@ -4,7 +4,7 @@ alert_threshold = 2.0
 browser_cycles = 1
 custom_data = true
 gecko_profile_entries = 131072000 # 1GB
-gecko_profile_features = "js,stackwalk,cpu"
+gecko_profile_features = "js,stackwalk,cpu,memory"
 gecko_profile_threads = "GeckoMain,DOM Worker,IndexedDB"
 lower_is_better = true
 measure = "cpuTime"

testing/talos/talos/gecko_profile.py

@@ -42,7 +42,7 @@ class GeckoProfile(object):
             "gecko_profile_entries", int(128 * 1024 * 1024 / 8)
         )
         gecko_profile_features = test_config.get(
-            "gecko_profile_features", "js,stackwalk,cpu,screenshots"
+            "gecko_profile_features", "js,stackwalk,cpu,screenshots,memory"
         )
         gecko_profile_threads = test_config.get(
             "gecko_profile_threads", "GeckoMain,Compositor,Renderer"

toolkit/components/extensions/schemas/geckoProfiler.json

@@ -47,7 +47,8 @@
           "power",
           "responsiveness",
           "cpufreq",
-          "bandwidth"
+          "bandwidth",
+          "memory"
         ]
       },
       {

tools/profiler/core/memory_hooks.cpp

@@ -584,6 +584,7 @@ BaseProfilerCount* install_memory_hooks() {
     ThreadIntercept::Init();
   } else {
     sCounter->Clear();
+    sCounter->Register();
   }
   jemalloc_replace_dynamic(replace_init);
   return sCounter;
@@ -635,4 +636,10 @@ void disable_native_allocations() {
   }
 }
 
+void unregister_memory_counter() {
+  if (sCounter) {
+    sCounter->Unregister();
+  }
+}
+
 }  // namespace mozilla::profiler

tools/profiler/core/memory_hooks.h

@@ -17,6 +17,7 @@ BaseProfilerCount* install_memory_hooks();
 void remove_memory_hooks();
 void enable_native_allocations();
 void disable_native_allocations();
+void unregister_memory_counter();
 
 }  // namespace profiler
 }  // namespace mozilla

tools/profiler/core/platform.cpp

@@ -405,6 +405,7 @@ static uint32_t AvailableFeatures() {
   }
 #else
   // The memory hooks are not available.
+  ProfilerFeature::ClearMemory(features);
   ProfilerFeature::ClearNativeAllocations(features);
 #endif
 
@@ -1310,6 +1311,11 @@ class ActivePS {
 
 #undef PS_GET_FEATURE
 
+  static bool ShouldInstallMemoryHooks(PSLockRef) {
+    MOZ_ASSERT(sInstance);
+    return ProfilerFeature::ShouldInstallMemoryHooks(sInstance->mFeatures);
+  }
+
   static uint32_t JSFlags(PSLockRef aLock) {
     uint32_t Flags = 0;
     Flags |=
@@ -5819,9 +5825,16 @@ void profiler_init(void* aStackTop) {
   profiler_mark_thread_awake();
 
 #if defined(MOZ_REPLACE_MALLOC) && defined(MOZ_PROFILER_MEMORY)
-  // Start counting memory allocations (outside of lock because this may call
-  // profiler_add_sampled_counter which would attempt to take the lock.)
-  ActivePS::SetMemoryCounter(mozilla::profiler::install_memory_hooks());
+  if (ProfilerFeature::ShouldInstallMemoryHooks(features)) {
+    // Start counting memory allocations (outside of lock because this may call
+    // profiler_add_sampled_counter which would attempt to take the lock.)
+    ActivePS::SetMemoryCounter(mozilla::profiler::install_memory_hooks());
+  } else {
+    // Unregister the memory counter in case it was registered before. This will
+    // make sure that the empty memory counter from the previous profiler run is
+    // removed completely and we don't serialize the memory counters.
+    mozilla::profiler::unregister_memory_counter();
+  }
 #endif
 
   invoke_profiler_state_change_callbacks(ProfilingState::Started);
@@ -6448,9 +6461,16 @@ RefPtr<GenericPromise> profiler_start(PowerOfTwo32 aCapacity, double aInterval,
   }
 
 #if defined(MOZ_REPLACE_MALLOC) && defined(MOZ_PROFILER_MEMORY)
-  // Start counting memory allocations (outside of lock because this may call
-  // profiler_add_sampled_counter which would attempt to take the lock.)
-  ActivePS::SetMemoryCounter(mozilla::profiler::install_memory_hooks());
+  if (ProfilerFeature::ShouldInstallMemoryHooks(aFeatures)) {
+    // Start counting memory allocations (outside of lock because this may call
+    // profiler_add_sampled_counter which would attempt to take the lock.)
+    ActivePS::SetMemoryCounter(mozilla::profiler::install_memory_hooks());
+  } else {
+    // Unregister the memory counter in case it was registered before. This will
+    // make sure that the empty memory counter from the previous profiler run is
+    // removed completely and we don't serialize the memory counters.
+    mozilla::profiler::unregister_memory_counter();
+  }
 #endif
 
   invoke_profiler_state_change_callbacks(ProfilingState::Started);
@@ -6574,7 +6594,8 @@ void profiler_ensure_started(PowerOfTwo32 aCapacity, double aInterval,
   }
 
 #if defined(MOZ_REPLACE_MALLOC) && defined(MOZ_PROFILER_MEMORY)
-  if (ActivePS::FeatureNativeAllocations(aLock)) {
+  if (ActivePS::FeatureNativeAllocations(aLock) &&
+      ActivePS::ShouldInstallMemoryHooks(aLock)) {
     mozilla::profiler::disable_native_allocations();
   }
 #endif

tools/profiler/public/ProfilerCounts.h

@@ -20,6 +20,7 @@
 
 #  include "mozilla/Assertions.h"
 #  include "mozilla/Atomics.h"
+#  include "mozilla/DataMutex.h"
 
 class BaseProfilerCount;
 void profiler_add_sampled_counter(BaseProfilerCount* aCounter);
@@ -188,13 +189,33 @@ class ProfilerCounterTotal final : public BaseProfilerCount {
  public:
   ProfilerCounterTotal(const char* aLabel, const char* aCategory,
                        const char* aDescription)
-      : BaseProfilerCount(aLabel, &mCounter, &mNumber, aCategory,
-                          aDescription) {
+      : BaseProfilerCount(aLabel, &mCounter, &mNumber, aCategory, aDescription),
+        mRegistered(false, "ProfilerCounterTotal::mRegistered") {
     // Assume we're in libxul
+    Register();
+  }
+
+  virtual ~ProfilerCounterTotal() { Unregister(); }
+
+  void Register() {
+    auto registered = mRegistered.Lock();
+    if (*registered) {
+      return;
+    }
+
+    *registered = true;
     profiler_add_sampled_counter(this);
   }
 
-  virtual ~ProfilerCounterTotal() { profiler_remove_sampled_counter(this); }
+  void Unregister() {
+    auto registered = mRegistered.Lock();
+    if (!*registered) {
+      return;
+    }
+
+    *registered = false;
+    profiler_remove_sampled_counter(this);
+  }
 
   BaseProfilerCount& operator++() {
     Add(1);
@@ -208,6 +229,9 @@ class ProfilerCounterTotal final : public BaseProfilerCount {
 
   ProfilerAtomicSigned mCounter;
   ProfilerAtomicUnsigned mNumber;
+  // Using OffTheBooksMutex here because we intentionally leak memory counters
+  // if they are initialized.
+  mozilla::DataMutexBase<bool, mozilla::OffTheBooksMutex> mRegistered;
 };
 
 // Defines a counter that is sampled on each profiler tick, with a running

tools/profiler/public/ProfilerState.h

@@ -118,7 +118,10 @@
         "every CPU core for every profiler sample.")                       \
                                                                            \
   MACRO(23, "bandwidth", Bandwidth,                                        \
-        "Record the network bandwidth used for every profiler sample.")
+        "Record the network bandwidth used for every profiler sample.")    \
+  MACRO(                                                                   \
+      24, "memory", Memory,                                                \
+      "Track the memory allocations and deallocations per process over time.")
 // *** Synchronize with lists in BaseProfilerState.h and geckoProfiler.json ***
 
 struct ProfilerFeature {
@@ -138,6 +141,12 @@ struct ProfilerFeature {
   PROFILER_FOR_EACH_FEATURE(DECLARE)
 
 #undef DECLARE
+
+  [[nodiscard]] static constexpr bool ShouldInstallMemoryHooks(
+      uint32_t aFeatures) {
+    return ProfilerFeature::HasMemory(aFeatures) ||
+           ProfilerFeature::HasNativeAllocations(aFeatures);
+  }
 };
 
 // clang-format off