1607891 - Add fennec (68) support to raptor-browsertime

Status: RESOLVED FIXED

(Testing :: Performance, enhancement, P1)

Description

[Robert Wood [:rwood]]

Ideally we want raptor-browsertime also running on fennec so we can compare fenix vs fennec (68). We currently have browsertime (via mach raptor) running with GVE and Fenix but not fennec.

Comment 1

[Robert Wood [:rwood]]

https://treeherder.mozilla.org/#/jobs?repo=try&tier=1%2C2%2C3&revision=673f9fdf58a93b0c88b13c9ae6ed183fdd16b513&selectedJob=284074316

A couple of issues to figure out:

[task 2020-01-08T20:44:25.881Z] 20:44:22 INFO - results.MissingResultsError: Browsertime cycle missing timeToContentfulPaint measurement

Is that expected for fennec68?

Also webdriver is unable to shutdown the fennec app and fails with:

[task 2020-01-08T20:44:25.881Z] 20:44:21 INFO - [2020-01-08 20:44:21] INFO: [browsertime] Browsertime pageload ended. [task 2020-01-08T20:44:25.881Z] 20:44:21 INFO - [2020-01-08 20:44:21] DEBUG: [browsertime] Telling browser to quit. [task 2020-01-08T20:44:25.881Z] 20:44:21 INFO - 1578516261906 webdriver::server DEBUG -> DELETE /session/585db3bf-20d5-4fdb-b283-aed727f231fc [task 2020-01-08T20:44:25.881Z] 20:44:21 INFO - 1578516261908 webdriver::server DEBUG <- 500 Internal Server Error {"value":{"error":"unsupported operation","message":"Only supported in Firefox","stacktrace":"WebDriverError@chrome://marionette/content/error.js:175:5\nUnsupportedOperationError@chrome://marionette/content/error.js:493:5\nassert.that/<@chrome://marionette/content/assert.js:428:13\nassert.firefox@chrome://marionette/content/assert.js:97:57\nGeckoDriver.prototype.quit@chrome://marionette/content/driver.js:3459:10\ndespatch@chrome://marionette/content/server.js:305:40\nexecute@chrome://marionette/content/server.js:275:16\nonPacket/<@chrome://marionette/content/server.js:248:20\nonPacket@chrome://marionette/content/server.js:249:9\n_onJSONObjectReady/<@chrome://marionette/content/transport.js:503:20\n"}}

Comment 2

[Robert Wood [:rwood]]

[task 2020-01-08T20:44:25.881Z] 20:44:22 INFO - results.MissingResultsError: Browsertime cycle missing timeToContentfulPaint measurement

For raptor-webext on fennec68 we don't measure fcp (equivalent of 'timeToContentfulPaint' in browsertime) either so looks like this is expected.

Note that Tarek has recently added support for the 'measure' test ini setting for raptor-browsertime in Bug 1592746.

So to get around this in browsertime there's a few solutions:

• use a separate browsertime-tp6m.ini just for fennec68 (and add the 'measure =' line and don't include fcp; but that's not ideal);
• for now inside raptor just manually check for fennec and if running on that, don't expect 'firstContentfulPaint' in the btime results; also not ideal
• change the 'measure =' test ini setting so it can be app specific; i.e. something like: measure = { 'fenix': 'fnbpaint, fcp, dcf, loadtime', 'fennec': 'fnbpaint, dcf, loadtime' }

I would opt for the third option (make the 'measure' line app specific). Tarek what do you think?

Comment 3

[Tarek Ziadé (:tarek)]

Also webdriver is unable to shutdown the fennec app and fails with

yes this is a known limitation. you need to omit that session delete call there and just let adb kill Firefox once the business isdone.

I would opt for the third option (make the 'measure' line app specific). Tarek what do you think?

I am curious about one thing: when do we need to tweak the measure list? sounds like exceptions where we want to skip one or two metrics?
maybe we could have a default list for metrics per app/platform and let people add a skip-measure field?
if it's the case, that could greatly reduce the need to maintain the field in all inis.

Comment 4

[Robert Wood [:rwood]]

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

Also webdriver is unable to shutdown the fennec app and fails with

yes this is a known limitation. you need to omit that session delete call there and just let adb kill Firefox once the business isdone.

I would opt for the third option (make the 'measure' line app specific). Tarek what do you think?

I am curious about one thing: when do we need to tweak the measure list? sounds like exceptions where we want to skip one or two metrics?
maybe we could have a default list for metrics per app/platform and let people add a skip-measure field?
if it's the case, that could greatly reduce the need to maintain the field in all inis.

When I think on this more, TBH I don't think we even need that ability - to opt out of a specific measurement type - on a per test basis at all. I think it just adds extra complexity when it comes to checking results, knowing what is being measured where, data on the perfherder graphs, etc. I like your idea of having a list of default measurement types per browser/app, so we can skip 'timeToContenftulPaint' on fennec, but I think having that list internal to raptor would be fine. In the existing raptor-webext most tests measure the same thing (except fcp on fennec, and chrome measurements). I think we only changed individual test measurement options if a particular site had a problem with a specific measurement; in my opinion then we shouldn't add that site until that issue is resolved. What do you guys think?

Comment 5

[Robert Wood [:rwood]]

I brought this up at our daily standup, and we are in agreement that there's no need to have a 'measure' key at the test INI level; and we will just have raptor internally check what measurements are expected in the browsertime results, based on browser/app.

Comment 6

[Robert Wood [:rwood]]

https://treeherder.mozilla.org/#/jobs?repo=try&tier=1%2C2%2C3&revision=f4868ae91e84d349e33eea2458b1f4fb13d3406e

Comment 7

[Robert Wood [:rwood]]

(In reply to Robert Wood [:rwood] from comment #6)

https://treeherder.mozilla.org/#/jobs?repo=try&tier=1%2C2%2C3&revision=f4868ae91e84d349e33eea2458b1f4fb13d3406e

Green fennec browsertime cold page-load job ^. The visual metrics failure will be looked at / addressed in Bug 1604527, this bug will just be used for browsertime fennec page-load support.

I confirmed locally that the webdriver error on fennec shutdown only happens after the final browser cycle; the app is being shutdown between browser cycles and only left running after the last browser cycle is finished, which IMO is not an issue because if the app is still running and the test is run again, the app is shutdown and restarted as part as the new test run.

Comment 8

[Robert Wood [:rwood]]

Attachment: Bug 1607891 - Add fennec (68) support to raptor-browsertime

Comment 9

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/411561688e91
Add fennec (68) support to raptor-browsertime r=tarek

Comment 10

[Andreea Pavel [:apavel]]

https://hg.mozilla.org/mozilla-central/rev/411561688e91