Improve how we compute the power consumption numbers from the Firefox Profiler output #2220
Conversation
…l 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.
…ge' 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.
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.
|
Hi @canova thanks for the fixes, I'll take a look tonight. So there's another thing about the calculations in sitespeedio/sitespeed.io#4329 - I've verified that too: if you test multiple URLs within one run, the cpu metrics in the trace seems to increase. Is that how it works (e.g we need to change our implementation on our side if a user tests multiple pages). |
|
@soulgalore Ah, I wasn't aware of this issue. It seems like it's because of the 3rd item I mentioned in the list above, which is being fixed here: 9504fc7 To give more context, it seems like the first values in the counter samples have different meanings per platform. I'm assuming you and @jannekalliola have macOS with Apple Silicon chips, because this is the behavior I encountered while I was testing too. It seems like when we call the macOS API for getting the power usage values for the first time, it's returning a very high number, which @fqueze suspected that it's the power value since the process start time. This explains why we are seeing increasing amount of values in the sitespeed.io power metrics. It's because it's never being reset, so it's always include the power usage numbers since the process has started. To fix this, we simply ignore the initial values now in 9504fc7. I tested with this PR applied on sitespeed.io and it seems like we are now getting expected values, like: Also, as I side note, the units in the sitespeed.io results look incorrect. They should be |
Hey @soulgalore!
I was looking at some of the profiles and the powerConsumption metrics that's generated using the Firefox Profiler. And realized that there were some issues related to how we compute this number.
This feature was first implemented here (and this is the issue)
processesfield, so now we look at the profiles recursively and sum all the power consumption counters coming from all the processes.countvalues. In some platforms the first value can either be different, and might mean other things, or it can be incorrect. We already have this logic in the profiler frontend to ignore the first value for all the counters, so we weren't hitting this issue before. But I encountered this issue while I was comparing the values from the profiler frontend vs the browsertime.I recommend looking at the PR commit-by-commit as the commit messages also provide some context.
To be able to test locally, you can follow the instructions here: sitespeedio/sitespeed.io#3944 (comment)