Closed Bug 1384272 Opened 8 years ago Closed 7 years ago

Add a talos test that tracks the performance of opening the preferences

Categories

(Testing :: Talos, enhancement)

Product:
Testing
Component:
Talos
Version:
Version 3
Type:
enhancement
Priority:
Not set
Severity:
normal

Tracking

(firefox60 fixed)

Status:
RESOLVED FIXED
Milestone:
mozilla60
Tracking Flags:
Tracking Status
firefox60 --- fixed

People

(Reporter: jaws, Assigned: rwood)

References

Details

(Whiteboard: [PI:February])

Attachments

(1 file)

Bug 1384272 - Add a talos test that tracks the performance of opening about:preferences;
59 bytes, text/x-review-board-request
jmaher
: review+
Details
This test should have the following subtests: Open about:preferences#general (default view) Open about:preferences#search Open about:preferences#privacy Open about:preferences#sync Open about:preferences#privacy-reports (exercises some of the search code) Open about:preferences, clicks in the search field, and searches for "password" Open about:preferences, clicks in the search field, and searches for "password", clicks on the "Saved Logins" button, and waits for the "Saved Logins" dialog to complete loading
:jaws, is there performance concerns with these pages? Are there special consideration when loading or getting events on these pages? As it stands we don't have any simple way to click in a search field and type, we could probably copy code from mochitest to make that happen. What specifically do we want to measure, page load time via onload event (other than the last 2)?
Well, we have https://bugzilla.mozilla.org/show_bug.cgi?id=1382170 and we probably could have avoided that with a talos test. Running the searches isn't super important, but it exercises a (expected) common workflow. I think we would want to measure the time until the end of the last DOMContentLoaded. Right now, one of those measures at 342ms on my local machine.
we will review this in the August cycle of Talos work- we have a lot of PTO that month and other scheduled changes- thanks for the info!
Whiteboard: [PI:August]
Whiteboard: [PI:August] → [PI:September]
Hi :jaws, not sure if this is something that you still have a need for - but if so, if you could put together a test patch that drives the test page/clicks and searches etc, then I could help integrate it into talos and test it out on the various platforms.
Whiteboard: [PI:September]
Is there any chance to get any preliminary test patch? I'm working on bug 1424682 and I'd like to verify that I'm not regressing performance.
Sorry I didn't see comment 4 until now because it was missing a needinfo flag and I my bugmail is too frequent. Robert, for a first pass could we just load the following? about:preferences about:preferences#search about:preferences#privacy about:preferences#sync We could handle clicking and searches later. Would you still need someone to create a test patch for this? Or do we have a simple way to load pages? The two bugs referenced in https://bugzilla.mozilla.org/show_bug.cgi?id=1425676#c2 seem to measure more complex situations.
Flags: needinfo?(rwood)
(In reply to Jared Wein [:jaws] (please needinfo? me) from comment #6) > Sorry I didn't see comment 4 until now because it was missing a needinfo > flag and I my bugmail is too frequent. > > Robert, for a first pass could we just load the following? > > about:preferences > about:preferences#search > about:preferences#privacy > about:preferences#sync > > We could handle clicking and searches later. Would you still need someone to > create a test patch for this? Or do we have a simple way to load pages? The > two bugs referenced in > https://bugzilla.mozilla.org/show_bug.cgi?id=1425676#c2 seem to measure more > complex situations. Hi Jared, first question is what do you want to measure? Comment 2 says "I think we would want to measure the time until the end of the last DOMContentLoaded". How is that measured? Is there an event we can setup a handler to grab that? Currently a pageloader test can measure time to get the onload event (with or without moz after paint), the time to first non blank paint, or hero element (that requires a page archive). If this test doesn't want to measure any of those, then the first step is we will need to add support to talos for that new type of measurement. Also, who will be the dev contact for this new test (i.e. who will be looking into performance regressions, test breakages, etc)? Thanks!
Flags: needinfo?(rwood)
We can measure until first paint after onload. I can be the dev contact for the test.
Flags: needinfo?(rwood)
I talked to :jaws on IRC and suggested we use time-to-first-non-blank-paint instead. If I understand correctly, onload captures also at least some of the lazy loading which doesn't contribute to the perceived performance.
Ok thanks. As well as adding the new test itself (and seeing how stable/noisy it is on all the platforms, etc) this will require new taskcluster/bb configs and a corresponding treeherder etl update patch. What platform(s) is this needed on, and is there an order of priority?
Flags: needinfo?(rwood)
Whiteboard: [PI:February]
We'll want this on linux, osx, and windows. I would like this on all versions of those operating systems if possible. We should focus on getting this running on Windows first, then OSX, then Linux. In case you were asking about priority of which pages, the order can be: 1. about:preferences 2. about:preferences#search 3. about:preferences#privacy 4. about:preferences#sync Eventually we will want to load #4 with a sync account configured but we can do that in a follow-up.
(In reply to Jared Wein [:jaws] (please needinfo? me) from comment #12) > We'll want this on linux, osx, and windows. I would like this on all > versions of those operating systems if possible. We should focus on getting > this running on Windows first, then OSX, then Linux. In case you were asking > about priority of which pages, the order can be: > > 1. about:preferences > 2. about:preferences#search > 3. about:preferences#privacy > 4. about:preferences#sync > > Eventually we will want to load #4 with a sync account configured but we can > do that in a follow-up. Thanks for the info. We are restricted to what we run in production now - OSX 10.10, Win 10, and Linux64 (Ubuntu 12.04). Win 7 talos won't be running much longer. I won't be able to get to this until February - Jared if you want go ahead and create the talos pageload test and get that running locally, then when I get cycles I can take it from there and test it out, create the configs etc.
Hi from February :) Is it something you'd have time to tackle now?
Flags: needinfo?(rwood)
Hi Gandalf, yes it's on our February priorities list (to add a new pageloader test for the basic pages in comment 12, measuring fnbpaint).
Flags: needinfo?(rwood)
Assignee: nobody → rwood
Status: NEW → ASSIGNED
Got a start. The new talos test suite will be called 'about-preferences'. Note to self: The URL's with '#' i.e. 'about:preferences#search' don't work with our current pageloader url code; TODO: add support for this type of URL.
Loading 'about:preferences' via talos pageloader works fine for all 25 cycles. However loading any of the 'about:preferences#*' has a strange issue. One example, if the test manifest only has 'about:preferences#search' - the first time 'about:preferences#search' is loaded, we receive the 'load' event just fine (and we are able to get timeToNonBlankPaint just fine also). So running the test with --tppagecycles 1 works great. However, for more than one cycle, in the second cycle we never receive the 'load' event and the test hangs/times out. Another example, if the test manifest has 'about:preferences' and 'about:preferences#search'. The first url ('about:preferences') works fine for all 25 tppaagecycles, however when the test moves on to 'about:preferences#search', this time the very first tppagecycle fails - the 'load' event is never received even the first time. So it looks like once 'about:preferences' -or- 'about:preferences#search' is loaded once, any subsequent loads of 'about:preferences#search' (or any other about:preferences#*) doesn't trigger a new 'load' event. The page IS displayed in the browser though - and if I manually hit the browser refresh button, then the 'load' event is received. Is there some kind of a bug here? Is the refresh forcing a new 'load' event, but loading about:preferences#* via content.loadURI is not? Or maybe this is expected behaviour on these types of pages? If so, we can't measure them... unless there's an alternative 'load' event or something.
Flags: needinfo?(gandalf)
one idea is to not reload the same page, but cycle through them: tpcycles = 25 tppagecycles = 1 that will break on >1 about:preferences#... maybe we can force an about:blank between them, or a dummy pageload to break it up?
I'm not familiar too much with the way Preferences handle pane selection on load. Redirecting to :jaws
Flags: needinfo?(gandalf) → needinfo?(jaws)
Navigating from about:preferences to about:preferences#search, or about:preferences#search to about:preferences#privacy generates a 'hashchange' event on the `window` that you can listen for. We use this event to determine which section of the preferences to display, https://searchfox.org/mozilla-central/rev/d03ad8843e3bf2e856126bc53b0475c595e5183b/browser/components/preferences/in-content/preferences.js#75 This would be testing changing categories. If you want to test a clean page load for each of the categories then you will need to put a dummy about:blank in between each of the page loads.
Flags: needinfo?(jaws)
Thanks all! Implemented it by: - setting tpcycles 25, and tppagecycles 1 (loads each page in the manifest once after eachother then repeats entire set) - added 'about:blank' in the manifest in between each test page - turned off the requirement of first-non-blank-paint if the page is 'about:blank' - made sure any measurements for 'about:blank' are not reported This ensures that no 'about:preferences#' page is loaded immediately after itself or one another, so that we receive the 'load' (and fnbpaint) events each time.
Comment on attachment 8949906 [details] Bug 1384272 - Add a talos test that tracks the performance of opening about:preferences; https://reviewboard.mozilla.org/r/219216/#review226198 just one comment about the suite ::: taskcluster/ci/test/talos.yml:42 (Diff revision 5) > + max-run-time: 900 > + mozharness: > + extra-options: > + - --suite=about-preferences > + - --add-option > + - --webServer,localhost I would prefer if we put this in the chromez suite- the runtime is so short here we could really save a bit of overhead- also chrome related tests in chromez... :)
Attachment #8949906 - Flags: review?(jmaher) → review+
Depends on: 1438340, 1438339
(In reply to Joel Maher ( :jmaher) (UTC-5) from comment #29) > I would prefer if we put this in the chromez suite- the runtime is so short > here we could really save a bit of overhead- also chrome related tests in > chromez... :) Thanks for the review, I added the test to the chromez suite as you suggested (good idea!).
Comment on attachment 8949906 [details] Bug 1384272 - Add a talos test that tracks the performance of opening about:preferences; Hey :jmaher, I found a couple of bugs while working on this new test, which uses fnbpaint. The pageloader wasn't actually waiting for fnbpaint; it was moving to the next page after the regular delay, and just luckily we were always getting a value for fnbpaint anyway in the current tests that use fnbpaint (tp6). Also there was another bug in the fnbpaint code, it was never setting the retry count back to zero, so if one pagecycle didn't get the fnbpaint value on the first try, and retried, then if the next pagecount didn't get fnbpaint, its retry counter was continued from the previous pagecycle. I've updated this patch to fix both of these issues. Can I get another r? please just for these latest updates please? Comment 36 has the latest up-to-date try run. Thanks!
Attachment #8949906 - Flags: review+ → review?(jmaher)
Comment on attachment 8949906 [details] Bug 1384272 - Add a talos test that tracks the performance of opening about:preferences; https://reviewboard.mozilla.org/r/219216/#review227616 this looks good, thanks for updating :)
Attachment #8949906 - Flags: review?(jmaher) → review+
Pushed by rwood@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/ecb24bcac0cf Add a talos test that tracks the performance of opening about:preferences; r=jmaher
https://hg.mozilla.org/mozilla-central/rev/ecb24bcac0cf
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
status-firefox60: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla60
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: