1503632 - High CPU usage when hovering links, thumbnails and even empty space

Status: RESOLVED FIXED

(Firefox :: New Tab Page, defect, P2)

Description

[Andreti]

Perf-html profile: http://bit.ly/2PA0Rtc
Video: https://www.youtube.com/watch?v=ht5GqZ_ukIg

Comment 1

[Asif Youssuff]

Dave, saw some of your comments on reddit and hoped you could direct this reddit spawned issue appropriately to the right component. Sorry about the NI!

https://www.reddit.com/r/firefox/comments/9sx52m/firefox_630b4_chew_up_20_of_a_32ghz_cpu_when_i/

Comment 2

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

It looks like at least in the thumbnail hover case, we're animating the box-shadow on the main thread (pretty easy to see with paint flashing).

Comment 3

[Florian Quèze [:florian]]

In the profile I see some activity in the parent process, reacting to the mousemove events. The related JS code in the profile doesn't exist anymore, as bug 1509489 has changed it significantly. Maybe this part is fixed, or at least improved. I assume this is the CPU use when hovering empty areas.

When hovering the thumbnails, I see in the profile that we spend a lot of time painting, and running with paint flashing shows that when hovering a "Top Sites" thumbnail, we sometimes repaint the whole "Highlights" section.

Tim, could someone in your team check what's going on on the Activity Stream side? Thanks!

Comment 4

[Tim Spurway [:tspurway]]

We have triaged this into our current iteration.  We will get it assigned and start working on it asap

Comment 5

[Ian Bicking (:ianbicking)]

I've done a bit of investigation. I don't think this has changed in the current code. This seems to be related to mouseenter/mouseleave/mouseout events.

If I disable the onMouseEnter/onMouseLeave events (in CollapsibleSection), AND disable mouseenter/mousemove in React itself, then the performance issue goes away. Even if there aren't any mouse events (that I can find) registered in our React code, React still seems to add event listeners. Looking at a few other React sites, they also seem to cause this same modest CPU spike during mouse movement.

Running document.addEventListener("mousemove", () => {}) in the console (e.g., on this page) causes similar behavior.

Comment 6

[Florian Quèze [:florian]]

(In reply to Ian Bicking (:ianbicking) from comment #5)

Do you know why we are repainting a much larger area when hovering a thumbnail? You can see this behavior by enabling paint flashing in about:config (nglayout.debug.paint_flashing).

Comment 7

[Dan Mosedale (:dmosedale, :dmose)]

Check the React relnotes for more recent versions to see if it's fixed.

If not, and you can test just installing the newest version with npm and then copying the installed minified thing from `node_modules` over to the right place in our vendor directory.

Comment 8

[Ian Bicking (:ianbicking)]

(In reply to Florian Quèze [:florian] from comment #6)
> Do you know why we are repainting a much larger area when hovering a
> thumbnail? You can see this behavior by enabling paint flashing in
> about:config (nglayout.debug.paint_flashing).

I'm not 100% clear, but I see several repaintings of the entire card when the context menu is animated into visibility on hover. Is this what you are referring to?

Re: React, there's no notes related to this in the changelog, and I believe the use of global event listeners is a deliberate design decision for React, purportedly for performance reasons. Those performance benefits seem unlikely in the case of mouse movement events, which fire continuously when asked for, yet there's few actual listeners.

Comment 9

[Florian Quèze [:florian]]

(In reply to Ian Bicking (:ianbicking) from comment #8)
> (In reply to Florian Quèze [:florian] from comment #6)
> > Do you know why we are repainting a much larger area when hovering a
> > thumbnail? You can see this behavior by enabling paint flashing in
> > about:config (nglayout.debug.paint_flashing).
> 
> I'm not 100% clear, but I see several repaintings of the entire card when
> the context menu is animated into visibility on hover. Is this what you are
> referring to?

https://youtu.be/nU3ey2ourc8 is what I'm seeing.

When hovering the 'Top Sites' items, the 'Highlights' area is sometimes repainted for no obvious reason.

Comment 10

[GitHub Bugzilla PR Linker]

Attachment: Link to GitHub pull-request: https://github.com/mozilla/activity-stream/pull/4590

Comment 11

[[github robot]]

Commit pushed to master at https://github.com/mozilla/activity-stream

https://github.com/mozilla/activity-stream/commit/8f0bbbffef8b115da304cfd641bd04dd95baba85
Fix Bug 1503632, reduce CPU usage when moving mouse on newtab (#4590)

This removes the use of React's onMouseEnter/onMouseLeave, which causes React to add a document-level mouseenter/mouseleave/mousemove handler. This handler causes any mouse movement to trigger an event (which React eventually throws away).

This patch instead manually creates a listener only for the elements we care about.

Comment 12

[Ed Lee :Mardak]

https://hg.mozilla.org/mozilla-central/rev/e1439f07a4a4

Comment 13

[Ryan VanderMeulen [:RyanVM]]

Is this something we should consider uplifting to Beta or can it ride the trains?

Comment 14

[Ian Bicking (:ianbicking)]

The performance issue is really very minor, so it can ride the trains.