Closed Bug 1363438 Opened 2 years ago Closed 2 years ago
15% regression in several component speedometer tests around April 24
[Tracking Requested - why for this release]: Examples of tests that show this regression. Angular2-TypeScript-TodoMVC-Adding100Items-sync - https://arewefastyet.com/#machine=17&view=single&suite=speedometer-misc&subtest=Angular2-TypeScript-TodoMVC-Adding100Items-sync AngularJS-TodoMVC-Adding100Items-async - https://arewefastyet.com/#machine=17&view=single&suite=speedometer-misc&subtest=AngularJS-TodoMVC-Adding100Items-async AngularJS-TodoMVC-CompletingAllItems-async - https://arewefastyet.com/#machine=17&view=single&suite=speedometer-misc&subtest=AngularJS-TodoMVC-CompletingAllItems-async Elm-TodoMVC-Adding100Items-sync - https://arewefastyet.com/#machine=17&view=single&suite=speedometer-misc&subtest=Elm-TodoMVC-Adding100Items-sync AWFY suggests a regression range of https://hg.mozilla.org/integration/mozilla-inbound/log?rev=6efe7915f97b389c51b85bb2e1e45004f33ca680%3A%3A0f97f76c0b34673f2bf31c96eb1285416836f565%20and%20%216efe7915f97b389c51b85bb2e1e45004f33ca680 There are two core changes that seem possible that they introduced the regression. They are Bug 1359415 - move threadsafety checks inside nsAutoOwningThread and Bug 1358275 - Preliminary cleanup: refactor nsTextFragment::UpdateBidiFlag()
Once we get the regressing bug we will know. It is unlikely that 54 is affected but I wanted to be cautious.
Certainly possible; we moved threadsafety checks for AddRef/Release out-of-line. 54 can't be affected, because none of the changesets landed in 54.
It's possible the change to nsTextFragment::UpdateBidiFlag in bug 1358275 could cause a regression, given that it took the loop out-of-line, though I didn't think this was such a hot codepath that it would matter. Can we get a profile of a regressing test run, to see if either this or the thread-safety stuff from bug 1359415 shows up at significant levels?
I will email you a link to the tests that you can profile, Jonathan.
Filtering for stacks including "nsTextFragment" on a profile from running the full set of speedometer tests shows me a total of only 4ms (and half of that is in the destructor), so it doesn't look like the nsTextFragment::UpdateBidiFlag change can be behind this.
The overall numbers for the Speedometer test do not show the regression. Only some tests do, for example speedometer-misc-Elm-TodoMVC-Adding100Items-sync shows the regression clearly. https://arewefastyet.com/#machine=17&view=single&suite=speedometer-misc&subtest=Elm-TodoMVC-Adding100Items-sync Do we have some documentation about running individual speedometer tests in a similar manner to AWFY?
So I found https://arewefastyet.com/schedule.php which when logged in allows you to trigger AWFY runs based on builds from our source trees or try. I am going to update my inbound tree, push to try for builds and see if this works as expected.
Kevin, Benjamin Bouvier has taken over the maintenance of AWFY, CCing him here in case you have questions from him. :-)
Kevin, did you get a chance to bisect and find the offending bug here?
I've not yet. I'll see what I can do on Tuesday when I get back or ask for help. I have a set of steps that should be able to find the regression. Try -> AWFS schedule -> wait for results, might try pushing several changesets as there seems to be a few hour lag on AWFY scheduled tests.
Setting fix-optional for 55 since we're going to beta soon and there's no assignee.
Kevin, did you finish your bisection attempts here?
I was under the impression that we cannot ship with any regressions wrt Quantum release criteria, leading up to 57. Is that an incorrect assumption? Tagged as blocking 55 unless someone strongly disagrees.
I may be misreading the AWFY charts, but I see a performance improvement, not a regression, on April 24 in the AWFY graphs linked from comment 0. We had a lot of AWFY configuration changes for Speedometer V2 around that time. Given that those measurements are now two months old and we've made big performance improvements  lately, can we just resolve this bug as WFM?  https://arewefastyet.com/#machine=36&view=single&suite=speedometer-misc&subtest=score
Status: NEW → RESOLVED
Closed: 2 years ago
Resolution: --- → WORKSFORME
Whiteboard: [qf:p1] [monitoring] → [qf:p1] [pi-monitoring]
You need to log in before you can comment on or make changes to this bug.