From a01791e36e31bdd518ad0799dedff69207c4e35c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Naz=C4=B1m=20Can=20Alt=C4=B1nova?= Date: Mon, 9 Dec 2024 21:31:01 +0100 Subject: [PATCH] Improve how we compute the power consumption numbers from the Firefox Profiler output (#2220) * 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. * Fix a typo * 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. * 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 | 66 ++++++++++++++++++++++++++++-------- 1 file changed, 52 insertions(+), 14 deletions(-) diff --git a/lib/firefox/geckoProfiler.js b/lib/firefox/geckoProfiler.js index 1a83dd3757..3eb8cb0702 100644 --- a/lib/firefox/geckoProfiler.js +++ b/lib/firefox/geckoProfiler.js @@ -8,26 +8,69 @@ 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) { 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; } +// 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' && + // 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); + } + } + + 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. @@ -213,20 +256,15 @@ 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`, 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'