Closed Bug 1405411 Opened 7 years ago Closed 7 years ago

stylo: Loading YouTube's material design Subscriptions panel causes excessive CPU usage

Categories

(Core :: CSS Parsing and Computation, defect, P1)

57 Branch
defect

Tracking

()

RESOLVED FIXED
Tracking Status
firefox-esr52 --- unaffected
firefox56 --- unaffected
firefox57 + fixed
firefox58 --- fixed

People

(Reporter: smartfon.reddit, Assigned: heycam)

References

Details

(Keywords: regression)

Attachments

(1 obsolete file)

User Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:57.0) Gecko/20100101 Firefox/57.0
Build ID: 20171002181526

Steps to reproduce:

Use YouTube's new material design. Sign in and subscribe to 200 channels. Open https://www.youtube.com/feed/subscriptions 

The CPU usage can reach 40-60% and stays that way for 5-10 seconds, while the subscribed channel images/URLs load on the left panel under "SUBSCRIPTIONS".

Does not happen with the old YouTube layout. Does not happen if you sign into another material design account where you haven't subscribed to 200 channels. Does not happen on Chrome with material design and 200 subscribed channels.
User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:57.0) Gecko/20100101 Firefox/57.0
Build-ID: 20171002181526

I'm not sure if it's the same problem, but it sounds very much like the same i have.

Subscriptions: 256

I've recorded the behavior:

Old Layout: https://youtu.be/LF7MSzUwyFk

New layout: https://youtu.be/DFfNeQ7j5WE

You just have to tell me what kind of information you need and I try to provide as much as I can.
I got midway through reporting a bug before discovering this bug, so I'm going to share most of what I wrote to try and give this issue further context and helpful info. I feel it is a serious one.

====

A couple days ago a reddit user (on an extremely capable machine) reported suddenly seeing very poor performance on youtube. Disabling the new youtube layout resolved the issue. Note that they had had the new layout previously with no problem.

The issue started for them on the morning of Tuesday Oct 3.

https://www.reddit.com/r/firefox/comments/7411eg/youtube_high_cpuusage_firefox_57b4/

Multiple other users replied confirming the same experience - including the timing.
I've since seen a user on IRC with the same report and another report here
https://www.reddit.com/r/firefox/comments/74i1i4/youtube_slows_down_firefox_to_about_12_second/
(With another user reporting the same thing in the comments.)

I've posted replies to the affected individuals to gauge their subscription list size.
The reporter at the original link indicated their subscription list is 256. (Ah, it looks like the same person is here in comment 1 !)

There are reports from users on 56, 57 & 58, so version doesn't seem to matter. 

But the users don't seem to be reporting poor performance as momentary, per comment 0, but rather as continuous(!).

Even if this is not technically our fault it should be on our radar to address as it's negatively affecting user perception - and now's not a great time for reports of bad performance on one of the internet's largest sites.
Severity: normal → major
Status: UNCONFIRMED → NEW
Ever confirmed: true
Component: Untriaged → General
Product: Firefox → Core
It would be fantastic if you could record a profile while the CPU activity spikes using the profiler:

https://developer.mozilla.org/docs/Mozilla/Performance/Reporting_a_Performance_Problem

This would help us immensely to figure out what is going on.
User Agent 	Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:58.0) Gecko/20100101 Firefox/58.0
Build ID 	20171005220204


I was able to reproduce the issue on the latest Nightly 58.0a1 (Build ID 20171005220204) on Windows 10 and Mac 10.12. However I was also able to raise the CPU usage just by moving the mouse cursor to and from the subscriptions sidebar in while not logged into an account.

Neither behavior are reproducible on Firefox Release. Mozregression revealed the following:
Last good revision: bc829569880635c52d6e3d54f51cd7d3df180186
First bad revision: 4cfb674227051e22bab651e5759f3de503a50560
Pushlog:
https://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=bc829569880635c52d6e3d54f51cd7d3df180186&tochange=4cfb674227051e22bab651e5759f3de503a50560

I've also taken a cleopatra profile: https://perfht.ml/2fQ5jjZ.

:billm, could you please take a look at this?
Blocks: 1364821
Severity: major → normal
Component: General → XPCOM
Flags: needinfo?(wmccloskey)
Keywords: regression
OS: Unspecified → All
Hardware: Unspecified → All
I've recorded my profiles, hopefully that will help you!


User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:58.0) Gecko/20100101 Firefox/58.0
Build-ID: 20171006100327

youtube.com:

https://perfht.ml/2y4k24A


youtube.com/feed/subscriptions:

https://perfht.ml/2y4zKg8

----------------------

User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:57.0) Gecko/20100101 Firefox/57.0
Build-ID: 20171002181526

youtube.com:

https://perfht.ml/2y4WkFF


youtube.com/feed/subscriptions:

https://perfht.ml/2y4P902
I asked a reddit user with 575 subscriptions to run a profile. They shared the following:

> https://perfht.ml/2yMzLTn 
> Went though the home page a bit, loaded a video and fullscreened it a few times and tried to go to subs page witch led to a internal server error still.

User is on 56.
In a previous reply they indicated performance was somewhat better today making me wonder if youtube had improved the code.

Full conversation with them here: https://www.reddit.com/r/firefox/comments/7411eg/youtube_high_cpuusage_firefox_57b4/dnzmxt5/?context=10000
From these profiles, it looks like all the time is being spent in Stylo. That makes sense. I don't really believe the bisection result. I did my own bisection and came up with this:
https://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=320642944e42a889db13c6c55b404e32319d4de6&tochange=52285ea5e54c73d3ed824544cef2ee3f195f05e6

I don't know if that changeset had anything to do with Stylo being enabled. It just seems to be changing the version number to 57.

It makes sense that Stylo would cause higher CPU usage since it uses multiple threads. Cameron, feel free to pass this to whomever has time.
Flags: needinfo?(wmccloskey) → needinfo?(cam)
I've disabled Stylo and tested it again and it seems that fixed the high CPU usage problem.
I can reproduce this locally.  I created a new Google profile and added 75 channel subscriptions (which seemed to be some sort of limit, when adding subscriptions quickly).

https://perfht.ml/2fX8UjS

I haven't yet dug into exactly what the page is doing, though I can see from the profile that it's doing a bunch of work (in desktop_polymer.js) in requestAnimationFrame between the restyles.  In the restyles themselves, around half of the time is spent dealing with CSS custom property cascading and resolution.

Tentatively -> P2, since this seems like something important to fix.
Component: XPCOM → CSS Parsing and Computation
Priority: -- → P2
Summary: Loading YouTube's material design Subscriptions panel causes excessive CPU usage → stylo: Loading YouTube's material design Subscriptions panel causes excessive CPU usage
On my reddit post from October 3rd, quoted on Comment 2, i've talked to someone named "Yonez". We examined the issue together and he described whats exactly happening when the high CPU load kicks in:

https://www.reddit.com/r/firefox/comments/7411eg/youtube_high_cpuusage_firefox_57b4/dnvdt05/

"...One thing is certain; if it was the infinite scroll or the subscriptions bug returning we would see the network tab requests running like a train, which it doesn't. Everything is behaving as it should, until the player code kicks in (the base.js and/or www-player-xxxx.css file)."
(In reply to Cameron McCormack (:heycam) from comment #9)
> I can reproduce this locally.  I created a new Google profile and added 75
> channel subscriptions (which seemed to be some sort of limit, when adding
> subscriptions quickly).
> 
> https://perfht.ml/2fX8UjS

It seems to me that your profile is quite different than those provided in comment 5.

In the profiles in comment 5, I see majority of time (>50%) are spent on waiting for some lock, which may be related to font / color things, while this profile shows some long running cascading...
(In reply to Xidorn Quan [:xidorn] UTC+10 from comment #11)
> It seems to me that your profile is quite different than those provided in
> In the profiles in comment 5, I see majority of time (>50%) are spent on
> waiting for some lock, which may be related to font / color things, while
> this profile shows some long running cascading...

For whatever reason, it looks the profiles from others above don't show the work happening on the style worker threads, so all we see is the time spent waiting for the rayon lock.  (My profile also has that lock waiting time, in addition to the restyling working going on.)
Flags: needinfo?(cam)
Here is yet another recent post about poor performance and lagginess of the youtube interface with several "me too" replies.
https://www.reddit.com/r/firefox/comments/74mxjk/firefox_is_really_lagging_on_youtube/

That user is on 56. Per my previous comment, I've seen multiple other users reporting sudden poor performance.

It's possible that Stylo is a part of the equation, but with several 56 users reporting at the same time, I'm not convinced it's the main problem.
It's possible the 56 users were in the stylo pref experiment population.  (You could ask them to check about:support to see what it says in the "Stylo" row under "Application Basics", if you want to know for sure.)
Although I guess we don't run the experiment on release, which is where 56 is now...
(In reply to Cameron McCormack (:heycam) from comment #12)
> (In reply to Xidorn Quan [:xidorn] UTC+10 from comment #11)
> > It seems to me that your profile is quite different than those provided in
> > In the profiles in comment 5, I see majority of time (>50%) are spent on
> > waiting for some lock, which may be related to font / color things, while
> > this profile shows some long running cascading...
> 
> For whatever reason, it looks the profiles from others above don't show the
> work happening on the style worker threads, so all we see is the time spent
> waiting for the rayon lock.  (My profile also has that lock waiting time, in
> addition to the restyling working going on.)

Ah... Gecko Profiler don't track style threads by default! That's terrible... we should fix that...
What i've also noted is that when Stylo is disabled, even when the high CPU usage is gone, the Youtube-UI stays a bit laggy (HW Acceleration is enabled). 

- Clicking on the upper right menu icon takes 1-3 sec. until the menu appears
- Scrolling via scroll bar or the WebExtension "ScrollAnywhere" feels clunky and choppy.
(In reply to Cameron McCormack (:heycam) from comment #9)
> I can reproduce this locally.  I created a new Google profile and added 75
> channel subscriptions (which seemed to be some sort of limit, when adding
> subscriptions quickly).
> 
> https://perfht.ml/2fX8UjS

Was this profile done with debug assertions? I'm surprised to swe SliceExt::contains in there.
Yeah, it was a local build that probably was --enable-debug --enable-optimize.
I have an issue with the new YouTube layout.

steps to reproduce:

1-open YouTube trending page for example.

2-refresh this page, I see about 60% CPU usage With Firefox 57 (same results with private windows).

3-refresh the same page on chrome 61, I see only 15% CPU usage.


Firefox 57 beta 6, Windows 10 (1703) , i7 3770K CPU, HW is disabled(blacklisted)
(In reply to Xidorn Quan [:xidorn] UTC+10 from comment #16)
> (In reply to Cameron McCormack (:heycam) from comment #12)
> > (In reply to Xidorn Quan [:xidorn] UTC+10 from comment #11)
> > > It seems to me that your profile is quite different than those provided in
> > > In the profiles in comment 5, I see majority of time (>50%) are spent on
> > > waiting for some lock, which may be related to font / color things, while
> > > this profile shows some long running cascading...

The work is all under Servo_TraverseSubtree, which is the entry point for restyling.

> > 
> > For whatever reason, it looks the profiles from others above don't show the
> > work happening on the style worker threads, so all we see is the time spent
> > waiting for the rayon lock.  (My profile also has that lock waiting time, in
> > addition to the restyling working going on.)
> 
> Ah... Gecko Profiler don't track style threads by default! That's
> terrible... we should fix that...

Not sure I agree. Profiling the style threads by default would add 3, 6, or more threads, which is a significant increase in overhead, and clutters the UI. It's very easy to tell if a profile is spending time on style threads by filtering for Servo_TraverseSubtree. And profiling the precise work on the style threads is also easy, but just add "StyleThread" to the list of threads and reprofiling.
So, there may be several things going on here. I believe youtube is currently rolling out a new, polymer/WebComponents redesign, for which Firefox (which doesn't yet support WebComponents) gets a polyfill. I have been tracking a lot of custom properties usage on youtube, which may or may not be related.

From the profiles, it looks like some combination of the polyfill and stylo are causing us to do tons or repeated restyles. We have the channels to ask for help from Youtube if we need it, but the first step is to understand the problem.

This issue is a significant risk for 57 and we need to get on top of it. Cameron, can you dive in?
Flags: needinfo?(cam)
Priority: P2 → P1
Also, as a heads-up to anyone investigating this, I'm about to add some temporary diagnostic code [1] to nightly in order verify the hashmaps we use in the style system. The code as it stands will iterate the custom properties map after every insertion, which would cause quadratic behavior with a large number of properties. If the bottleneck is in custom properties, per comment 9, then this may or may not make things a lot slower on nightly for a few days until the diagnostics are removed. 

[1] https://github.com/servo/servo/pull/18779
I'll keep digging.
Assignee: nobody → cam
Status: NEW → ASSIGNED
Flags: needinfo?(cam)
Focusing just on one aspect of page slowness, when I move the mouse to the subscriptions list in the sidebar on the left (per comment 4), I get 474 ms of work (this is with STYLO_THREADS=1):

https://perfht.ml/2hWxhyI

* 12% under mouseover event handler, flushing frames for a clientHeight getter
* 54% in a subsequent style flush, after a custom property is changed
* 20% in a post-traversal for that style flush
* 11% painting

The cause of the style flush is the script setting a custom property on inline style on the <ytd-app> element.  That element basically contains the entire page.  The custom property that is changed (--ytd-app-scroll-offset) is referenced by rules that only the <ytd-app> matches, and the properties that it's referenced by are non-inherited properties.  We don't know that the changed custom property values will have no effect on any given element, so we resubstitute and recascade everything.

When producing the CustomPropertiesMap (which contains the computed values of all the custom properties for the element), we can't even just clone the parent's Arc<CustomPropertiesMap>, because every element in the document matches a rule that sets a bunch of custom properties:

  html:not([style-scope]):not(.style-scope):not([color-exp]) * {
    --yt-primary-color: hsl(0, 0%, 6.7%);
    /* ... 85 other custom properties ... */
  }

so we need to go through the "combine specified custom properties with inherited computed custom properties" path.  All except one of those specified custom properties, from that rule above, don't have var() references in their value.

The style flush work breaks down to:

* 39% in custom_properties::finish_cascade
* 26% in custom_properties::cascade
* 9% in Gecko_CalcStyleDifference, half of which is in Servo_ComputedValues_EqualCustomProperties
* the rest is mostly regular cascade work

The post-traversal work breaks down to:

* 61% dropping of old style contexts
* 22% under anonymous box style updating, a little under half of which is spent in Servo_ComputedValues_EqualCustomProperties while in CalcStyleDifference
So for this case I don't think we're doing anything stupid; we need to recascade all the elements since we do have new custom property values to propagate.

I have around 5,000 elements under the <ytd-app>, but there are only 25 unique sets of computed custom properties on them.  So I wonder if we could do some sort of TLS caching of Arc<CustomPropertiesMap> values, keyed off (specified custom properties, inherited custom properties)?
Looking a profile of a load of the whole subscriptions page, until it settles down, 7% of all time taken is due to Servo_StyleContext_Release.  Maybe we can defer some of that work, like IncrementalClearCOMRuleArray / DeferredFinalize?
One thing we could look at is to improve the raw performance of the custom properties code. I know some algorithms in there could maybe be optimized for the common case (no recursive variable references, no overriding of other properties...).
Yes, indeed.  Actually around 25% of all time taken during the page load is under a custom_properties::* function.
Here's my whole page load profile btw: https://perfht.ml/2hWT7SO
If only, we have a whole walk up over the computed properties map even when there are no references. That can surely be improved.

I'll build and submit a benchmark cascading some custom properties from a pre-existing map for now, and see how can we improve it.
(In reply to Emilio Cobos Álvarez [:emilio] from comment #31)
> I'll build and submit a benchmark cascading some custom properties from a
> pre-existing map for now, and see how can we improve it.

https://github.com/servo/servo/pull/18781
So, I've been looking at how Blink does this, and they're somewhat smarter than us. In particular, the observation is that the computed value generated by specified values that don't contain any variable references are effectively the same as computed values, so their "computed values map" is effectively just a copy of the specified values one when there are no variable references.

I'm not sure what would Simon think about the removing the split between custom_properties::SpecifiedValue and custom_properties::ComputedValue, but if we ditch it we can get to do all this much faster and with less memory traffic.
Another profile from a Nightly user:

Here's YT on Nightly. Same problems. https://perfht.ml/2ktR7SZ

https://www.reddit.com/r/firefox/comments/749lnx/firefox_56_is_handling_multiple_tabs_beautifully/do0ktbo/

And one on release: 

Okay, here's the Perf.io link for Youtube: https://perfht.ml/2kwhdVxI didn't disable any addons for this, either, but the delay is the same as when I run a clean, new profile sans addons, so I don't think they could be causing it.

https://www.reddit.com/r/firefox/comments/749lnx/firefox_56_is_handling_multiple_tabs_beautifully/dnzu2f8/
My investigations and the sudden influx of posts and corroborating comments on /r/firefox indicate that 56 users are also suddenly seeing poor performance which reinforces that this is not just a Stylo issue - and it seemed to just start this last week (with an update to the new layout I'm guessing).

Here is a 56 user that just posted a few minutes ago. They have 456 subscriptions 
https://www.reddit.com/r/firefox/comments/753dkr/firefox_loading_all_of_my_youtube_subscritions_it/

They write:

> Hey ! As the title says, Firefox always loads ALL of my subscriptions before enabling me to click on the " Subscribtion " button. So basically the front page loads, and I have to wait for a few minutes before actually being able to use the website.
> 
> Oh and also, if I click on the YT player, it doesn't pause/run the video instantly, it takes 5 to 10 seconds..

====
Here is another recent post (from yesterday) with more discussion and input.
https://www.reddit.com/r/firefox/comments/750yhy/youtube_facebook_extremely_slow_and_unresponsive/
(In reply to Caspy7 from comment #36)
> My investigations and the sudden influx of posts and corroborating comments
> on /r/firefox indicate that 56 users are also suddenly seeing poor
> performance which reinforces that this is not just a Stylo issue - and it
> seemed to just start this last week (with an update to the new layout I'm
> guessing).
> 
> Here is a 56 user that just posted a few minutes ago. They have 456
> subscriptions 
> https://www.reddit.com/r/firefox/comments/753dkr/
> firefox_loading_all_of_my_youtube_subscritions_it/
> 
> They write:
> 
> > Hey ! As the title says, Firefox always loads ALL of my subscriptions before enabling me to click on the " Subscribtion " button. So basically the front page loads, and I have to wait for a few minutes before actually being able to use the website.
> > 
> > Oh and also, if I click on the YT player, it doesn't pause/run the video instantly, it takes 5 to 10 seconds..
> 
> ====
> Here is another recent post (from yesterday) with more discussion and input.
> https://www.reddit.com/r/firefox/comments/750yhy/
> youtube_facebook_extremely_slow_and_unresponsive/

I am using Firefox v58 and setting the layout.css.servo.enabled to false actually reduces Cpu usage from 55% to under 20% which is a similar result Firefox v56. So I think the issue is caused by the Stylo.
(In reply to Emilio Cobos Álvarez [:emilio] from comment #34)
> So, I've been looking at how Blink does this, and they're somewhat smarter
> than us. In particular, the observation is that the computed value generated
> by specified values that don't contain any variable references are
> effectively the same as computed values, so their "computed values map" is
> effectively just a copy of the specified values one when there are no
> variable references.

FWIW I prototyped something like this and helped a lot in my micro-benchmark. I haven't run all the tests yet, and my day is over now, but I'd expect it to help a bit with this.

It lives right now in:

  https://github.com/emilio/servo/tree/faster-custom-props
(In reply to Caspy7 from comment #36)
> Here is another recent post (from yesterday) with more discussion and input.
> https://www.reddit.com/r/firefox/comments/750yhy/
> youtube_facebook_extremely_slow_and_unresponsive/

Does anybody have consistent STR (a profile too ideally) for the claimed FB slowdown? If so, mind filing another bug for it? (Feel free to cc / ni? me)

Thanks a lot for the feedback!
Flags: needinfo?(caspy77)
(In reply to Cameron McCormack (:heycam) from comment #25)
> * 61% dropping of old style contexts
> * 22% under anonymous box style updating, a little under half of which is
> spent in Servo_ComputedValues_EqualCustomProperties while in
> CalcStyleDifference

Other thing we can optimize here, would be to split style difference between:

 (1) "propagation" difference, that is, whether inherit / reset structs change.
 (2) "visual" difference, that is, changes which need an actual effect by layout.

For (1), we need to look to custom props, but not for (2), and (2) is actually what we care about there.
Is the idea that we could avoid calling Servo_ComputedValues_EqualCustomProperties when we're in a RestyleKind::CascadeOnly because we should be able to tell from the work we do in cascade() whether we have a different CustomPropertiesMap or not?  That makes sense.
I wonder how much it would help to have separate RestyleKind / RestyleHint values for cascading custom and non-custom properties separately.
(In reply to Cameron McCormack (:heycam) from comment #41)
> Is the idea that we could avoid calling
> Servo_ComputedValues_EqualCustomProperties when we're in a
> RestyleKind::CascadeOnly because we should be able to tell from the work we
> do in cascade() whether we have a different CustomPropertiesMap or not? 
> That makes sense.

Not quite, it's not clear to me how that would work, care to elaborate?

I'm saying that, in particular, for the anonymous box restyling stuff, we only care about actual change hints, and custom properties can't generate any.

(In reply to Cameron McCormack (:heycam) from comment #42)
> I wonder how much it would help to have separate RestyleKind / RestyleHint
> values for cascading custom and non-custom properties separately.

How would this work? You may need to cascade non-custom properties anyway, since they may contain references to the custom ones. Or are you thinking in optimizing the case of when custom properties don't change?
(In reply to Emilio Cobos Álvarez [:emilio] from comment #43)
> I'm saying that, in particular, for the anonymous box restyling stuff, we
> only care about actual change hints, and custom properties can't generate
> any.

Ah, I overlooked that you were referring to the anonymous box restyling specifically.

> (In reply to Cameron McCormack (:heycam) from comment #42)
> > I wonder how much it would help to have separate RestyleKind / RestyleHint
> > values for cascading custom and non-custom properties separately.
> 
> How would this work? You may need to cascade non-custom properties anyway,
> since they may contain references to the custom ones. Or are you thinking in
> optimizing the case of when custom properties don't change?

Yes, you would need to check whether any substitutions would be required.  The profile doesn't really show significant time in non-custom property cascading, though.  I was thinking more of the reverse case, were we could skip the custom property cascading.  But I'm not yet sure how much of the custom property cascading we do is due to to non-custom-property-only cascades.
(In reply to Cameron McCormack (:heycam) from comment #44)
> I was thinking more of the reverse case, were we could
> skip the custom property cascading.  But I'm not yet sure how much of the
> custom property cascading we do is due to to non-custom-property-only
> cascades.

I just tried this, and it doesn't help.
Tracking 57+ based on Comment 22.
https://github.com/servo/servo/pull/18791 and https://github.com/servo/servo/pull/18798 should help here a fair bit, according to my profiles.

I think the second one can actually be improved, but I'll wait for Xidorn's input in bug 1403839.
See Also: → 1403839
Btw, it's not clear to me we should track this as a proper regression, it seems per comment 36 this also happens in 56.
Been meaning to add that the user I referenced on 56 with 456 subscriptions 
https://www.reddit.com/r/firefox/comments/753dkr/firefox_loading_all_of_my_youtube_subscritions_it/
ended up removing all or most of their subscriptions [before I could get them to give me a perf profile] thereby resolving their issue.

I do not know if there are one or two issues here, but it seems like there's an issue with the subscription list for users with many subscriptions and I think it either started or got way worse with last Tuesday's update of Youtube's new layout.  (Either reverting to the old layout or removing one's subscriptions seems to resolve it.)
Flags: needinfo?(caspy77)
I'm experiencing these issues on Firefox 57.0b6, even with the new layout disabled, even when logged out. Hope this helps... Let me know if you want any more info, a performance report, whatever.
Depends on: 1407246
> I'm experiencing these issues on Firefox 57.0b6, even with the new layout disabled, even when logged out.

Could you file a new bug and post a link to a profile there?  See https://developer.mozilla.org/en-US/docs/Mozilla/Performance/Reporting_a_Performance_Problem for instructions on creating a profile.
I'm wondering now whether YouTube made a change on their end in the last few days to their new layout.  I no longer see the rule I mention in comment 25, which was the cause for the excessive custom properties cascade times, at least in the "hover over the sidebar" case.

I took some new profiles (my test YouTube account now has 150 subscriptions).  This is for hovering over the sidebar:

Current Beta 57: https://perfht.ml/2kFbAV3
Current Nightly 58 + patches: https://perfht.ml/2kDRJ8G

So there's not much difference now.  It's still ~150 ms of work, 50% of which is flushing styles right after script does its work of toggling --ytd-app-scroll-offset on the <ytd-app>.  Since we no longer have to re-cascade custom properties on all the descendants, having a separate cascade mode just for custom properties (unless we find regular properties that have var() values) might be helpful to eliminate the cascading and CalcStyleDifference time here.

And this is for the entire page load:

Current Beta 57: https://perfht.ml/2kGgYXS
Current Nightly 58 + patches: https://perfht.ml/2kCK8Hg

We still have the same pattern of requestAnimationFrame-driven lazy loading of the subscriptions list as before.  Each of these script blocks are 30-40 ms long, and seem to be mostly the instantiation of the <ytd-guide-entry-renderer> elements.  After each of these are small restyles and reflows.  The restyles are 6 ms styling 2 ms frame construction.  So on my machine, during this part of the page load, the page feels usable to me.

Then there are the eight or nine ~180 ms restyles towards the end of the page load.  When these are happening the jank is more obvious.  I'll look into what is triggering these restyles.
I looked at one of those large restyles, and it's due to re-selector matching the entire document.  This happens in response to a <yt-page-navigation> element being inserted into the document along with a <style> element for its styles.  The <style> contains:

  yt-page-navigation-progress { ... }
  yt-page-navigation-progress, #progress.yt-page-navigation-progress { ... }
  #progress.yt-page-navigation-progress { ... }

The invalidation machinery only handles invalidating elements in the document when the added rules have an ancestor combinator and have a class or ID in the sequence of simple selectors right before the ancestor combinator.  So it falls back by marking the document as fully invalid.

It's hard to know where a good trade-off lies when deciding how much matching to perform with the selectors on the newly added rules.  In this case, given there is only a single <yt-page-navigation> element in the document, it seems like recording an invalidation for yt-page-navigation-progress and #progress would be preferable to bailing out.
Depends on: 1407522
Attachment #8917271 - Attachment is obsolete: true
Attachment #8917271 - Flags: review?(emilio)
Cameron, should we wontfix this bug for Beta 57? Is the new YouTube website slower with Stylo than with the old Gecko style system?
Flags: needinfo?(cam)
Here's some profiles as of today:

Beta 57 stylo disabled: https://perf-html.io/public/523fe26c6a915a60a16e855ac7e05446b42dc780/calltree/?hiddenThreads=&range=3.3340_8.4317&thread=4&threadOrder=0-2-3-4-1
Beta 57 stylo enabled: https://perf-html.io/public/2d3d0535255c57f4c1ff34d52f45b5fbd7b6c1be/calltree/?hiddenThreads=&range=2.1368_9.0058&thread=2&threadOrder=0-2-1
Nightly stylo enabled: https://perf-html.io/public/dbed95315d25c9b2bed236cf1bfe65033be9db7f/calltree/?hiddenThreads=&range=3.0256_8.0924&thread=3&threadOrder=0-2-3-1
Beta 57 stylo enabled with bug 1407522 uplifted: https://perf-html.io/public/4e8b756e19287c07bf19abcd9a73cbf92fdeea3a/calltree/?hiddenThreads=&range=5.3267_9.8939&thread=2&threadOrder=0-2-1

The big difference is the chunks of styling work per comment 53 that happen towards the end of the page.  With stylo disabled, Gecko spends some time in these -- 60-70 ms each.  With stylo enabled on beta, these are around 180 ms each.  On Nightly, or with the bug 1407522 patch uplifted to Beta, they're reduced to 30-50 ms each.

I don't think the other patches that improved the custom properties cascade performance need to be uplifted now, but I think bug 1407522 should be.
Flags: needinfo?(cam)
I've requested uplift for that patch in bug 1407522.
Just tested latest nightly With YouTube, and honestly I don't see much difference between nightly 58 and Firefox 57 beta.

Loading(refreshing) a YouTube page still consumes double CPU usage than chrome and it's still laggy.

P.s. Tested without Subscriptions.
Flags: needinfo?(cam)
(In reply to the_goodone1 from comment #58)
> Just tested latest nightly With YouTube, and honestly I don't see much
> difference between nightly 58 and Firefox 57 beta.
> 
> Loading(refreshing) a YouTube page still consumes double CPU usage than
> chrome and it's still laggy.
> 
> P.s. Tested without Subscriptions.

This didn't include the fix, can you retest?
Flags: needinfo?(the_goodone1)
FWIW I saw other reports on Reddit that their YouTube performance issues were resolved, and specifically for the Subscriptions page my local testing saw it much improved after bug 1407522 landed.  If there are still issues with certain YouTube pages (since comment 58 mentions without subscriptions) then probably we can file a new bug for that.
Flags: needinfo?(cam)
(In reply to Cameron McCormack (:heycam) (away 24 Oct – 12 Nov) from comment #60)
> FWIW I saw other reports on Reddit that their YouTube performance issues
> were resolved, and specifically for the Subscriptions page my local testing
> saw it much improved after bug 1407522 landed.  If there are still issues
> with certain YouTube pages (since comment 58 mentions without subscriptions)
> then probably we can file a new bug for that.

Looks like we are good to resolve this bug after bug 1407522 landed.
Please file new bugs once you've found different cases.
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Flags: needinfo?(the_goodone1)
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: