Ionuț Goldan [:igoldan] Reporter
	Description • 7 years ago

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

	Ionuț Goldan [:igoldan] Reporter
	Comment 1 • 7 years ago

:michal Please look over this perf regression. Can we resolve it, or should we accept/back it out?

	Michal Novotny [:michal]
	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.

	Kris Maglione [:kmag]
	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.

	Ionuț Goldan [:igoldan] 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?

	Marion Daly [:mdaly]
	Comment 5 • 7 years ago

Status?

	Ionuț Goldan [:igoldan] 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.