1601333 - Browsertime conditioning sequence is very different and longer than Raptor web-ext

Status: RESOLVED FIXED

(Testing :: Performance, defect, P1)

Description

[Andrew Creskey [:acreskey]]

When running cold page load tests through Browsertime the initial conditioning process involves additional steps and delays.

Because we are trying to compare the two test frameworks, these steps should match as closely as possible.

This is also an explanation for the longer test times when running Browsertime (appears to explain all of the additional time).

Raptor web-ext cold load:

-launch browser
-wait 30 seconds (post_startup_delay)
-create new tab - about:blank
-load page

Browsertime:

-launch browser
-wait 5 seconds (foreground_delay)
-navigate to about:blank
-wait 5 seconds after load event
-wait 30 seconds (post_startup_delay)
-wait 1 second (page_cycle_delay) (raptor does this after the first test, from what I see)
-load page

This is a ~37% increase in conditioning time which will affect noise and also performance.

https://searchfox.org/mozilla-central/source/testing/raptor/browsertime/browsertime_pageload.js

Comment 1

[Andrew Creskey [:acreskey]]

To get an idea of the impact on test scores of this difference I made a push where raptor web-ext followed the same conditioning steps as Browsertime.
Compared against its parent commit, you can see that the test scores change significantly.
https://treeherder.mozilla.org/perf.html#/compare?originalProject=try&originalRevision=2bc4923e103b168406dc734d9498489c517af576&newProject=try&newRevision=8b83d651eeec4a14435ae531dabd52ce82ed8b09&framework=10

~8.7% improvement to raptor-tp6-bing-firefox-cold is the most impacted of the subset I ran this on.
Profiling now to try and find out exactly why.

Comment 2

[Tarek Ziadé (:tarek)]

wait 5 seconds (foreground_delay)

Can't we kill foreground_delay right way ? why do we need it ?

wait 5 seconds after load event"

where is that coming from ? if we can't remove it, can't we reduce post_startup_delay to 25 s in that case ?

wait 1 second (page_cycle_delay) (raptor does this after the first test, from what I see)

isn't just a matter of moving it at the end of the loop and not doing it at the last iteration?

It's easy to try out, it's just tweaks into https://searchfox.org/mozilla-central/source/testing/raptor/browsertime/browsertime_pageload.js

Happy to help

Comment 3

[Tarek Ziadé (:tarek)]

Attachment: Bug 1601333 - Browsertime conditioning sequence is very different and longer than Raptor web-ext

Comment 4

[Tarek Ziadé (:tarek)]

try push with a tweaked version of the BT script https://treeherder.mozilla.org/#/jobs?repo=try&revision=2b6f9a6c5aaed147933848bf9be9697f11a03dbd

Comment 5

[Andrew Creskey [:acreskey]]

(In reply to Tarek Ziadé (:tarek) from comment #2)

wait 5 seconds (foreground_delay)

Can't we kill foreground_delay right way ? why do we need it ?

Given that we're waiting for post_startup_delay, yes, I think we can remove it.

wait 5 seconds after load event"

where is that coming from ? if we can't remove it, can't we reduce post_startup_delay to 25 s in that case ?

This is part of browsertime's logic -- it waits after the load event because the page may not yet be visually complete. If we remove it from browsertime it will break visual metrics.

Since we're focusing on cold page loads at the moment, and there is no other page being loaded after, I don't think this is a problem to keep this.

wait 1 second (page_cycle_delay) (raptor does this after the first test, from what I see)

I was wrong about that -- Raptor does actually do the 1 second delay before loading each url.
https://searchfox.org/mozilla-central/rev/62a130ba0ac80f75175e4b65536290b52391f116/testing/raptor/webext/raptor/runner.js#466

isn't just a matter of moving it at the end of the loop and not doing it at the last iteration?

It's easy to try out, it's just tweaks into https://searchfox.org/mozilla-central/source/testing/raptor/browsertime/browsertime_pageload.js

Happy to help

Comment 6

[Tarek Ziadé (:tarek)]

Try run https://treeherder.mozilla.org/#/jobs?repo=try&revision=209fd84422d6079f5b2d1c4fa957374e58039b1c

Comment 7

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/00b43fdcfb5f
Browsertime conditioning sequence is very different and longer than Raptor web-ext r=acreskey

Comment 8

[Cosmin Sabou [:CosminS]]

https://hg.mozilla.org/mozilla-central/rev/00b43fdcfb5f