Closed Bug 1449187 Opened 2 years ago Closed 2 years ago
Perma opt OS X and Windows browser/base/content/test/performance/browser
_urlbar _keyed _search .js when Gecko 61 merges to Beta on 2018-04-26
Bug 1449187 - Whitelist extra autocomplete reflows that happen due to a default bookmark with a long URL on beta/release.
59 bytes, text/x-review-board-request
central-as-beta simulation: https://treeherder.mozilla.org/#/jobs?repo=try&revision=62fff771b6a6ffadea4f7383c0986f0e5f4f2cda&group_state=expanded&filter-resultStatus=testfailed&filter-resultStatus=busted&filter-resultStatus=exception&filter-resultStatus=retry&filter-resultStatus=usercancel&filter-resultStatus=runnable&selectedJob=170516891 Failure log: https://treeherder.mozilla.org/logviewer.html#?job_id=170517739&repo=try 03:40:45 INFO - Buffered messages finished 03:40:45 INFO - TEST-UNEXPECTED-FAIL | browser/base/content/test/performance/browser_urlbar_keyed_search.js | unexpected reflow at _handleOverflow@chrome://global/content/bindings/autocomplete.xml hit 1 times 03:40:45 INFO - Stack: 03:40:45 INFO - _handleOverflow@chrome://global/content/bindings/autocomplete.xml:2053:26 03:40:45 INFO - _onOverflow@chrome://global/content/bindings/autocomplete.xml:1444:11 03:40:45 INFO - onoverflow@chrome://browser/content/browser.xul:1:1 03:40:45 INFO - - false == true - JS frame :: chrome://mochitests/content/browser/browser/base/content/test/performance/head.js :: reportUnexpectedReflows :: line 190 03:40:45 INFO - Stack trace: 03:40:45 INFO - chrome://mochitests/content/browser/browser/base/content/test/performance/head.js:reportUnexpectedReflows:190 03:40:45 INFO - chrome://mochitests/content/browser/browser/base/content/test/performance/head.js:withPerfObserver:630 Push log from last good to first bad: https://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=f0d13136b3580182df241674cf97a3fbf1543c2a%20&tochange=97cdd8febc40ac6025bce5dec9f8dadb8e62f906 Florian, can you spot what causes this (almost opt-only) reflow? Thank you.
This seems to be using a different code path than what we expect, eg. I see '_adjustAcItem' frames in the stacks, whereas our whitelisted reflows have '_reuseAcItem'. Mike or Marco may have ideas.
I honestly don't have a very specific idea of why this should differ between Beta and Nightly, both code paths are possible, _reuseAcItem is used when we have a match that is absolutely identical to an existing one, while _adjustAcItem is used to "adjust" a match that has some differences. I can't think of any nightly-only features in the location bar at this time. Thus it just sounds timing-sensitive. Maybe we should just reduce the whitelisted stack, it's clear from quite some time that handleOverUnderflow causes these reflows, there's no big win into restricting the stack further.
I'm not sure why this would be different. :/ I agree that the most expedient solution is probably to shorten the stack to just _handleOverflow.
Bisection shows it starts with https://hg.mozilla.org/integration/mozilla-inbound/rev/330dc2385e33 / bug 1393881. This replaced the address of the SUMO bookmark which ships with non-Nightly builds to a very long url. central-as-beta simulations: last good: https://treeherder.mozilla.org/#/jobs?repo=try&revision=2a056401db63e1aaaa4f7f42752d3926e25a8fc2 first bad: https://treeherder.mozilla.org/#/jobs?repo=try&revision=0d964a123c9b1ee3f6cd42c34cd6404681691110&selectedJob=171389746
Mark, this is an unassigned P1 tracking 61+. Can you please help find an owner?
It's likely the long url changes the timing we take to populate and adds more reflows, because it will send overflow events more often. I don't think it's worth calling this a regression, those reflows were just not accounted for by the test, but were hiding there. I suggested shortening the stack, but looking at the logs this doesn't look like a shift, since I don't the see ones we track being reported as "missing"... We may just have to eat the pill and add these new stacks. The scope doesn't change, we should avoid all of these in bug 1356532.
Assignee: nobody → mak77
Status: NEW → ASSIGNED
Comment on attachment 8965699 [details] Bug 1449187 - Whitelist extra autocomplete reflows that happen due to a default bookmark with a long URL on beta/release. https://reviewboard.mozilla.org/r/234554/#review240180 Thanks!
Attachment #8965699 - Flags: review?(mconley) → review+
Comment on attachment 8965699 [details] Bug 1449187 - Whitelist extra autocomplete reflows that happen due to a default bookmark with a long URL on beta/release. https://reviewboard.mozilla.org/r/234554/#review240182 ::: browser/base/content/test/performance/browser_urlbar_keyed_search.js:18 (Diff revision 1) > * See https://developer.mozilla.org/en-US/Firefox/Performance_best_practices_for_Firefox_fe_engineers > * for tips on how to do that. > */ > > /* These reflows happen only the first time the awesomebar panel opens. */ > -const EXPECTED_REFLOWS_FIRST_OPEN = [ > +let EXPECTED_REFLOWS_FIRST_OPEN = [ Oh, fwiw, this can stay a const.
Something went wrong with the try simulations. Not sure what, but my testing locally, I'm fairly sure this is right. So I'll push it and we'll find out with the next simulation ;-)
Pushed by firstname.lastname@example.org: https://hg.mozilla.org/integration/autoland/rev/a07ed16f9792 Whitelist extra autocomplete reflows that happen due to a default bookmark with a long URL on beta/release. r=mconley
Thank you. Verified fixed in yesterday's central-as-beta simulation: https://treeherder.mozilla.org/#/jobs?repo=try&revision=e4fc08a6b378f769a7e598623495db608da5c323&filter-resultStatus=testfailed&filter-resultStatus=busted&filter-resultStatus=exception&filter-resultStatus=retry&filter-resultStatus=usercancel&filter-resultStatus=runnable&group_state=expanded
You need to log in before you can comment on or make changes to this bug.