Closed Bug 1652048 Opened 4 years ago Closed 3 years ago

Potentially unnecessary idle tasks queued for background processes via PerformanceCounters

Categories

(Core :: Performance, defect)

defect

Tracking

()

RESOLVED WORKSFORME
Performance Impact low

People

(Reporter: mstange, Unassigned)

Details

(Keywords: perf:resource-use)

This was found in bug 1649976 comment 20 and following:

When Fission is enabled, and uBlock Origin is installed, loading certain web pages can cause activity in PerformanceCounters code in Fission content processes that are otherwise completely idle.

It is unclear whether this is a problem, or whether it is expected, and it is also unclear if it is related to uBlock Origin or whether it can happen with any add-on.

Tarek, is this necessary? It seems like something we wouldn't want to be doing if we have many fission processes, for battery reasons if nothing else.

Flags: needinfo?(tarek)
Whiteboard: [qf:p3:resource]

those counters are used in about:performance to display the JS activity in addons, it's sent back via IPDL every second (flushed)

We could tweak that option gTimingMaxAge so it's only every 5 seconds maybe?

content processes that are otherwise completely idle

are they really completely idle ? because if counters are sent back to the main process, it means they were incremented (unless there's a bug)

https://searchfox.org/mozilla-central/source/toolkit/components/extensions/PerformanceCounters.jsm#75

we could also add a new option that waits for the local counter to reach a minimum amount.

Notice that this flush mechanism was added by a request from Kris to avoid growing the memory footprint.
In my opinion, we would be ok flushing only of about:process asks for it (by keeping the counters incremented in each process, and flushed only if about:process asks for it) it's just a few numbers.

Flags: needinfo?(tarek) → needinfo?(kmaglione+bmo)

(In reply to Tarek Ziadé (:tarek) from comment #2)

are they really completely idle ? because if counters are sent back to the main process, it means they were incremented (unless there's a bug)

That's the question that needs to be investigated. They looked idle in the profile but maybe they weren't. To investigate this, I think it should be enough to use ublock origin with fission enabled and trigger a pageload, and then see whether we hit this code in all content processes or just in the ones that are involved in the loaded page.

I don't think anything needs to change here. If performance counters are being increased, the processes are definitely not idle. And if we flush only when about:performance asks, that means we wind up accumulating separate sets of performance counters in each process, and the memory of that could add up fairly significantly, from what I recall. Given the relatively low cost of the flush, I don't think there's any reason to remove it.

Flags: needinfo?(kmaglione+bmo)

triaging - Markus is this bug still relevant?

Flags: needinfo?(mstange.moz)

According to comment 4 this works as intended. And I haven't seen this again since I filed the bug. Let's close this.

Status: NEW → RESOLVED
Closed: 3 years ago
Flags: needinfo?(mstange.moz)
Resolution: --- → WORKSFORME
Performance Impact: --- → P3
Whiteboard: [qf:p3:resource]
You need to log in before you can comment on or make changes to this bug.