Skip to content

Commit

Permalink
Bug 1729815 - Remove unnecessary profiler feature "threads" - r=julie…
Browse files Browse the repository at this point in the history 
…nw,perftest-reviewers,AlexandruIonescu

This feature doesn't have any effect anymore.

Differential Revision: https://phabricator.services.mozilla.com/D133860
  • Loading branch information
@squelart
squelart committed Dec 16, 2021
1 parent 4226932 commit c40dcd1
Show file tree
Hide file tree
Showing 18 changed files with 60 additions and 95 deletions.
1 change: 0 additions & 1 deletion devtools/shared/performance-new/gecko-profiler-interface.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,6 @@ class ActorReadyGeckoProfilerInterface {
"stackwalk",
"cpu",
"responsiveness",
"threads",
"leaf",
],
threads: options.threads || ["GeckoMain", "Compositor"],
Expand Down
13 changes: 2 additions & 11 deletions mozglue/baseprofiler/core/platform.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -231,8 +231,7 @@ static uint32_t AvailableFeatures() {
// Default features common to all contexts (even if not available).
static constexpr uint32_t DefaultFeatures() {
return ProfilerFeature::Java | ProfilerFeature::JS | ProfilerFeature::Leaf |
ProfilerFeature::StackWalk | ProfilerFeature::Threads |
ProfilerFeature::CPUUtilization;
ProfilerFeature::StackWalk | ProfilerFeature::CPUUtilization;
}

// Extra default features when MOZ_PROFILER_STARTUP is set (even if not
Expand Down Expand Up @@ -604,13 +603,6 @@ class ActivePS {
// Filter out any features unavailable in this platform/configuration.
aFeatures &= AvailableFeatures();

// Always enable ProfilerFeature::Threads if we have a filter, because
// users sometimes ask to filter by a list of threads but forget to
// explicitly specify ProfilerFeature::Threads.
if (aFilterCount > 0) {
aFeatures |= ProfilerFeature::Threads;
}

// Some features imply others.
if (aFeatures & ProfilerFeature::FileIOAll) {
aFeatures |= ProfilerFeature::MainThreadIO | ProfilerFeature::FileIO;
Expand Down Expand Up @@ -751,8 +743,7 @@ class ActivePS {

static bool ShouldProfileThread(PSLockRef aLock, ThreadInfo* aInfo) {
MOZ_ASSERT(sInstance);
return ((aInfo->IsMainThread() || FeatureThreads(aLock)) &&
sInstance->ThreadSelected(aInfo->Name()));
return sInstance->ThreadSelected(aInfo->Name());
}

PS_GET(uint32_t, Generation)
Expand Down
28 changes: 13 additions & 15 deletions mozglue/baseprofiler/public/BaseProfilerState.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -178,46 +178,44 @@ class MOZ_RAII AutoProfilerStats {
MACRO(9, "stackwalk", StackWalk, \
"Walk the C++ stack, not available on all platforms") \
\
MACRO(10, "threads", Threads, "Profile the registered secondary threads") \
MACRO(10, "jstracer", JSTracer, "Enable tracing of the JavaScript engine") \
\
MACRO(11, "jstracer", JSTracer, "Enable tracing of the JavaScript engine") \
\
MACRO(12, "jsallocations", JSAllocations, \
MACRO(11, "jsallocations", JSAllocations, \
"Have the JavaScript engine track allocations") \
\
MACRO(13, "nostacksampling", NoStackSampling, \
MACRO(12, "nostacksampling", NoStackSampling, \
"Disable all stack sampling: Cancels \"js\", \"leaf\", " \
"\"stackwalk\" and labels") \
\
MACRO(14, "preferencereads", PreferenceReads, \
MACRO(13, "preferencereads", PreferenceReads, \
"Track when preferences are read") \
\
MACRO(15, "nativeallocations", NativeAllocations, \
MACRO(14, "nativeallocations", NativeAllocations, \
"Collect the stacks from a smaller subset of all native " \
"allocations, biasing towards collecting larger allocations") \
\
MACRO(16, "ipcmessages", IPCMessages, \
MACRO(15, "ipcmessages", IPCMessages, \
"Have the IPC layer track cross-process messages") \
\
MACRO(17, "audiocallbacktracing", AudioCallbackTracing, \
MACRO(16, "audiocallbacktracing", AudioCallbackTracing, \
"Audio callback tracing") \
\
MACRO(18, "cpu", CPUUtilization, "CPU utilization") \
MACRO(17, "cpu", CPUUtilization, "CPU utilization") \
\
MACRO(19, "notimerresolutionchange", NoTimerResolutionChange, \
MACRO(18, "notimerresolutionchange", NoTimerResolutionChange, \
"Do not adjust the timer resolution for fast sampling, so that " \
"other Firefox timers do not get affected") \
\
MACRO(20, "cpuallthreads", CPUAllThreads, \
MACRO(19, "cpuallthreads", CPUAllThreads, \
"Sample the CPU utilization of all registered threads") \
\
MACRO(21, "samplingallthreads", SamplingAllThreads, \
MACRO(20, "samplingallthreads", SamplingAllThreads, \
"Sample the stacks of all registered threads") \
\
MACRO(22, "markersallthreads", MarkersAllThreads, \
MACRO(21, "markersallthreads", MarkersAllThreads, \
"Record markers from all registered threads") \
\
MACRO(23, "unregisteredthreads", UnregisteredThreads, \
MACRO(22, "unregisteredthreads", UnregisteredThreads, \
"Discover and profile unregistered threads -- beware: expensive!")
// *** Synchronize with lists in ProfilerState.h and geckoProfiler.json ***

Expand Down
3 changes: 1 addition & 2 deletions mozglue/tests/TestBaseProfiler.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3931,8 +3931,7 @@ void TestProfiler() {
// Profile all registered threads.
MOZ_RELEASE_ASSERT(filters.append(""));
const uint32_t features = baseprofiler::ProfilerFeature::Leaf |
baseprofiler::ProfilerFeature::StackWalk |
baseprofiler::ProfilerFeature::Threads;
baseprofiler::ProfilerFeature::StackWalk;
baseprofiler::profiler_start(baseprofiler::BASE_PROFILER_DEFAULT_ENTRIES,
BASE_PROFILER_DEFAULT_INTERVAL, features,
filters.begin(), filters.length());
Expand Down
2 changes: 1 addition & 1 deletion testing/raptor/webext/raptor/runner.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -382,7 +382,7 @@ async function startGeckoProfiling() {
);
const features = geckoFeatures
? geckoFeatures.split(",")
: ["js", "leaf", "stackwalk", "cpu", "threads", "responsiveness"];
: ["js", "leaf", "stackwalk", "cpu", "responsiveness"];
await ext.geckoProfiler.start({
bufferSize: geckoEntries,
interval: geckoInterval,
Expand Down
1 change: 0 additions & 1 deletion toolkit/components/extensions/schemas/geckoProfiler.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,6 @@
"screenshots",
"seqstyle",
"stackwalk",
"threads",
"jstracer",
"jsallocations",
"nostacksampling",
Expand Down
14 changes: 3 additions & 11 deletions tools/profiler/core/platform.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -274,8 +274,8 @@ static uint32_t AvailableFeatures() {
// Default features common to all contexts (even if not available).
static constexpr uint32_t DefaultFeatures() {
return ProfilerFeature::Java | ProfilerFeature::JS | ProfilerFeature::Leaf |
ProfilerFeature::StackWalk | ProfilerFeature::Threads |
ProfilerFeature::CPUUtilization | ProfilerFeature::Screenshots;
ProfilerFeature::StackWalk | ProfilerFeature::CPUUtilization |
ProfilerFeature::Screenshots;
}

// Extra default features when MOZ_PROFILER_STARTUP is set (even if not
Expand Down Expand Up @@ -648,13 +648,6 @@ class ActivePS {
// Filter out any features unavailable in this platform/configuration.
aFeatures &= AvailableFeatures();

// Always enable ProfilerFeature::Threads if we have a filter, because
// users sometimes ask to filter by a list of threads but forget to
// explicitly specify ProfilerFeature::Threads.
if (aFilterCount > 0) {
aFeatures |= ProfilerFeature::Threads;
}

// Some features imply others.
if (aFeatures & ProfilerFeature::FileIOAll) {
aFeatures |= ProfilerFeature::MainThreadIO | ProfilerFeature::FileIO;
Expand Down Expand Up @@ -846,8 +839,7 @@ class ActivePS {
static ThreadProfilingFeatures ProfilingFeaturesForThread(
PSLockRef aLock, const ThreadRegistrationInfo& aInfo) {
MOZ_ASSERT(sInstance);
if ((aInfo.IsMainThread() || FeatureThreads(aLock)) &&
sInstance->ThreadSelected(aInfo.Name())) {
if (sInstance->ThreadSelected(aInfo.Name())) {
// This thread was selected by the user, record everything.
return ThreadProfilingFeatures::Any;
}
Expand Down
28 changes: 13 additions & 15 deletions tools/profiler/public/ProfilerState.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -57,46 +57,44 @@
MACRO(9, "stackwalk", StackWalk, \
"Walk the C++ stack, not available on all platforms") \
\
MACRO(10, "threads", Threads, "Profile the registered secondary threads") \
MACRO(10, "jstracer", JSTracer, "Enable tracing of the JavaScript engine") \
\
MACRO(11, "jstracer", JSTracer, "Enable tracing of the JavaScript engine") \
\
MACRO(12, "jsallocations", JSAllocations, \
MACRO(11, "jsallocations", JSAllocations, \
"Have the JavaScript engine track allocations") \
\
MACRO(13, "nostacksampling", NoStackSampling, \
MACRO(12, "nostacksampling", NoStackSampling, \
"Disable all stack sampling: Cancels \"js\", \"leaf\", " \
"\"stackwalk\" and labels") \
\
MACRO(14, "preferencereads", PreferenceReads, \
MACRO(13, "preferencereads", PreferenceReads, \
"Track when preferences are read") \
\
MACRO(15, "nativeallocations", NativeAllocations, \
MACRO(14, "nativeallocations", NativeAllocations, \
"Collect the stacks from a smaller subset of all native " \
"allocations, biasing towards collecting larger allocations") \
\
MACRO(16, "ipcmessages", IPCMessages, \
MACRO(15, "ipcmessages", IPCMessages, \
"Have the IPC layer track cross-process messages") \
\
MACRO(17, "audiocallbacktracing", AudioCallbackTracing, \
MACRO(16, "audiocallbacktracing", AudioCallbackTracing, \
"Audio callback tracing") \
\
MACRO(18, "cpu", CPUUtilization, "CPU utilization") \
MACRO(17, "cpu", CPUUtilization, "CPU utilization") \
\
MACRO(19, "notimerresolutionchange", NoTimerResolutionChange, \
MACRO(18, "notimerresolutionchange", NoTimerResolutionChange, \
"Do not adjust the timer resolution for sampling, so that other " \
"Firefox timers do not get affected") \
\
MACRO(20, "cpuallthreads", CPUAllThreads, \
MACRO(19, "cpuallthreads", CPUAllThreads, \
"Sample the CPU utilization of all registered threads") \
\
MACRO(21, "samplingallthreads", SamplingAllThreads, \
MACRO(20, "samplingallthreads", SamplingAllThreads, \
"Sample the stacks of all registered threads") \
\
MACRO(22, "markersallthreads", MarkersAllThreads, \
MACRO(21, "markersallthreads", MarkersAllThreads, \
"Record markers from all registered threads") \
\
MACRO(23, "unregisteredthreads", UnregisteredThreads, \
MACRO(22, "unregisteredthreads", UnregisteredThreads, \
"Discover and profile unregistered threads -- beware: expensive!")
// *** Synchronize with lists in BaseProfilerState.h and geckoProfiler.json ***

Expand Down
4 changes: 2 additions & 2 deletions tools/profiler/tests/browser/browser_test_feature_ipcmessages.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ add_task(async function test_profile_feature_ipcmessges() {
const url = BASE_URL + "simple.html";

info("Open a tab while profiling IPC messages.");
startProfiler({ features: ["threads", "leaf", "ipcmessages"] });
startProfiler({ features: ["leaf", "ipcmessages"] });
info("Started the profiler sucessfully! Now, let's open a tab.");

await BrowserTestUtils.withNewTab(url, async contentBrowser => {
Expand Down Expand Up @@ -71,7 +71,7 @@ add_task(async function test_profile_feature_ipcmessges() {
});

info("Now open a tab without profiling IPC messages.");
startProfiler({ features: ["threads", "leaf"] });
startProfiler({ features: ["leaf"] });

await BrowserTestUtils.withNewTab(url, async contentBrowser => {
const contentPid = await SpecialPowers.spawn(
Expand Down
4 changes: 2 additions & 2 deletions tools/profiler/tests/browser/browser_test_feature_jsallocations.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ add_task(async function test_profile_feature_jsallocations() {
"The profiler is not currently active"
);

startProfiler({ features: ["threads", "js", "jsallocations"] });
startProfiler({ features: ["js", "jsallocations"] });

const url = BASE_URL + "do_work_500ms.html";
await BrowserTestUtils.withNewTab(url, async contentBrowser => {
Expand Down Expand Up @@ -48,7 +48,7 @@ add_task(async function test_profile_feature_jsallocations() {
);
}

startProfiler({ features: ["threads", "js"] });
startProfiler({ features: ["js"] });
// Now reload the tab with a clean run.
gBrowser.reload();
await wait(500);
Expand Down
4 changes: 2 additions & 2 deletions tools/profiler/tests/browser/browser_test_feature_nostacksampling.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ add_task(async function test_profile_feature_nostacksampling() {
"The profiler is not currently active"
);

startProfiler({ features: ["threads", "js", "nostacksampling"] });
startProfiler({ features: ["js", "nostacksampling"] });

const url = BASE_URL + "do_work_500ms.html";
await BrowserTestUtils.withNewTab(url, async contentBrowser => {
Expand Down Expand Up @@ -46,7 +46,7 @@ add_task(async function test_profile_feature_nostacksampling() {

// Flush out any straggling allocation markers that may have not been collected
// yet by starting and stopping the profiler once.
startProfiler({ features: ["threads", "js"] });
startProfiler({ features: ["js"] });

// Now reload the tab with a clean run.
gBrowser.reload();
Expand Down
4 changes: 2 additions & 2 deletions tools/profiler/tests/browser/browser_test_feature_preferencereads.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ add_task(async function test_profile_feature_preferencereads() {
"The profiler is not currently active"
);

startProfiler({ features: ["threads", "leaf", "preferencereads"] });
startProfiler({ features: ["leaf", "preferencereads"] });

const url = BASE_URL + "fixed_height.html";
await BrowserTestUtils.withNewTab(url, async contentBrowser => {
Expand All @@ -67,7 +67,7 @@ add_task(async function test_profile_feature_preferencereads() {
);
}

startProfiler({ features: ["threads", "leaf"] });
startProfiler({ features: ["leaf"] });
// Now reload the tab with a clean run.
await ContentTask.spawn(contentBrowser, null, () => {
return new Promise(resolve => {
Expand Down
2 changes: 1 addition & 1 deletion tools/profiler/tests/chrome/test_profile_worker.html
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@
const settings = {
entries: 1000000, // 9MB
interval: 1, // ms
features: ["js", "threads", "leaf", "stackwalk", "cpu"],
features: ["js", "leaf", "stackwalk", "cpu"],
threads: ["GeckoMain", "Compositor", "Worker"], // most common combination
};

Expand Down
2 changes: 1 addition & 1 deletion tools/profiler/tests/chrome/test_profile_worker_bug_1428076.html
View file
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@
const settings = {
entries: 1000000, // 9MB
interval: 1, // ms
features: ["js", "threads", "leaf", "stackwalk"],
features: ["js", "leaf", "stackwalk"],
threads: ["GeckoMain", "Compositor", "Worker"], // most common combination
};

Expand Down
Loading

0 comments on commit c40dcd1

Please sign in to comment.