1812080 - 10.98 - 9.54% a11yr dhtml.html / a11yr dhtml.html + 1 more (Windows) regression on Fri January 20 2023

Status: RESOLVED FIXED

(Core :: XPCOM, defect)

Description

[Andrej (:aglavic)]

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

Regressions:

Ratio	Test	Platform	Options	Absolute values (old vs new)
11%	a11yr dhtml.html	windows10-64-shippable-qr	e10s fission stylo webrender-sw	826.51 -> 917.28
10%	a11yr dhtml.html	windows10-64-shippable-qr	e10s fission stylo webrender	831.18 -> 910.70
10%	a11yr dhtml.html	windows10-64-shippable-qr	e10s fission stylo webrender	834.99 -> 914.68

Improvements:

Ratio	Test	Platform	Options	Absolute values (old vs new)
3%	tp5o_scroll	windows10-64-shippable-qr	e10s fission stylo webrender-sw	1.74 -> 1.68
2%	tp5o_scroll	windows10-64-shippable-qr	e10s fission stylo webrender-sw	1.73 -> 1.69

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) may be backed out in accordance with our regression policy.

If you need the profiling jobs you can trigger them yourself from treeherder job view or ask a sheriff to do that for you.

For more information on performance sheriffing please see our FAQ.

Comment 1

[BugBot [:suhaib / :marco/ :calixte]]

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

Comment 2

[Justin Link]

Andrej: Could I get the profiling jobs that are referred to in the last comment? It would help me understand what's happening. Thanks!

Comment 3

[Justin Link]

I have begun taking a look at this regression.

Locally (on Windows) I am able to run the cited test and I don't see any change in performance with or without my patch stack, although I'm not sure if I'm running with the same build configuration.

Is there any way to know "windows10-64-shippable-qr" corresponds to in terms of mozconfig settings?

Comment 4

[Florian Quèze [:florian]]

I triggered profiling jobs, but the profiles they generated are useless because of bug 1753483.

Comment 5

[Andrej (:aglavic)]

I triggered the profiled jobs but for the task of yours but it looks like :florian did first. Thank you Florian
As per: https://wiki.mozilla.org/TestEngineering/Performance/Platforms I believe the test is run on the windows platforms are the HPE Moonshot, I am not sure if there are any specific moz-config settings, I will investigate and follow up with you
In the meanwhile I will bring up bug 1753483 with the team as a higher urgency issue to help resolve the regression sooner.

Comment 6

[Andrej (:aglavic)]

:jlink you are best following up with the build team about the mozconfig file

Comment 7

[Justin Link]

Quick update on my progress so far:

TimerThread::AddTimer() will wake up the Timer Thread if a timer is added that "has a delay of zero or is put into the 'first' entry in the timer list". (It seems to me that the first part of that logic should be covered by the second but that's a separate topic.)

There seems to be a significant difference (several orders of magnitude) in the rate at which that case is triggered with the new code vs the old.

I believe that both correctly implement the logic described above but that that logic is actually incorrect and should be "if the newly added timer is set to fire before the Timer Thread was scheduled wake up next".

With the old way of keeping track of timers, the implementation of those pieces of logic are actually equivalent. With the new way of storing timers they are not.

I will be pushing up a new change to try soon to test this.

Comment 8

[Justin Link]

Attachment: Bug 1812080 - Fixed Timer Thread wake-up logic when adding timers r=smaug,florian

Really we want to wake-up the Timer Thread now if the newly-added timer
wants to fire before the Timer Thread was scheduled to wake up. The
previous logic happened to work correctly under previous insertion
behavior isn't correct now.

Comment 9

[Justin Link]

Try run with the new change showing the performance back at its previous level: https://treeherder.mozilla.org/jobs?repo=try&revision=8a7cb4e1c6f62cfa4658634a0b428193f0dad3c3&selectedTaskRun=Ug4vPbwSR7KU5G6GY1hPFQ.0

Comment 10

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/0afb0ef7c18b
Fixed Timer Thread wake-up logic when adding timers r=smaug,florian

Comment 11

[Iulian Moraru]

https://hg.mozilla.org/mozilla-central/rev/0afb0ef7c18b