Closed Bug 1454053 Opened 6 years ago Closed 6 years ago

re-record tp6 pages for raptor

Categories

(Testing :: Raptor, enhancement)

Product:
Testing
Component:
Raptor
Type:
enhancement
Priority:
Not set
Severity:
normal

Tracking

(firefox63 fixed)

Status:
RESOLVED FIXED
Milestone:
mozilla63
Tracking Flags:
Tracking Status
firefox63 --- fixed

People

(Reporter: jmaher, Assigned: rwood)

References

(Blocks 1 open bug)

Details

(Whiteboard: [PI:June])

Attachments

(1 file, 3 obsolete files)

Bug 1454053 - re-record tp6 pages for raptor;
59 bytes, text/x-review-board-request
igoldan
: review+
Details 
our tp6 pages are out of date and youtube is bimodal on windows due to a large popup ad that shows either before/after our measurement.

we should record fresh pages for:
amazon.com - load amazon.com, click insearch box, search for "star trek", click search button, click on first item, scroll down page
google.com - load google.com, click in search box, type "star trek", click search
youtube.com - load youtube.com, click in top search box, type "star trek", click search, click first link, scroll to bottom of page
facebook.com - we should load this with data and login- I don't have a facebook account, so I am not sure if I can spell this out.

with the work in bug 1436827, we should also include the large google doc- at least for getting some data on tp6.


I know we would ideally like to update tp5 to make it more valid (bug 1444212), but this seems a bit more of an urgent matter and something that will serve as a way to document what needs to be done in order to update a larger pageset.
:tarek, I know you have been able to record a page from your work in bug 1436827, could you help out with getting some page recordings for the 4 other pages?

:rwood, can you help collect these into a proper bundle/format so that we can get them on tooltool (ideally to not replace, but exist in parallel to original pageset) and run from automation.
Flags: needinfo?(tarek)
Flags: needinfo?(rwood)
Ok. We want the pagesets to exist in parallel but we only want the new pages actually running in automation right (but we'll keep the old pageset around in case we revert)?

We use a special Facebook test account, I'll see if I can find the credentials in my old emails and if so I'll fw it on.
Flags: needinfo?(rwood)
(In reply to Joel Maher ( :jmaher) (UTC-5) from comment #0)
> our tp6 pages are out of date and youtube is bimodal on windows due to a
> large popup ad that shows either before/after our measurement.

Further investigations late Friday suggest that youtube is bimodal even with the ad removed. I'm investigating more, but at the moment there's no particular urgency on my end to re-record the pages. There's a chance that re-recording youtube might make it less bimodal, but it could also make it worse or make some of the other tests bimodal.

That said, if the pages are out of date it could be worth generating new recordings at some point and exploring what the numbers look like. I do think that running both sets in parallel for a little while probably makes sense.
:bholley, thanks for the update.

ok, lets make sure this is on track to be done in Q2 (ideally this month), but lets not drop other work for this to rush this week or next week.
Sure I can record them this week (notice that with my french IP I might get specific content in some of them)
Flags: needinfo?(tarek)
I think that would be fine- if this is as simple as the last recording, then I need to figure out how to get recordings to work on windows, or do work in a linux VM
Whiteboard: [PI:April] → [PI:May]
Hi Tarek, is there an update on the new recordings perhaps? I don't think it matters re: specific content with your french IP. If there are new recordings (uploaded to tooltool) I will implement them in the new Raptor tp6. Thanks!
Flags: needinfo?(tarek)
Whiteboard: [PI:May] → [PI:June]
My understanding was that we wanted to make sure anyone could record mitmproxy without configuration problems.
Since Joel had issues, I built a tool to make sure he and anyone else could use mitmproxy 

This repo : https://github.com/tarekziade/mitm-firefox contains a docker image you can use to record any mitmproxy dump 
without having to configure/install anything

It runs a firefox inside a docker, you can reach from your browser via a vnc server, and let you
trust a CA certificate for that session. Once you've finished, it spits a dump file you can later reuse in mitmproxy

For tooltool, do you mean adding a bug with the produced dump so it's manually added there ? or something else ?
I think that part could be automated too, by having a Task Cluster artifact updated when you change the dump.
Like what was done for the heavy profile scenario.
Flags: needinfo?(tarek)
This new mitmproxy pageset will include refreshes of the existing tp6 pages, as well as new pages that will be introduced for Bug 1436827 - and will be implemented on raptor only (as eventually we will be turning off talos-tp6 in favour of raptor-tp6).
Assignee: nobody → rwood
Status: NEW → ASSIGNED
Component: Talos → Raptor
Note: Use Tarek's code here, to insert the hero element into the mimproxy recording:

https://github.com/tarekziade/mitmflow
Update: The mitmproxy recordings for the google suite (doc/slide/sheet) is quite large - almost 30 MB just for those alone - so I'm going to have separate archives for the tp6 pageset and gdocs pageset.
Blocks: 1449180
Summary: re-record tp6 pages → re-record tp6 pages for raptor
Depends on: 1477109
Note: New mitmproxy recordings for the google docs/sheets/slides tests will be done and added as part of Bug 1436827
I've updated the mitmproxy tp6 pagesets (our four main sites). I also added one hero element to each page recording.

amazon search: - added a hero element to the first laptop displayed in the search
facebook - added a hero element to the 'Home' icon at the top
google search - added a hero element to the first bigger photo of Obama that appears on the right
youtube - added a hero element to the main youtube logo on the top left

Once they're uploaded on tooltool (Bug 1477109) I'll finish this patch to update Raptor to use the new pagesets in production.
Also note, I've saved the html source of the new pagesets (as served out by mitmproxy during the tests) to our new perf-automation github pageset repo [1]. In case the bare html is needed for debugging (outside of mitmproxy/raptor).

[1] https://github.com/mozilla/perf-automation/tree/master/pagesets/mitmproxy/raptor-tp6
No longer depends on: 1436827
Attachment #8993052 - Attachment is obsolete: true
Attachment #8994202 - Attachment is obsolete: true
Attachment #8994202 - Flags: review?(igoldan)
(In reply to Robert Wood [:rwood] from comment #17)
> Also note, I've saved the html source of the new pagesets (as served out by
> mitmproxy during the tests) to our new perf-automation github pageset repo
> [1]. In case the bare html is needed for debugging (outside of
> mitmproxy/raptor).
> 
> [1]
> https://github.com/mozilla/perf-automation/tree/master/pagesets/mitmproxy/
> raptor-tp6

These snapshots are extremely useful - thank you for keeping them up to date!
(In reply to Bobby Holley (:bholley) from comment #24)

> These snapshots are extremely useful - thank you for keeping them up to date!

Thanks Bobby, glad they are helping out. FYI I am adding more, for google docs/slides/sheets, in Bug 1436827
Also fixing a few bugs in this same patch since the try run in Comment 22 was red:

- for tp6 google, landing on the first page [1] actually redirects to the page + '&cad=h'. So what was happening was the measure.js content was loading in both of those URLs - which was causing fnbpaint readings to be taken for each URL. Just noticed this now - if you look at historic tp6 google raptor tests on try you'll see there are more fnbpaint replicates than there are pagecycles. Fixed by changing the test URL to the final one, and measure.js is only loaded in that one page.

- when processing final output there was a bug where we were calling construct_results for 'pageload' types of tests; and it was just sending in the last test (i.e. tp6-youtube) only. And since I added a hero element to each page, it wasn't expecting hero values and fnbpaint values in construct_results. We don't need to call that anyway for non-pageload type of tests because we want Raptor to report on every test and subtest individually for now. For example, we want raptor tp6 google hero element and raptor tp6 google fnbpaint to have their own reported values, not a single mean for those two. Fixed by only calling construct_results for benchmark type tests.

- for hero element measurements, now rounding the result to nearest int - no more 145.000000012 for example

[1] https://www.google.com/search?hl=en&q=barack+obama
Blocks: 1436827
Comment on attachment 8994214 [details]
Bug 1454053 - re-record tp6 pages for raptor;

https://reviewboard.mozilla.org/r/258826/#review265992

::: testing/raptor/raptor/output.py:100
(Diff revision 4)
> -                LOG.error("output.summarize received unsupported test results type")
> -                return
>  
> -        # if there is more than one subtest, calculate a summary result
> +                # if there is more than one subtest, calculate a summary result
> -        if len(subtests) > 1:
> +                if len(subtests) > 1:
> -            suite['value'] = self.construct_results(vals, testname=test.name)
> +                    suite['value'] = self.construct_results(vals, testname=test.name)

Could we rename construct_results to construct_summary? Current identifier seems generic.  And we could then remove the upper comment.
Comment on attachment 8994214 [details]
Bug 1454053 - re-record tp6 pages for raptor;

https://reviewboard.mozilla.org/r/258826/#review265992

Notice that the equivalent tp6 Chrome tests are failing. So these must be addressed.

Now that this had happened, I also saw from the error logs some misues of the self.info functions. For example, in testing/mozaharness/mozharness/mozilla/testing/raptor.py::Raptor.info, the function is expecting string values, not exception objects. You can see the places that need fixes here https://dxr.mozilla.org/mozilla-central/search?q=path%3Atesting%2Fraptor+%27self.info%27&redirect=false
Please address these also, so that errors are catched as expected.
Comment on attachment 8994214 [details]
Bug 1454053 - re-record tp6 pages for raptor;

https://reviewboard.mozilla.org/r/258826/#review266014
Attachment #8994214 - Flags: review?(igoldan) → review-
(In reply to Ionuț Goldan [:igoldan], Performance Sheriffing from comment #32)
> Comment on attachment 8994214 [details]
> Bug 1454053 - re-record tp6 pages for raptor;
> 
> https://reviewboard.mozilla.org/r/258826/#review265992
> 
> Notice that the equivalent tp6 Chrome tests are failing. So these must be
> addressed.
> 
> Now that this had happened, I also saw from the error logs some misues of
> the self.info functions. For example, in
> testing/mozaharness/mozharness/mozilla/testing/raptor.py::Raptor.info, the
> function is expecting string values, not exception objects. You can see the
> places that need fixes here
> https://dxr.mozilla.org/mozilla-central/
> search?q=path%3Atesting%2Fraptor+%27self.info%27&redirect=false
> Please address these also, so that errors are catched as expected.

TP6 is not supported on Chrome. It is enabled on try so we can work on it in the future, but it is not supported in production yet, so if I could get a re-review please, thanks :)
Comment on attachment 8994214 [details]
Bug 1454053 - re-record tp6 pages for raptor;

Thanks sure I'll rename construct_results to construct_summary
Attachment #8994214 - Flags: review- → review?
I also updated the info exception statements, thanks!
Comment on attachment 8994214 [details]
Bug 1454053 - re-record tp6 pages for raptor;

https://reviewboard.mozilla.org/r/258826/#review266060

Awesome, thanks!
Attachment #8994214 - Flags: review+
:rwood sorry for the tp6 on Chrome confusion.
(In reply to Ionuț Goldan [:igoldan], Performance Sheriffing from comment #39)
> :rwood sorry for the tp6 on Chrome confusion.

No need - it's good you checked that anyway, thank you! MozReview is giving me an internal error at the moment when I try to publish my updates, but I'll try again later and get those updates in before landing. Thanks :)
Attachment #8994214 - Attachment is obsolete: true
Attachment #8994214 - Flags: review?
Comment on attachment 8994495 [details]
Bug 1454053 - re-record tp6 pages for raptor;

https://reviewboard.mozilla.org/r/259052/#review266074
Attachment #8994495 - Flags: review?(igoldan) → review+
If I run all the tests in this configuration (on the new pages) with both fnbpaint and hero elements being measured:

raptor-firefox-tp6-amazon raptor-firefox-tp6-amazon-fnbpaint opt: 906
raptor-firefox-tp6-amazon raptor-firefox-tp6-amazon-hero:hero1 opt: 2645.5
raptor-firefox-tp6-facebook raptor-firefox-tp6-facebook-fnbpaint opt: 1037
raptor-firefox-tp6-facebook raptor-firefox-tp6-facebook-hero:hero1 opt: 2545
raptor-firefox-tp6-google raptor-firefox-tp6-google-fnbpaint opt: 276
raptor-firefox-tp6-google raptor-firefox-tp6-google-hero:hero1 opt: 645.5
raptor-firefox-tp6-youtube raptor-firefox-tp6-youtube-fnbpaint opt: 500
raptor-firefox-tp6-youtube raptor-firefox-tp6-youtube-hero:hero1 opt: 2551.5

That's just one platform. That's too many data points. For now I'm going to land the new pagesets but only turn on fnbpaint for each page so there will just be 4 data points. I've filed Bug 1478057 to add work to Raptor so that we can measure the multiple measurements (i.e. fnbpaint and hero) in each page but just report one summary / median value i.e. raptor-firefox-tp6-google instead of reporting each measurement separately.
Pushed by rwood@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/70d57bb33a7f
re-record tp6 pages for raptor; r=igoldan
https://hg.mozilla.org/mozilla-central/rev/70d57bb33a7f
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
status-firefox63: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla63
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: