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

RESOLVED WORKSFORME

Status

()

defect
P2
normal
RESOLVED WORKSFORME
2 years ago
2 years ago

People

(Reporter: igoldan, Assigned: gkrizsanits)

Tracking

({perf, regression, talos-regression})

56 Branch
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox-esr52 unaffected, firefox55 unaffected, firefox56 affected)

Details

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
:Mardak Looks like the latest push for fixing the memory regressions affected the DAMP Talos tests on mutliple platforms. Please look over these also.
Flags: needinfo?(edilee)
Component: Activity Streams: General → Activity Streams: Newtab
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. ?
Blocks: 1363601
Component: Activity Streams: Newtab → DOM: Content Processes
Flags: needinfo?(edilee) → needinfo?(gkrizsanits)
Product: Firefox → Core
we will be merging trunk to beta next week- it would be nice to have this understood if not resolved before we merge.
(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.
Flags: needinfo?(gkrizsanits)
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?
Flags: needinfo?(jmaher)
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 :(
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.
Flags: needinfo?(jmaher)
What's the status of 56 here? PPM is disabled there I believe? Is there more follow-up work to be done still?
Flags: needinfo?(gkrizsanits)
Version: unspecified → 56 Branch
(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.
Depends on: 1385249
Flags: needinfo?(gkrizsanits)
Assignee: nobody → gkrizsanits
Priority: -- → P2
fixed by backout.
Status: NEW → RESOLVED
Closed: 2 years ago
Resolution: --- → WORKSFORME
You need to log in before you can comment on or make changes to this bug.