Closed
Bug 1448290
Opened 7 years ago
Closed 6 years ago
0.75 - 2.36% sessionrestore (linux64, windows10-64) regression on push 8b721870d808 (Wed Mar 21 2018)
Categories
(Core :: Networking, defect)
Core
Networking
Tracking
()
RESOLVED
WONTFIX
Tracking | Status | |
---|---|---|
firefox-esr52 | --- | unaffected |
firefox59 | --- | unaffected |
firefox60 | --- | unaffected |
firefox61 | --- | wontfix |
People
(Reporter: igoldan, Unassigned)
References
Details
(Keywords: perf, regression, talos-regression)
Talos has detected a Firefox performance regression from push: https://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=797722641610dad674216f190a8f4d7a76af9b8c&tochange=8b721870d808f23a4f515e98516e76c3dc9e99b3 As author of one of the patches included in that push, we need your help to address this regression. Regressions: 2% sessionrestore linux64 pgo e10s stylo 266.96 -> 273.25 1% sessionrestore windows10-64 pgo e10s stylo365.17 -> 367.92 You can find links to graphs and comparison views for each of the above tests at: https://treeherder.mozilla.org/perf.html#/alerts?id=12312 On the page above you can see an alert for each affected platform as well as a link to a graph showing the history of scores for this test. There is also a link to a treeherder page showing the Talos jobs in a pushlog format. To learn more about the regressing test(s), please see: https://wiki.mozilla.org/Buildbot/Talos/Tests For information on reproducing and debugging the regression, either on try or locally, see: https://wiki.mozilla.org/Buildbot/Talos/Running *** Please let us know your plans within 3 business days, or the offending patch(es) will be backed out! *** Our wiki page outlines the common responses and expectations: https://wiki.mozilla.org/Buildbot/Talos/RegressionBugsHandling
Reporter | ||
Updated•7 years ago
|
Component: Untriaged → Networking
Product: Firefox → Core
Reporter | ||
Comment 1•7 years ago
|
||
:michal Please look over this perf regression. Can we resolve it, or should we accept/back it out?
Flags: needinfo?(michal.novotny)
Updated•7 years ago
|
status-firefox59:
--- → unaffected
status-firefox60:
--- → unaffected
status-firefox61:
--- → affected
status-firefox-esr52:
--- → unaffected
Comment 2•7 years ago
|
||
The patch in bug 1373708 adds some overhead because it moves I/O to a background thread. The performance gain occurs only when accessing jar file which is not present in OS's cache. This should be visible in cold startups. When the file is cached there is a minor performance hit caused by the posted events. I don't think there is anything we can do about it in the JAR channel code.
Flags: needinfo?(michal.novotny)
Comment 3•7 years ago
|
||
I don't think it's so much a matter of overhead as latency. When we load jar channels in background threads, there's more opportunity for other tasks to wind up in the main thread event queue before the requests complete, and unblock whatever work was waiting on them. I run into these kinds of regressions pretty frequently when I add asynchrony to things that happen during startup. It's a problem we need to look into, but I don't think it should block bug 1373708, since the wins it gives us in cold startup performance should easily outweigh the noise it adds to sessionrestore times.
Reporter | ||
Comment 4•7 years ago
|
||
(In reply to Kris Maglione [:kmag] (long backlog; ping on IRC if you're blocked) from comment #3) > I run into these kinds of regressions pretty frequently when I add > asynchrony to things that happen during startup. It's a problem we need to > look into, but I don't think it should block bug 1373708, since the wins it > gives us in cold startup performance should easily outweigh the noise it > adds to sessionrestore times. Can we file a separate bug, describing this improvement we can do and block that instead?
Flags: needinfo?(kmaglione+bmo)
Reporter | ||
Comment 6•6 years ago
|
||
(In reply to Marion Daly [:mdaly] from comment #5) > Status? An improvement of same magnitude landed somewhere around March 29. It was enough to cancel this one. Given the time that passed, I would close this bug as WONTFIX.
Reporter | ||
Updated•6 years ago
|
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → WONTFIX
Updated•6 years ago
|
Updated•6 years ago
|
Flags: needinfo?(jduell.mcbugs)
Updated•6 years ago
|
Flags: needinfo?(kmaglione+bmo)
You need to log in
before you can comment on or make changes to this bug.
Description
•