Backed out 4 changesets (bug 1735397) for causing spidermonkey build bustages on check_vanilla_allocations.py. CLOSED TREE

Backed out changeset 724e44c19665 (bug 1735397)
Backed out changeset 64546b56bac0 (bug 1735397)
Backed out changeset d55f04c0a444 (bug 1735397)
Backed out changeset 19762488b965 (bug 1735397)

mozglue/baseprofiler/core/BaseAndGeckoProfilerDetail.cpp

@@ -1,55 +0,0 @@
-/* -*- Mode: C++; tab-width: 2; indent-tabs-mode: nil; c-basic-offset: 2 -*- */
-/* vim: set ts=8 sts=2 et sw=2 tw=80: */
-/* This Source Code Form is subject to the terms of the Mozilla Public
- * License, v. 2.0. If a copy of the MPL was not distributed with this
- * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
-
-#include "mozilla/BaseAndGeckoProfilerDetail.h"
-
-#include <cstring>
-#include <string>
-#include <string_view>
-
-namespace mozilla::profiler::detail {
-
-constexpr std::string_view scPidPrefix = "pid:";
-
-[[nodiscard]] MFBT_API bool FilterHasPid(
-    const char* aFilter, baseprofiler::BaseProfilerProcessId aPid) {
-  if (strncmp(aFilter, scPidPrefix.data(), scPidPrefix.length()) != 0) {
-    // The filter is not starting with "pid:".
-    return false;
-  }
-
-  const std::string pidString = std::to_string(aPid.ToNumber());
-  return pidString == (aFilter + scPidPrefix.length());
-}
-
-[[nodiscard]] MFBT_API bool FiltersExcludePid(
-    Span<const char* const> aFilters,
-    baseprofiler::BaseProfilerProcessId aPid) {
-  if (aFilters.empty()) {
-    return false;
-  }
-
-  // First, check if the list only contains "pid:..." strings.
-  for (const char* const filter : aFilters) {
-    if (strncmp(filter, scPidPrefix.data(), scPidPrefix.length()) != 0) {
-      // At least one filter is *not* a "pid:...", our pid is not excluded.
-      return false;
-    }
-  }
-
-  // Here, all filters start with "pid:". Check if the given pid is included.
-  const std::string pidString = std::to_string(aPid.ToNumber());
-  for (const char* const filter : aFilters) {
-    if (pidString == (filter + scPidPrefix.length())) {
-      // Our pid is present, so it's not excluded.
-      return false;
-    }
-  }
-  // Our pid was not in a list of only pids, so it's excluded.
-  return true;
-}
-
-}  // namespace mozilla::profiler::detail

mozglue/baseprofiler/core/platform.cpp

@@ -39,7 +39,6 @@
 // #include "memory_hooks.h"
 #include "mozilla/ArrayUtils.h"
 #include "mozilla/AutoProfilerLabel.h"
-#include "mozilla/BaseAndGeckoProfilerDetail.h"
 #include "mozilla/BaseProfilerDetail.h"
 #include "mozilla/DoubleConversion.h"
 #include "mozilla/Printf.h"
@@ -670,9 +669,13 @@ class ActivePS {
         return true;
       }
 
-      // If the filter is "pid:<my pid>", profile all threads.
-      if (mozilla::profiler::detail::FilterHasPid(filter.c_str())) {
-        return true;
+      // If the filter starts with pid:, check for a pid match
+      if (filter.find("pid:") == 0) {
+        std::string mypid =
+            std::to_string(profiler_current_process_id().ToNumber());
+        if (filter.compare(4, std::string::npos, mypid) == 0) {
+          return true;
+        }
       }
     }
 
@@ -2728,11 +2731,6 @@ void profiler_init(void* aStackTop) {
     if (startupFilters && startupFilters[0] != '\0') {
       filters = SplitAtCommas(startupFilters, filterStorage);
       LOG("- MOZ_PROFILER_STARTUP_FILTERS = %s", startupFilters);
-
-      if (mozilla::profiler::detail::FiltersExcludePid(filters)) {
-        LOG(" -> This process is excluded and won't be profiled");
-        return;
-      }
     }
 
     locked_profiler_start(lock, capacity, interval, features, filters.begin(),

mozglue/baseprofiler/moz.build

@@ -80,7 +80,6 @@ EXPORTS += [
 ]
 
 EXPORTS.mozilla += [
-    "public/BaseAndGeckoProfilerDetail.h",
     "public/BaseProfileJSONWriter.h",
     "public/BaseProfilerCounts.h",
     "public/BaseProfilerDetail.h",
@@ -109,7 +108,6 @@ EXPORTS.mozilla += [
 ]
 
 UNIFIED_SOURCES += [
-    "core/BaseAndGeckoProfilerDetail.cpp",
     "core/ProfilerUtils.cpp",
 ]
 

mozglue/baseprofiler/public/BaseAndGeckoProfilerDetail.h

@@ -1,43 +0,0 @@
-/* -*- Mode: C++; tab-width: 2; indent-tabs-mode: nil; c-basic-offset: 2 -*- */
-/* vim: set ts=8 sts=2 et sw=2 tw=80: */
-/* This Source Code Form is subject to the terms of the Mozilla Public
- * License, v. 2.0. If a copy of the MPL was not distributed with this
- * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
-
-// Internal Base and Gecko Profiler utilities.
-// It should declare or define things that are used in both profilers, but not
-// needed outside of the profilers.
-// In particular, it is *not* included in popular headers like BaseProfiler.h
-// and GeckoProfiler.h, to avoid rebuilding the world when this is modified.
-
-#ifndef BaseAndGeckoProfilerDetail_h
-#define BaseAndGeckoProfilerDetail_h
-
-#include "mozilla/BaseProfilerUtils.h"
-#include "mozilla/Span.h"
-#include "mozilla/Types.h"
-
-namespace mozilla::profiler::detail {
-
-// True if the filter is exactly "pid:<aPid>".
-[[nodiscard]] MFBT_API bool FilterHasPid(
-    const char* aFilter, baseprofiler::BaseProfilerProcessId aPid =
-                             baseprofiler::profiler_current_process_id());
-
-// Only true if the filters only contain "pid:..." strings, and *none* of them
-// is exactly "pid:<aPid>". E.g.:
-// - [], 123                     -> false (no pids)
-// - ["main"], 123               -> false (not all pids)
-// - ["main", "pid:123"], 123    -> false (not all pids)
-// - ["pid:123"], 123            -> false (all pids, including "pid:123")
-// - ["pid:123", "pid:456"], 123 -> false (all pids, including "pid:123")
-// - ["pid:456"], 123            -> true (all pids, but no "pid:123")
-// - ["pid:456", "pid:789"], 123 -> true (all pids, but no "pid:123")
-[[nodiscard]] MFBT_API bool FiltersExcludePid(
-    Span<const char* const> aFilters,
-    baseprofiler::BaseProfilerProcessId aPid =
-        baseprofiler::profiler_current_process_id());
-
-}  // namespace mozilla::profiler::detail
-
-#endif  // BaseAndGeckoProfilerDetail_h

mozglue/tests/TestBaseProfiler.cpp

@@ -7,7 +7,6 @@
 #include "BaseProfiler.h"
 
 #include "mozilla/Attributes.h"
-#include "mozilla/BaseAndGeckoProfilerDetail.h"
 #include "mozilla/BaseProfileJSONWriter.h"
 #include "mozilla/FloatingPoint.h"
 #include "mozilla/ProgressLogger.h"
@@ -230,77 +229,6 @@ void TestProfilerUtils() {
   printf("TestProfilerUtils done\n");
 }
 
-void TestBaseAndProfilerDetail() {
-  printf("TestBaseAndProfilerDetail...\n");
-
-  {
-    using mozilla::profiler::detail::FilterHasPid;
-    const auto pid123 =
-        mozilla::baseprofiler::BaseProfilerProcessId::FromNumber(123);
-
-    MOZ_RELEASE_ASSERT(!FilterHasPid("", pid123));
-    MOZ_RELEASE_ASSERT(!FilterHasPid("filter", pid123));
-    MOZ_RELEASE_ASSERT(!FilterHasPid("123", pid123));
-    MOZ_RELEASE_ASSERT(!FilterHasPid("pid123", pid123));
-    MOZ_RELEASE_ASSERT(!FilterHasPid("pid=123", pid123));
-    MOZ_RELEASE_ASSERT(!FilterHasPid("pid:", pid123));
-    MOZ_RELEASE_ASSERT(!FilterHasPid("pid:12", pid123));
-    MOZ_RELEASE_ASSERT(!FilterHasPid("pid:1234", pid123));
-    MOZ_RELEASE_ASSERT(FilterHasPid("pid:123", pid123));
-  }
-
-  {
-    using mozilla::profiler::detail::FiltersExcludePid;
-    const auto pid123 =
-        mozilla::baseprofiler::BaseProfilerProcessId::FromNumber(123);
-
-    MOZ_RELEASE_ASSERT(
-        !FiltersExcludePid(mozilla::Span<const char*>{}, pid123));
-
-    {
-      const char* const filters[] = {"main"};
-      MOZ_RELEASE_ASSERT(!FiltersExcludePid(filters, pid123));
-    }
-
-    {
-      const char* const filters[] = {"main", "pid:123"};
-      MOZ_RELEASE_ASSERT(!FiltersExcludePid(filters, pid123));
-    }
-
-    {
-      const char* const filters[] = {"main", "pid:456"};
-      MOZ_RELEASE_ASSERT(!FiltersExcludePid(filters, pid123));
-    }
-
-    {
-      const char* const filters[] = {"pid:123"};
-      MOZ_RELEASE_ASSERT(!FiltersExcludePid(filters, pid123));
-    }
-
-    {
-      const char* const filters[] = {"pid:123", "pid:456"};
-      MOZ_RELEASE_ASSERT(!FiltersExcludePid(filters, pid123));
-    }
-
-    {
-      const char* const filters[] = {"pid:456", "pid:123"};
-      MOZ_RELEASE_ASSERT(!FiltersExcludePid(filters, pid123));
-    }
-
-    {
-      const char* const filters[] = {"pid:456"};
-      MOZ_RELEASE_ASSERT(FiltersExcludePid(filters, pid123));
-    }
-
-    {
-      const char* const filters[] = {"pid:456", "pid:789"};
-      MOZ_RELEASE_ASSERT(FiltersExcludePid(filters, pid123));
-    }
-  }
-
-  printf("TestBaseAndProfilerDetail done\n");
-}
-
 void TestProportionValue() {
   printf("TestProportionValue...\n");
 
@@ -5201,7 +5129,6 @@ int main()
 #endif  // MOZ_GECKO_PROFILER
 
   TestProfilerUtils();
-  TestBaseAndProfilerDetail();
   TestProportionValue();
   TestProgressLogger();
   // Note that there are two `TestProfiler{,Markers}` functions above, depending

tools/profiler/core/platform.cpp

@@ -47,7 +47,6 @@
 #include "memory_hooks.h"
 #include "mozilla/ArrayUtils.h"
 #include "mozilla/AutoProfilerLabel.h"
-#include "mozilla/BaseAndGeckoProfilerDetail.h"
 #include "mozilla/ExtensionPolicyService.h"
 #include "mozilla/extensions/WebExtensionPolicy.h"
 #include "mozilla/Monitor.h"
@@ -781,9 +780,13 @@ class ActivePS {
         return true;
       }
 
-      // If the filter is "pid:<my pid>", profile all threads.
-      if (mozilla::profiler::detail::FilterHasPid(filter.c_str())) {
-        return true;
+      // If the filter starts with pid:, check for a pid match
+      if (filter.find("pid:") == 0) {
+        std::string mypid =
+            std::to_string(profiler_current_process_id().ToNumber());
+        if (filter.compare(4, std::string::npos, mypid) == 0) {
+          return true;
+        }
       }
     }
 
@@ -5003,11 +5006,6 @@ void profiler_init(void* aStackTop) {
     if (startupFilters && startupFilters[0] != '\0') {
       filters = SplitAtCommas(startupFilters, filterStorage);
       LOG("- MOZ_PROFILER_STARTUP_FILTERS = %s", startupFilters);
-
-      if (mozilla::profiler::detail::FiltersExcludePid(filters)) {
-        LOG(" -> This process is excluded and won't be profiled");
-        return;
-      }
     }
 
     const char* startupActiveTabID =

tools/profiler/gecko/ProfilerChild.cpp

@@ -298,19 +298,15 @@ void ProfilerChild::GatherProfileThreadFunction(
   using namespace mozilla::literals::ProportionValue_literals;  // For `1_pc`.
 
   auto writer = MakeUnique<SpliceableChunkedJSONWriter>();
-  if (!profiler_get_profile_json(
-          *writer,
-          /* aSinceTime */ 0,
-          /* aIsShuttingDown */ false,
-          progressLogger.CreateSubLoggerFromTo(
-              1_pc,
-              "profiler_get_profile_json_into_lazily_allocated_buffer started",
-              99_pc,
-              "profiler_get_profile_json_into_lazily_allocated_buffer done"))) {
-    // Failed to get a profile (profiler not running?), reset the writer
-    // pointer, so that we'll send an empty string.
-    writer.reset();
-  }
+  profiler_get_profile_json(
+      *writer,
+      /* aSinceTime */ 0,
+      /* aIsShuttingDown */ false,
+      progressLogger.CreateSubLoggerFromTo(
+          1_pc,
+          "profiler_get_profile_json_into_lazily_allocated_buffer started",
+          99_pc,
+          "profiler_get_profile_json_into_lazily_allocated_buffer done"));
 
   if (NS_WARN_IF(NS_FAILED(
           parameters->profilerChild->mThread->Dispatch(NS_NewRunnableFunction(
@@ -334,26 +330,17 @@ void ProfilerChild::GatherProfileThreadFunction(
                 // Shmem allocation and promise resolution must be made on the
                 // ProfilerChild thread, that's why this task was needed here.
                 mozilla::ipc::Shmem shmem;
-                if (writer) {
-                  writer->ChunkedWriteFunc().CopyDataIntoLazilyAllocatedBuffer(
-                      [&](size_t allocationSize) -> char* {
-                        if (parameters->profilerChild->AllocShmem(
-                                allocationSize,
-                                mozilla::ipc::Shmem::SharedMemory::TYPE_BASIC,
-                                &shmem)) {
-                          return shmem.get<char>();
-                        }
-                        return nullptr;
-                      });
-                  writer = nullptr;
-                } else {
-                  // No profile, send an empty string.
-                  if (parameters->profilerChild->AllocShmem(
-                          1, mozilla::ipc::Shmem::SharedMemory::TYPE_BASIC,
-                          &shmem)) {
-                    shmem.get<char>()[0] = '\0';
-                  }
-                }
+                writer->ChunkedWriteFunc().CopyDataIntoLazilyAllocatedBuffer(
+                    [&](size_t allocationSize) -> char* {
+                      if (parameters->profilerChild->AllocShmem(
+                              allocationSize,
+                              mozilla::ipc::Shmem::SharedMemory::TYPE_BASIC,
+                              &shmem)) {
+                        return shmem.get<char>();
+                      }
+                      return nullptr;
+                    });
+                writer = nullptr;
 
                 parameters->resolver(std::move(shmem));
               }))))) {

tools/profiler/gecko/ProfilerParent.cpp

@@ -12,7 +12,6 @@
 #endif
 
 #include "GeckoProfiler.h"
-#include "mozilla/BaseAndGeckoProfilerDetail.h"
 #include "mozilla/BaseProfilerDetail.h"
 #include "mozilla/ClearOnShutdown.h"
 #include "mozilla/DataMutex.h"
@@ -627,22 +626,15 @@ void ProfilerParent::Init() {
     ipcParams.features() = features;
     ipcParams.activeTabID() = activeTabID;
 
-    // If the filters exclude our pid, make sure it's stopped, otherwise
-    // continue with starting it.
-    if (!profiler::detail::FiltersExcludePid(
-            filters, ProfilerProcessId::FromNumber(mChildPid))) {
-      ipcParams.filters().SetCapacity(filters.length());
-      for (const char* filter : filters) {
-        ipcParams.filters().AppendElement(filter);
-      }
-
-      Unused << SendEnsureStarted(ipcParams);
-      RequestChunkManagerUpdate();
-      return;
+    for (uint32_t i = 0; i < filters.length(); ++i) {
+      ipcParams.filters().AppendElement(filters[i]);
     }
-  }
 
-  Unused << SendStop();
+    Unused << SendEnsureStarted(ipcParams);
+    RequestChunkManagerUpdate();
+  } else {
+    Unused << SendStop();
+  }
 }
 
 ProfilerParent::~ProfilerParent() {
@@ -761,21 +753,10 @@ void ProfilerParent::ProfilerStarted(nsIProfilerStartParams* aParams) {
   aParams->GetInterval(&ipcParams.interval());
   aParams->GetFeatures(&ipcParams.features());
   ipcParams.filters() = aParams->GetFilters().Clone();
-  // We need filters as a Span<const char*> to test pids in the lambda below.
-  auto filtersCStrings = nsTArray<const char*>{aParams->GetFilters().Length()};
-  for (const auto& filter : aParams->GetFilters()) {
-    filtersCStrings.AppendElement(filter.Data());
-  }
   aParams->GetActiveTabID(&ipcParams.activeTabID());
 
   ProfilerParentTracker::ProfilerStarted(ipcParams.entries());
   ProfilerParentTracker::Enumerate([&](ProfilerParent* profilerParent) {
-    if (profiler::detail::FiltersExcludePid(
-            filtersCStrings,
-            ProfilerProcessId::FromNumber(profilerParent->mChildPid))) {
-      // This pid is excluded, don't start the profiler at all.
-      return;
-    }
     Unused << profilerParent->SendStart(ipcParams);
     profilerParent->RequestChunkManagerUpdate();
   });

tools/profiler/tests/browser/browser.ini

@@ -18,11 +18,6 @@ support-files = fixed_height.html
 
 [browser_test_markers_parent_process.js]
 
-[browser_test_profile_capture_by_pid.js]
-skip-if = os == "win" && os_version == "6.1" # No thread names on win7, needed for these tests
-https_first_disabled = true
-support-files = single_frame.html
-
 [browser_test_profile_fission.js]
 support-files = single_frame.html
 

tools/profiler/tests/browser/browser_test_profile_capture_by_pid.js

@@ -1,206 +0,0 @@
-/* This Source Code Form is subject to the terms of the Mozilla Public
- * License, v. 2.0. If a copy of the MPL was not distributed with this
- * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
-
-function ProcessHasSamplerThread(process) {
-  return process.threads.some(t => t.name == "SamplerThread");
-}
-
-async function GetPidsWithSamplerThread() {
-  let parentProc = await ChromeUtils.requestProcInfo();
-
-  let pids = parentProc.children
-    .filter(ProcessHasSamplerThread)
-    .map(proc => proc.pid);
-  if (ProcessHasSamplerThread(parentProc)) {
-    pids.unshift(parentProc.pid);
-  }
-  return pids;
-}
-
-// fnFilterWithContentId: Called with content child pid, returns filters to use.
-// E.g.: 123 => ["GeckoMain", "pid:123"], or 123 => ["pid:456"].
-async function test_with_filter(fnFilterWithContentId) {
-  Assert.ok(!Services.profiler.IsActive());
-  info("Clear the previous pages just in case we still some open tabs.");
-  await Services.profiler.ClearAllPages();
-
-  info("Open a tab with single_frame.html in it.");
-  const url = BASE_URL + "single_frame.html";
-  return BrowserTestUtils.withNewTab(url, async function(contentBrowser) {
-    const contentPid = await SpecialPowers.spawn(contentBrowser, [], () => {
-      return Services.appinfo.processID;
-    });
-
-    Assert.deepEqual(
-      await GetPidsWithSamplerThread(),
-      [],
-      "There should be no SamplerThreads before starting the profiler"
-    );
-
-    info("Start the profiler to test filters including 'pid:<content>'.");
-    startProfiler({ threads: fnFilterWithContentId(contentPid) });
-
-    let pidsWithSamplerThread = null;
-    await TestUtils.waitForCondition(
-      async function() {
-        let pidsStringBefore = JSON.stringify(pidsWithSamplerThread);
-        pidsWithSamplerThread = await GetPidsWithSamplerThread();
-        return JSON.stringify(pidsWithSamplerThread) == pidsStringBefore;
-      },
-      "Wait for sampler threads to stabilize after profiler start",
-      /* interval (ms) */ 250,
-      /* maxTries */ 10
-    );
-
-    info("Capture the profile data.");
-    const profile = await stopAndGetProfile();
-
-    await TestUtils.waitForCondition(async function() {
-      return (await GetPidsWithSamplerThread()).length == 0;
-    }, "Wait for all sampler threads to stop after profiler stop");
-
-    return { contentPid, pidsWithSamplerThread, profile };
-  });
-}
-
-add_task(async function browser_test_profile_capture_along_with_content_pid() {
-  const {
-    contentPid,
-    pidsWithSamplerThread,
-    profile,
-  } = await test_with_filter(contentPid => ["GeckoMain", "pid:" + contentPid]);
-
-  Assert.greater(
-    pidsWithSamplerThread.length,
-    2,
-    "There should be lots of SamplerThreads after starting the profiler"
-  );
-
-  let contentProcessIndex = profile.processes.findIndex(
-    p => p.threads[0].pid == contentPid
-  );
-  Assert.notEqual(
-    contentProcessIndex,
-    -1,
-    "The content process should be present"
-  );
-
-  // Note: Some threads may not be registered, so we can't expect that many. But
-  // 10 is much more than the default 4.
-  Assert.greater(
-    profile.processes[contentProcessIndex].threads.length,
-    10,
-    "The content process should have many threads"
-  );
-
-  Assert.equal(
-    profile.threads.length,
-    1,
-    "The parent process should have only one thread"
-  );
-  Assert.equal(
-    profile.threads[0].name,
-    "GeckoMain",
-    "The parent process should have the main thread"
-  );
-});
-
-add_task(async function browser_test_profile_capture_along_with_other_pid() {
-  const parentPid = Services.appinfo.processID;
-  const {
-    contentPid,
-    pidsWithSamplerThread,
-    profile,
-  } = await test_with_filter(contentPid => ["GeckoMain", "pid:" + parentPid]);
-
-  Assert.greater(
-    pidsWithSamplerThread.length,
-    2,
-    "There should be lots of SamplerThreads after starting the profiler"
-  );
-
-  let contentProcessIndex = profile.processes.findIndex(
-    p => p.threads[0].pid == contentPid
-  );
-  Assert.notEqual(
-    contentProcessIndex,
-    -1,
-    "The content process should be present"
-  );
-
-  Assert.equal(
-    profile.processes[contentProcessIndex].threads.length,
-    1,
-    "The content process should have only one thread"
-  );
-
-  // Note: Some threads may not be registered, so we can't expect that many. But
-  // 10 is much more than the default 4.
-  Assert.greater(
-    profile.threads.length,
-    10,
-    "The parent process should have many threads"
-  );
-});
-
-add_task(async function browser_test_profile_capture_by_only_content_pid() {
-  const parentPid = Services.appinfo.processID;
-  const {
-    contentPid,
-    pidsWithSamplerThread,
-    profile,
-  } = await test_with_filter(contentPid => ["pid:" + contentPid]);
-
-  // The sampler thread always runs in the parent process, see bug 1754100.
-  Assert.deepEqual(
-    pidsWithSamplerThread,
-    [parentPid, contentPid],
-    "There should only be SamplerThreads in the parent and the target child"
-  );
-
-  Assert.equal(
-    profile.processes.length,
-    1,
-    "There should only be one child process"
-  );
-  // Note: Some threads may not be registered, so we can't expect that many. But
-  // 10 is much more than the default 4.
-  Assert.greater(
-    profile.processes[0].threads.length,
-    10,
-    "The child process should have many threads"
-  );
-  Assert.equal(
-    profile.processes[0].threads[0].pid,
-    contentPid,
-    "The only child process should be our content"
-  );
-});
-
-add_task(async function browser_test_profile_capture_by_only_parent_pid() {
-  const parentPid = Services.appinfo.processID;
-  const {
-    pidsWithSamplerThread,
-    profile,
-  } = await test_with_filter(contentPid => ["pid:" + parentPid]);
-
-  Assert.deepEqual(
-    pidsWithSamplerThread,
-    [parentPid],
-    "There should only be a SamplerThread in the parent"
-  );
-
-  // Note: Some threads may not be registered, so we can't expect that many. But
-  // 10 is much more than the default 4.
-  Assert.greater(
-    profile.threads.length,
-    10,
-    "The parent process should have many threads"
-  );
-  Assert.equal(
-    profile.processes.length,
-    0,
-    "There should be no child processes"
-  );
-});