1382608 - 3.49 - 4.14% damp (linux64, windows7-32) regression on push 4f418e6b759c5bdbc85f21559c528dfe9d2791c9 (Wed Jul 19 2017)

Status: RESOLVED WORKSFORME

(Core :: DOM: Content Processes, defect, P2)

Description

[Ionuț Goldan [:igoldan]]

Talos has detected a Firefox performance regression from push:

https://hg.mozilla.org/integration/autoland/pushloghtml?changeset=4f418e6b759c5bdbc85f21559c528dfe9d2791c9

As author of one of the patches included in that push, we need your help to address this regression.

Regressions:

  4%  damp summary linux64 pgo e10s     212.51 -> 221.31
  4%  damp summary linux64 opt e10s     241.22 -> 249.76
  3%  damp summary windows7-32 pgo e10s 200.57 -> 207.57


You can find links to graphs and comparison views for each of the above tests at: https://treeherder.mozilla.org/perf.html#/alerts?id=8095

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

Comment 1

[Ionuț Goldan [:igoldan]]

:Mardak Looks like the latest push for fixing the memory regressions affected the DAMP Talos tests on mutliple platforms. Please look over these also.

Comment 2

[Ed Lee :Mardak]

gabor turned off the preallocated process manager in bug 1363601 and can probably explain the regression better if there was a matching improvement when it was turned on to begin with. ?

Comment 3

[Joel Maher ( :jmaher ) (UTC -8)]

we will be merging trunk to beta next week- it would be nice to have this understood if not resolved before we merge.

Comment 4

[Gabor Krizsanits (INACTIVE)]

(In reply to Ed Lee :Mardak from comment #2)
> gabor turned off the preallocated process manager in bug 1363601 and can
> probably explain the regression better if there was a matching improvement
> when it was turned on to begin with. ?

It was turned on in bug 1376895 early May. And it did not cause any improvement on damp back then. I turned it off on Beta too, did it cause regression there too? Probably something changed that caused regression only without the ppm off, which we did not measure and now that we turned it off we see the regression. It's hard to tell what was that change. Either way, in it's current state I cannot turn it back on since the crashes, and other issues (activity stream related). I hope I can fix all the issues in the near future and turn it back on.

Comment 5

[Ed Lee :Mardak]

I believe gabor meant to reference bug 1341008 where the pref was turned on by default.

I retriggered g2 on linux64 opt a few times, and there were no significant damp changes on mozilla-beta when the pref was turned off:
https://treeherder.mozilla.org/perf.html#/compare?originalProject=mozilla-beta&originalRevision=293c0a44fda7&newProject=mozilla-beta&newRevision=e5f14b9ae6c4c1e0ff1c430503671289120db203&framework=1&filter=damp

jmaher, from comment 4, sounds like the pref `dom.ipc.processPrelaunch.enabled` won't be turned back on until the crash is fixed, and hopefully the damp regression goes away then. I'm guessing it won't be fixed before the merge, so does this bug just stay open blocked on some bug to fix crashes and re-enable?

Comment 6

[Gabor Krizsanits (INACTIVE)]

I think I know what's going on. That patch on bug 1364849 caused some improvements, and that patch did not make it to the current beta. That patch relied on the ppm, so we turned off that feature as well :(

Comment 7

[Joel Maher ( :jmaher ) (UTC -8)]

yeah, we can leave this bug open, it is good to know what state it is in and we can revisit it in a few weeks or so and check back in.

Comment 8

[Ryan VanderMeulen [:RyanVM]]

What's the status of 56 here? PPM is disabled there I believe? Is there more follow-up work to be done still?

Comment 9

[Gabor Krizsanits (INACTIVE)]

(In reply to Ryan VanderMeulen [:RyanVM] from comment #8)
> What's the status of 56 here? PPM is disabled there I believe? Is there more
> follow-up work to be done still?

I need to land a patch for bug 1376895, then reenable ppm on nightly. After a week or so I will uplift all 3 patches for 56 if all goes well.

Comment 10

[Jim Mathies [:jimm]]

fixed by backout.