Closed
Bug 1471961
Opened 6 years ago
Closed 6 years ago
tresize measurements can be thrown off by spurious paints
Categories
(Testing :: Talos, defect)
Tracking
(Not tracked)
RESOLVED
DUPLICATE
of bug 1496649
People
(Reporter: kats, Unassigned)
References
Details
The measuring code in tresize.js is intended to measure the time it takes to get the MozAfterPaint after resizing the window. The code does this 300 times, and then produces a score which is the average of the 300 intervals.
The problem is that the code seems to assume that the resizing is the only source of MozAfterPaint events. In fact, other things in the browser could change and trigger MozAfterPaint events, which can throw off the results. And actually the very first resize that the test does at [1] (which is before the timed resize) is one of those things - that resize will trigger a MozAfterPaint which can get picked up by the listener intended to listen for the first timed resize.
In practice the 20ms gap between timed resizes damps out this source of noise, because the spurious MozAfterPaints can fire in that 20ms gap without affecting the end results. However, in my local testing, I was definitely seeing a handful of cases where the recorded time interval was wrong because the code just waits for any old MozAfterPaint instead of the one that was triggered by the resize. Out of the 300 timed resizes, I don't think more than 20 or so would end up wrong, so on the whole this isn't a huge source of noise, but I'm filing this bug anyway because having this written down might save somebody else some time in the future.
[1] https://searchfox.org/mozilla-central/rev/d2966246905102b36ef5221b0e3cbccf7ea15a86/testing/talos/talos/startup_test/tresize/addon/content/tresize.js#71
Comment 1•6 years ago
|
||
thanks for filing this- maybe we need to look for specific sources instead of random MozAfterPaint- or possibly there is another event.
Do we have any way to easily determine/query what else generates mozAfterPaint events?
Reporter | ||
Comment 2•6 years ago
|
||
As far as I'm aware there's no "what generated this" type info on the MozAfterPaint. My thought on a possible fix here was to check via DOMWindowUtils.isMozAfterPaintPending if there is an inflight paint request, and defer the next timed resize until that paint is done. I don't know if tresize.js can access DOMWindowUtils though.
Reporter | ||
Updated•6 years ago
|
Comment 3•6 years ago
|
||
:aswan, this might be useful to consider looking at in relation to your changes to tresize.
Flags: needinfo?(aswan)
Comment 4•6 years ago
|
||
Ah, I didn't know of this bug but I think it will be addressed by https://phabricator.services.mozilla.com/D7968
I guess if that patch lands in more or less its current form, I can just dupe this to bug 1496649 at that time.
Flags: needinfo?(aswan)
Comment 5•6 years ago
|
||
kats, I think this patch addresses the issue described here:
https://hg.mozilla.org/mozilla-central/rev/8d9338c62294
If I've overlooked something, please re-open.
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → DUPLICATE
Reporter | ||
Comment 6•6 years ago
|
||
It's not really clear to me how the patch addresses the issue. As far as I can see you're still listening for MozAfterPaint events, and those are not necessarily a result of the resize actions. There could be, for example, changes in the layout that are timer-based and will trigger MozAfterPaint at random times, and those could still trigger your listeners.
However I don't have the time right now to set up the environment again to debug it and see if this is still a problem in practice.
Reporter | ||
Comment 7•6 years ago
|
||
Oh, never mind. I see now the transaction id checks are meant to guard against this. That seems reasonable, and should probably address my concern, yes. Sorry for the noise, and thanks for implementing this!
Comment 8•6 years ago
|
||
\o/
You need to log in
before you can comment on or make changes to this bug.
Description
•