From 5f92b8c1b5d4d194b438ed69fb2d34228f241564 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Naz=C4=B1m=20Can=20Alt=C4=B1nova?= Date: Thu, 5 Dec 2024 15:04:44 +0100 Subject: [PATCH 1/4] Add power consumption of all the processes while calculating the total power consumption Gecko profile json includes child processes inside it's `profile.processes` array. It's recursive data type that has the same properties. Previously we were only calculating the power consumption of the parent process. With this change, we should be able to add the consumption of all the processes, including the content process (which has the most impact), utility processes, webextension process etc. --- lib/firefox/geckoProfiler.js | 30 +++++++++++++++++++++--------- 1 file changed, 21 insertions(+), 9 deletions(-) diff --git a/lib/firefox/geckoProfiler.js b/lib/firefox/geckoProfiler.js index 1a83dd3757..2bd87bfecd 100644 --- a/lib/firefox/geckoProfiler.js +++ b/lib/firefox/geckoProfiler.js @@ -8,8 +8,8 @@ import { BrowserError } from '../support/errors.js'; const delay = ms => new Promise(res => setTimeout(res, ms)); const log = intel.getLogger('browsertime.firefox'); -// Return power usage in Wh -function computePowerSum(counter) { +// Return power usage in Wh for a single counter. +function computeCounterPowerSum(counter) { let sum = 0; // Older Firefoxes see https://github.com/sitespeedio/sitespeed.io/issues/3944#issuecomment-1871090793 if (counter.sample_groups) { @@ -28,6 +28,23 @@ function computePowerSum(counter) { return sum * 1e-12; } +// Return the power usage by the whole profile including the sub processes. +// The return value is in Wh value. +function computeFullProfilePower(profile) { + let power = 0; + for (const counter of profile.counters) { + if (counter.category === 'power') { + power += computeCounterPowerSum(counter); + } + } + + for (const subprocessProfile of profile.processes) { + power += computeFullProfilePower(subprocessProfile); + } + + return power; +} + /** * Timeout a promise after ms. Use promise.race to compete * about the timeout and the promise. @@ -220,13 +237,8 @@ export class GeckoProfiler { path.join(pathToFolder(url, options)) ) ); - let power = 0; - for (const counter of profile.counters) { - if (counter.category === 'power') { - power += computePowerSum(counter); - } - } - result.powerConsumption = Number(power); + + result.powerConsumption = computeFullProfilePower(profile); } else { log.warning( 'Missing power setting in geckoProfilerParams.features so power will not be collected' From 53c8bde1fedfca86622485a4e8d8da45909b8839 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Naz=C4=B1m=20Can=20Alt=C4=B1nova?= Date: Thu, 5 Dec 2024 18:14:41 +0100 Subject: [PATCH 2/4] Fix a typo --- lib/firefox/geckoProfiler.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/firefox/geckoProfiler.js b/lib/firefox/geckoProfiler.js index 2bd87bfecd..98b831cf94 100644 --- a/lib/firefox/geckoProfiler.js +++ b/lib/firefox/geckoProfiler.js @@ -230,7 +230,7 @@ export class GeckoProfiler { geckoProfilerDefaults.features ); if (chosenFeatures.includes('power')) { - log.info('Collecting CPU power consumtion'); + log.info('Collecting CPU power consumption'); const profile = JSON.parse( await storageManager.readData( `geckoProfile-${index}.json`, From 3003c71d50a155bd1ef7cf700d9b289a0f77f378 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Naz=C4=B1m=20Can=20Alt=C4=B1nova?= Date: Mon, 9 Dec 2024 13:02:37 +0100 Subject: [PATCH 3/4] Do not add the individual cpu component power values since 'CPU Package' will include them For Linux and Windows power values there is always a counter called 'CPU Package', this is the one we usually want. And besides this one, we might have individual CPU components, like DRAM, iGPU, individual CPU cores etc. These values are coming from individual components, but they are already included inside the 'CPU Package' metrics. So this means that in these platforms, if these individual components are captured, we are counting the power usage values twice. The values could look higher than it should. See the available tracks here: Linux: https://searchfox.org/mozilla-central/rev/5ad94327fc03d0e5bc6aa81c0d7d113c2dccf947/tools/profiler/core/PowerCounters-linux.cpp#48-70 Windows: https://searchfox.org/mozilla-central/rev/5ad94327fc03d0e5bc6aa81c0d7d113c2dccf947/tools/profiler/core/PowerCounters-win.cpp#32-55 So we shouldn't add them on top of the whole CPU package number, otherwise it might look like we consume double the power that CPU normally consumes. --- lib/firefox/geckoProfiler.js | 15 ++++++++++++++- 1 file changed, 14 insertions(+), 1 deletion(-) diff --git a/lib/firefox/geckoProfiler.js b/lib/firefox/geckoProfiler.js index 98b831cf94..f11fe39030 100644 --- a/lib/firefox/geckoProfiler.js +++ b/lib/firefox/geckoProfiler.js @@ -33,7 +33,20 @@ function computeCounterPowerSum(counter) { function computeFullProfilePower(profile) { let power = 0; for (const counter of profile.counters) { - if (counter.category === 'power') { + if ( + counter.category === 'power' && + // If power counters starts with "Power:", it means that there are several + // individual components that Firefox records. For Linux and Windows, + // intel CPUs might produce 'CPU package' to show the whole power + // consumption of the CPU. But on top of that it also shows some individual + // components like, DRAM, iGPU, individual CPU cores etc. We don't want + // to include them as it might produce double the power values it normally consumes. + // + // Linux track names: https://searchfox.org/mozilla-central/rev/5ad94327fc03d0e5bc6aa81c0d7d113c2dccf947/tools/profiler/core/PowerCounters-linux.cpp#48-70 + // Windows track names: https://searchfox.org/mozilla-central/rev/5ad94327fc03d0e5bc6aa81c0d7d113c2dccf947/tools/profiler/core/PowerCounters-win.cpp#32-55 + (!counter.name.startsWith('Power:') || + counter.name === 'Power: CPU package') + ) { power += computeCounterPowerSum(counter); } } From 9504fc78a13f9e8f6095f6fc6d44af752154d8ff Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Naz=C4=B1m=20Can=20Alt=C4=B1nova?= Date: Mon, 9 Dec 2024 13:16:37 +0100 Subject: [PATCH 4/4] Ignore the first counter value as it might be incorrect or unreliable This is already done in the profiler frontend here: https://github.com/firefox-devtools/profiler/blob/6a683fa6ae2733dae49ff460edf6ee98ca626f09/src/profile-logic/profile-data.js#L1740-L1743 Some platforms sometimes provide incorrect or unrealistic values. We should remove these values so it doesn't generate an incorrect value. Since this is only one value in a profile, we are not really concerned in case it's a correct number. It's a very small difference in a big array, removing this value is always safer. --- lib/firefox/geckoProfiler.js | 21 +++++++++++++++++---- 1 file changed, 17 insertions(+), 4 deletions(-) diff --git a/lib/firefox/geckoProfiler.js b/lib/firefox/geckoProfiler.js index f11fe39030..3eb8cb0702 100644 --- a/lib/firefox/geckoProfiler.js +++ b/lib/firefox/geckoProfiler.js @@ -15,16 +15,29 @@ function computeCounterPowerSum(counter) { if (counter.sample_groups) { for (const groups of counter.sample_groups) { const countIndex = groups.samples.schema.count; - for (const sample of groups.samples.data) { - sum += sample[countIndex]; + // NOTE: We intentionally ignore the first index of the counters as they + // might contain incorrect values. + for ( + let sampleIndex = 1; + sampleIndex < groups.samples.data.length; + sampleIndex++ + ) { + sum += groups.samples.data[sampleIndex][countIndex]; } } } else { const countIndex = counter.samples.schema.count; - for (const sample of counter.samples.data) { - sum += sample[countIndex]; + // NOTE: We intentionally ignore the first index of the counters as they + // might contain incorrect values. + for ( + let sampleIndex = 1; + sampleIndex < counter.samples.data.length; + sampleIndex++ + ) { + sum += counter.samples.data[sampleIndex][countIndex]; } } + return sum * 1e-12; }