Perfherder has detected a raptor performance regression from push e4c8bc7feaa60470c026236a7d89881a74b29d34. As author of one of the patches included in that push, we need your help to address this regression.

Regressions:

Improvements:

Details of the alert can be found in the alert summary, including links to graphs and comparisons for each of the affected tests. Please follow our guide to handling regression bugs and let us know your plans within 3 business days, or the offending patch(es) will be backed out in accordance with our regression policy.

For more information on performance sheriffing please see our FAQ.

Hej, I do have a theory what's going on with the regressions (have been looking at bug 1680743), just figuring out how to reproduce it best locally. Will come up with a try push today.

Here's a try push for the raptor-tp6-twitch tests: https://treeherder.mozilla.org/jobs?repo=try&revision=b1e538316828fcc40aa362ee60493ed0a1112b54

Edit: previous build had a typo, new try push: https://treeherder.mozilla.org/jobs?repo=try&revision=1aae799a53762b7ebae9c1bc85b26a2f216eead6

Set release status flags based on info from the regressing bug 1645528

This reinstates the behaviour from before D93173. It arguably vialotas
assumptions in other parts parts of the code, for example sActiveVsyncTimers
may have a wrong value, but we may have parts of the code that rely
on this behaviour (in contrast to properly calling StartTimer()).

Also alerts from browsertime:
== Change summary for alert #28004 (as of Mon, 07 Dec 2020 13:12:25 GMT) ==

Regressions:

Improvements:

For up to date results, see: https://treeherder.mozilla.org/perfherder/alerts?id=28004

I was just able to reproduce the biggest regression locally, i.e.

381% 	outlook 	loadtime 	linux64-shippable 	cold nocondprof 	1,270.29 -> 6,106.17

assuming ./mach raptor -t raptor-tp6-outlook-firefox --enable-webrender --cold is the correct command (I forgot --no-conditioned-profile, but I guess it doesn't make a bit difference here).

What strikes me is that the visual behaviour appears to be quite different. Florin, do you know who would be a good person to talk to about the test? I'm currently trying to understand what exactly we are actually measuring, as the new slower behaviour arguably looks more "correct".

Will attach some videos.

FTR., here are the numbers I get for the outlook test

current tip:
[5336, 5362, 5439, 5487, 5300, 7960, 1590, 1599, 7951, 5473, 5622, 5382, 5471, 8011, 1676, 5819, 5252, 5659, 5418, 5111, 5220, 5416, 1685, 5444, 5237]
pre 1645528:
[1722, 1723, 1664, 1610, 1570, 1640, 2595, 1575, 1643, 1709, 1749, 1621, 1610, 1781, 1638, 1670, 1588, 1752, 1788, 1568, 1617, 1639, 1710, 1661, 1610]

What kinda jumps to attention is that the test sometimes still finishes around the 1600 mark, as it did previously. But usually it will take a while longer and load more things. I wonder if this is another case of flakiness, but with a strong tendency in one direction. And if the new results are maybe the "real" or desired ones.

This is (in "current") a bimodal distribution. This isn't all that unusual; for example if a timer fires, and it's before onload, and it loads some large resource, that will block onload. If it fires after onload, it obviously doesn't. If the timer fires somewhere near onload, it could fall on either side due to noise.

This overall set of regressions is quite large....

:Sparky maybe you can help with some context on when these metric should fire exactly

also here are some information on the mtrics we gather:
https://wiki.mozilla.org/Performance/Glossary#Pageload
https://www.sitespeed.io/documentation/sitespeed.io/metrics/#pageloadtime

(In reply to Robert Mader [:rmader] from comment #6)

I was just able to reproduce the biggest regression locally, i.e.

381% 	outlook 	loadtime 	linux64-shippable 	cold nocondprof 	1,270.29 -> 6,106.17

assuming ./mach raptor -t raptor-tp6-outlook-firefox --enable-webrender --cold is the correct command (I forgot --no-conditioned-profile, but I guess it doesn't make a bit difference here).

What strikes me is that the visual behaviour appears to be quite different. Florin, do you know who would be a good person to talk to about the test? I'm currently trying to understand what exactly we are actually measuring, as the new slower behaviour arguably looks more "correct".

Will attach some videos.

you are running raptor webextension tests not browsertime (platform on which we logged the regression)
to run the test in browsertime you just need to add --brosertime in the command line

so the command is:
./mach raptor -t raptor-tp6-outlook-firefox --enable-webrender --cold --browsertime

Thanks, that was an important hint. With --browsertime the case is a little bit different: now both cases seem to visually behave pretty much equally - however, the reported loadtime still heavily diverges (roughly 1.6 vs 6 seconds, see videos).

I also checked some of the other heavily impacted test, especially twitch, and IIUC they all load elements via JS.
To me, I'm increasingly tending to the following conclusion:

1. the major regression appear to be all for loadtime, on sites where loadtime may not be a meaningful metric. They all rely on AJAX, which probably means the sentence "Important: this is only relevant to some pages, depending on how you page is built." from https://www.sitespeed.io/documentation/sitespeed.io/metrics/#pageloadtime applies to them.
2. the rest of regressions/improvements appears to equal each other out, more or less

Does that sound valid? :/

Olli, does the above sound reasonable to you?

That sounds reasonable, but it doesn't mean we should take major page load regressions.
Page load is after all the time when browser UI shows that the page has been loaded.

At least we should understand what is causing these changes in page load.
Given that there is a merge next week, should the regressing patch(es) be backed out for now?

Comment on attachment 9192261 [details]
browsertime-outlook-old.webm

The NEW behaviour (sorry wrong file name)

For the record: the previous behaviour can be restored by using a software timer instead of the vsyncchild. I'm currently trying to figure out what exactly behaves different between the two, with some theories I'm trying to verify.

It'd be good to know this week if we should back out bug 1645528 for 85.

(In reply to Julien Cristau [:jcristau] from comment #20)

It'd be good to know this week if we should back out bug 1645528 for 85.

Yes, please back out, at least for beta. I'm still investigating the issue so you could consider leaving it in for nightly - if that's possible (see also bug 1645528 comment 68).

Regressing patch backed out of 85:
https://hg.mozilla.org/releases/mozilla-beta/rev/01efe9d5b4e1006d7908576c8540139b633294df

This fixes a bunch of regressions:

• a wrong calculation in GetIdleDeadlineHint(), leading to pageload
  regressions.
• in certain situations we'd use StartupRefreshDriverTimer instead
  of VsyncRefreshDriverTimer when initializing timers early
• unnecessary use of BrowserChild on backends that don't opt for
  per-browser-child vsync - i.e. all but Wayland.

This is partly done by reverting to pre-1645528 behaviour, although
with some code simplifications.

FTR: I also played with some more radical changes, but given the
complexity of the code involved I found the regression potential too
big. Thus this is the most conservative solution I could come up with.

Try job: https://treeherder.mozilla.org/jobs?repo=try&revision=04be8d989a48dbc8342cd84ffc39e56629be18e8

Also checks for the regressions in bug 1680743 and bug 1673139

The try run looks good from what I can see - e.g. outlook loadtime, the biggest regression here, is back to 1287.5ms (https://treeherder.mozilla.org/perfherder/graphs?highlightAlerts=1&highlightChangelogData=1&series=try,2912794,1,13&timerange=1209600), twitch at 1035.5ms.

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

Backed out for causing crashes for Linux users:

https://hg.mozilla.org/integration/autoland/rev/f237c5e2a071dcaae542a34afc3c463099d5b1df

Crash example: bp-6d7050f8-c862-4a35-b396-179470201228

Err yes, a fix for that crashes is already landing in bug 1684352 - so, right, you can simply re-land, as just happened :)

https://hg.mozilla.org/mozilla-central/rev/59a0ddad5c28

== Change summary for alert #28291 (as of Wed, 30 Dec 2020 00:38:48 GMT) ==

Improvements:

For up to date results, see: https://treeherder.mozilla.org/perfherder/alerts?id=28291

== Change summary for alert #28307 (as of Thu, 31 Dec 2020 08:38:26 GMT) ==

Regressions:

Improvements:

For up to date results, see: https://treeherder.mozilla.org/perfherder/alerts?id=28307

== Change summary for alert #28312 (as of Thu, 31 Dec 2020 13:03:19 GMT) ==

Improvements:

For up to date results, see: https://treeherder.mozilla.org/perfherder/alerts?id=28312

== Change summary for alert #28292 (as of Wed, 30 Dec 2020 00:54:24 GMT) ==

Regressions:

Improvements:

For up to date results, see: https://treeherder.mozilla.org/perfherder/alerts?id=28292