1199987 - Find out how/why about:performance is slowing down Firefox

Status: RESOLVED WONTFIX

(Toolkit :: Performance Monitoring, defect)

Description

[David Teller [:Yoric] - still alive but not very active]

Currently, about:performance shows that about:performance slows down Firefox. We should investigate why.

Comment 1

[David Teller [:Yoric] - still alive but not very active]

Actually, I can guess. about:performance is taking snapshots of the entire state of Firefox, then applying complex algorithms to compute the difference between trees of snapshots and then using it to rebuild most of the DOM.

Now that we have PerformanceWatcher.jsm, we could redesign the flow to only update when a page/add-on is starting to cause jank/has stopped causing jank. This should be much more efficient, both in terms of DOM and in terms of statistics.

Comment 2

[Jens]

Attachment: 2016-06-09 10_23_22-about_performance - Firefox Developer Edition.png

With the Fx 47 release many people tried the new about:performance feature (which is great!). Biggest LOL in the comments was always that about:performance lists itself as slowing down Firefox. Well, that's a bit embarrassing imho and not the best marketing to have this issue here not fixed before the 47 release.

I have an older 32bit Duo Core Ubuntu 14.04 2.4 GHz (private) and a fairly new 64bit Windows 8.1 Intel i5-3570 (4 cores) with 3.40 GHz and 12 GB RAM. On both machines it's always: "about:performance may currently be slowing down Firefox." See screenshot.

Both for Firefox 47 and Firefox Developer Edition 48.0a2 (2016-06-06)

While I appreciate the honesty of "about:performance" I think some work should be put into optimizing it so that such a report does not happen under normal circumstances.

Comment 3

[David Teller [:Yoric] - still alive but not very active]

Well, it's mostly a matter of having the time to rewrite about:performance from scratch using PerformanceWatcher.jsm instead of PerformanceStats.jsm.

Comment 4

[David Teller [:Yoric] - still alive but not very active]

Rutuja, did you get a chance to get this rolling?

Comment 5

[rsurve]

(In reply to David Rajchenbach-Teller [:Yoric] (please use "needinfo") from comment #4)
> Rutuja, did you get a chance to get this rolling?

One of the major reasons why PerformanceStats slows down firefox is ‘polling’. The about:performance code relies on PerformanceStats that gives it a snapshot of the current values of jank, cpow and ticks, which are compared against the previous values of these variables. The difference or the ‘delta’ of these values is then compared against a threshold for good or average performance. PerformanceWatcher on the other hand uses an event-based model, wherein an alert notification is passed whenever a web-page or an add-on slows down. This saves a lot of computational overhead as there is no polling and active listeners do the work of reporting to the parent process, in the event when the performance of the child falls below a certain threshold. 
So, to sum it up:
1. When we try re-designing the aboutPerformance code using watcher, we don't need to do any delta calculations since a notification will be sent only when a web-page or add-on slows down.
2. The Watcher code uses Lower level functionalities of the stats code and use listeners to notify
The mechanism that it uses is still based on cpow, ticks and jank measurements.. except that we aren't doing it frequently and its event-triggered. 
3. If we are to use watcher instead of stats, we would have to replace the monitor with an event-listener trigger with otherwise similar attributes as the monitor.

I'll first try to make minor changes to the aboutperformance code to reflect watcher, once that gets working, I'll attempt making major changes, eliminating stats altogether..

Comment 6

[David Teller [:Yoric] - still alive but not very active]

(In reply to rsurve from comment #5)
> I'll first try to make minor changes to the aboutperformance code to reflect
> watcher, once that gets working, I'll attempt making major changes,
> eliminating stats altogether..

Note that we don't need the exact same statistics. Anything that informs users that there is a problem with a tab/add-on is useful.

Comment 7

[Rutuja]

This is how we could implement the changes:

1. Modify about:performance to eliminate the concept of 'delta' for computing the difference between two snapshots returned by the performancestats monitor with respect to jank and cpow, since we are not implementing thresholds. This means we eliminate polling by removing the delta prototype and several thresholds for measuring 'good'/'average'/'bad' performance.

2. Instead, use two listeners, one for reporting whenever an add-on slows down and the second for reporting whenever a tab slows down:

let Addon_listener = function(source, details) {

    console.log(`Oops, add-on ${source.addonId} seems to be slowing down Firefox.`, details);
  };
 PerformanceWatcher.addPerformanceListener({addonId: "[e-mail]"},  Addon_listener);
 //We can pass "*" as an argument instead of "[e-mail]" to be able to listen from all add-ons.
 


 let Window_listener = function(alerts) {
 
   for (let {source, details} of alerts) {
     console.log(`Oops, window ${source.windowId} seems to be slowing down Firefox.`, details);
   };
   // Special windowId 0 lets us to listen to all webpages.
  PerformanceWatcher.addPerformanceListener({windowId: 0}, Window_listener);

 Here, the arguments  'source' and 'destination' mean:
 source: {groupId, name, addonId, windowId, isSystem, processId} 
 Information on the source of the notification.

 details: {reason, highestJank, highestCPOW} 
 Information on the details of the notification.

3. PerformanceWatcher uses the concept of 'Observables' and 'Observers', which uses the PerformanceStats service by getting hold of the monitor:

 this._monitor = PerformanceStats.getMonitor(["jank", "cpow"]);

It does not use snapshots calculated by the monitor. Instead, it sets a threshold for
the service, which when crossed, sends an alert notification of the slowdown. 
This is the only place where threshold has been used in PerformanceWatcher:
performanceStatsService.jankAlertThreshold = 64000

We can adjust this value such that it represents severe impact or slowdown.

4. When these listeners are triggered, we can modify the 'View' section of about:performance and update the set of cached elements which are displayed with the elements that don't cause firefox to slowdown. Refresh and update should happen only when the alerts trigger the listeners.

With this, we won't have to bother about the statistics in detail (like max_frequency_for_no_impact, etc. and the thresholds for good/average performance). Alongside, we'll even be eliminating polling.
We'll have sufficient details coming from the alerts.

Comment 8

[David Teller [:Yoric] - still alive but not very active]

Rutuja, sounds like a plan. Remove direct dependencies on PerformanceStats and polling, replace it with PerformanceWatcher.

Comment 9

[Mike Conley (:mconley) (:⚙️)]

I agree that this plan sounds reasonable. One thing that might be worth considering is holding a "log" of these observables along with a timestamp for when they were captured. It should then be possible to show a timeline of how Firefox has been behaving for some interval of time (perhaps the last hour?). Again, I'm thinking of something like the line-graphs like in the Windows Task Manager.

I also know that billm has been interested in exposing more information that we get from Quantum DOM in about:performance. We should make sure we're in sync here so that we don't accidentally duplicate or invalidate work.

Comment 10

[Bill McCloskey [inactive unless it's an emergency] (:billm)]

Yes, I was planning on doing some work on about:performance. I would like to get to it in the next few weeks. Do you an idea when you'll be able to work on this, Rutuja?

Comment 11

[Rutuja]

(In reply to Bill McCloskey (:billm) from comment #10)
> Do you an idea when you'll be able to work
> on this, Rutuja?
I'll start working on this from now itself and will try to get it running in about a week or so.

Comment 12

[Rutuja]

Attachment: Discarding extra frequency impact details and using listeners

Comment 13

[Rutuja]

I have tried to come up with a patch which uses PerformanceWatcher and I haven't completely eliminated the Delta structure. Ideally, logging should happen whenever watcher detects a slowdown (which is isn't happening when I ran it), however a message saying "about:performance may be slowing down firefox" comes up, which happens when delta is less than max delta for average recent performance. The jank threshold for watcher is 64,000 and for max_delta_for_average_recent_performance is positive infinity in aboutperformance code. Why is this the case?
Whenever updates happen, on tracing the call stack from control->update to State.update and View.updatecategory, it is found that updates for current window/addons happen by 'promise snapshot'. Is this the place where the listeners should update on receiving a slowdown notification and also update cachedelements? Do the buffers used in the code correspond to snapshots calculated by performancestats?
Moreover, the framerate impact is 8 on 10, what does this indicate?

Comment 14

[David Teller [:Yoric] - still alive but not very active]

Apologies about the delay. I'll try and answer by end of day tomorrow.

Comment 15

[Mike Conley (:mconley) (:⚙️)]

Poke to Yoric

Comment 16

[David Teller [:Yoric] - still alive but not very active]

> The jank threshold for watcher is 64,000 and for max_delta_for_average_recent_performance is positive infinity in aboutperformance code. Why is this the case?

I don't see anything in the current aboutPerformance.js code that would make it infinite. Are you talking about the current aboutPerformance.js or your patched version?

> Whenever updates happen, on tracing the call stack from control->update to State.update and View.updatecategory, it is found that updates for current window/addons happen by 'promise snapshot'. Is this the place where the listeners should update on receiving a slowdown notification and also update cachedelements?

`promisedSnapshot` should disappear entirely, to be replaced by listeners.

> Do the buffers used in the code correspond to snapshots calculated by performancestats?

I think that the buffers should disappear.

> Moreover, the framerate impact is 8 on 10, what does this indicate?

Framerate impact is something provided by PerformanceStats/PerformanceWatcher to determine how many consecutive frames we skip. That's how we measure the slowdown. Unfortunately, it's not an exact measure, but the higher the framerate impact, the more visible the slowdown.

I hope this answers your questions. Apologies about the delay.

Comment 17

[Rutuja]

(In reply to David Teller [:Yoric] (please use "needinfo") from comment #16)
 
> I don't see anything in the current aboutPerformance.js code that would make
> it infinite. Are you talking about the current aboutPerformance.js or your
> patched version?
> 
By positive infinity, I meant something like:
Delta.MAX_DELTA_FOR_GOOD_RECENT_PERFORMANCE = {
  diff: {
    cpow: {
      totalCPOWTime: 0,
    },
    jank: {
      longestDuration: 3,
      totalUserTime: Number.POSITIVE_INFINITY,
      totalSystemTime: Number.POSITIVE_INFINITY
    },
    ticks: {
      ticks: Number.POSITIVE_INFINITY,
    }
  }
};
I'll try eliminating promisedSnapshot and buffers entirely and patch up again.
With our new approach, we wouldn't have detailed statistics about slowdown, but only a message saying if the tab currently performs well (which is the default message) or slows down firefox (which will override the default one, when the watcher event gets triggered) along with the timestamp detail.

Comment 18

[David Teller [:Yoric] - still alive but not very active]

(In reply to Rutuja from comment #17)
>  By positive infinity, I meant something like:
> Delta.MAX_DELTA_FOR_GOOD_RECENT_PERFORMANCE = {
>   diff: {
>     cpow: {
>       totalCPOWTime: 0,
>     },
>     jank: {
>       longestDuration: 3,
>       totalUserTime: Number.POSITIVE_INFINITY,
>       totalSystemTime: Number.POSITIVE_INFINITY
>     },
>     ticks: {
>       ticks: Number.POSITIVE_INFINITY,
>     }
>   }
> };

Ah, right. It just means that we don't care about `ticks` (i.e. the number of times the addon/webpage did something) to determine performance.

> I'll try eliminating promisedSnapshot and buffers entirely and patch up
> again.

Sounds good.

> With our new approach, we wouldn't have detailed statistics about slowdown,
> but only a message saying if the tab currently performs well (which is the
> default message) or slows down firefox (which will override the default one,
> when the watcher event gets triggered) along with the timestamp detail.

Yes, we would only have a message. If by "default one", you mean the yellow bar, then please don't touch it, that's another issue. I want to get rid of it entirely, but there is no agreement on the topic at the moment :)

Comment 19

[Florian Quèze [:florian]]

about:performance is being redesigned; mass closing the bugs related to parts of the current about:performance page that we are not keeping.

Our goals with the redesign are to reduce the overhead caused by having the page opened, increase the reliability of the displayed information, and make the offered information actionable for most users. The back-end work is being tracked in bug 1419681.