UrlbarProviderSearchTips.jsm appears in Talos ts_paint profiles
Categories
(Firefox :: Address Bar, task, P3)
Tracking
()
Tracking | Status | |
---|---|---|
firefox80 | --- | fixed |
People
(Reporter: mak, Assigned: adw)
References
(Blocks 1 open bug, Regressed 1 open bug)
Details
(Keywords: perf, Whiteboard: [fxperf:p3])
Attachments
(1 file)
I was running
mach talos-test -a ts_paint_webext --gecko-profile
and I noticed in the profile that UrlbarProviderSearchTips.jsm appears a couple times for lastBrowserUpdateDate and isBrowserShowingNotifications that are initialized from onLocationChange.
Can we do something to reduce the impact on Ts?
Reporter | ||
Updated•4 years ago
|
Reporter | ||
Updated•4 years ago
|
Comment 1•4 years ago
|
||
Here's a profile showing this: https://share.firefox.dev/2V7qovs
Assignee | ||
Comment 2•4 years ago
|
||
(In reply to Marco Bonardo [:mak] from comment #0)
Can we do something to reduce the impact on Ts?
_maybeShowTipForUrl
does several checks to determine whether to show a tip. The first two checks are isBrowserShowingNotification
and lastBrowserUpdateDate
. We should move those to the end because the (vast?) majority of the time the checks that follow those two will be hit and we can return early before ever having to check isBrowserShowingNotification
and lastBrowserUpdateDate
.
I don't know whether that's enough to help Ts because it depends on the page that Ts loads. If it loads about:home/newtab, then it won't help. But it's worth doing for sure.
Beyond that, the stack in comment 1 is pretty surprising. isBrowserShowingNotification
is a crazy big percentage. I don't really understand the stack below that call because it's all internal JS and compartments stuff...
Assignee | ||
Comment 3•4 years ago
|
||
(In reply to Drew Willcoxon :adw from comment #2)
We should move those to the end because the (vast?) majority of the time the checks that follow those two will be hit and we can return early before ever having to check
isBrowserShowingNotification
andlastBrowserUpdateDate
.
I'll make a patch for this.
Assignee | ||
Comment 4•4 years ago
|
||
Assignee | ||
Comment 5•4 years ago
|
||
If I'm reading that profile right, a big part is loading AppMenuNotifications.jsm and ShellService.jsm (via DefaultBrowserCheck.willCheckDefaultBrowser
in BrowserGlue.jsm).
At least for Ts, we could delay checking for tips until after delayed startup or whatever the condition is when Ts ends.
Reporter | ||
Comment 6•4 years ago
|
||
(In reply to Drew Willcoxon :adw from comment #5)
At least for Ts, we could delay checking for tips until after delayed startup or whatever the condition is when Ts ends.
I assume tips are a non-critical feature on startup, they will be effective even if shown later in the session. delayed startup would be a good start.
Assignee | ||
Updated•4 years ago
|
Updated•4 years ago
|
Assignee | ||
Comment 7•4 years ago
|
||
Pushed by dwillcoxon@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/8fb311e74315 Improve UrlbarProviderSearchTips's impact on ts_paint. r=mak
Assignee | ||
Updated•4 years ago
|
Comment 9•4 years ago
|
||
bugherder |
Comment 10•4 years ago
|
||
== Change summary for alert #26408 (as of Thu, 02 Jul 2020 03:27:46 GMT) ==
Improvements:
4% sessionrestore windows7-32-shippable opt e10s stylo 479.83 -> 458.50
3% startup_about_home_paint windows7-32-shippable opt e10s stylo 605.25 -> 586.75
3% startup_about_home_paint windows7-32-shippable opt e10s stylo 604.79 -> 586.83
For up to date results, see: https://treeherder.mozilla.org/perf.html#/alerts?id=26408
Reporter | ||
Comment 11•4 years ago
|
||
nice!
Description
•