32.36% glvideo Mean tick time across 100 ticks: (osx-cross) regression on push b84a4439260322c260fc272e3b8a3537b616d3f1 (Fri Jul 7 2017)

RESOLVED FIXED in Firefox 56

Status

()

Core
Memory Allocator
RESOLVED FIXED
5 months ago
4 months ago

People

(Reporter: igoldan, Assigned: glandium)

Tracking

(Blocks: 1 bug, {perf, regression, talos-regression})

unspecified
mozilla56
Unspecified
Mac OS X
perf, regression, talos-regression
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox-esr52 unaffected, firefox54 unaffected, firefox55 unaffected, firefox56 fixed)

Details

(Whiteboard: [MemShrink:P1])

MozReview Requests

()

Submitter Diff Changes Open Issues Last Updated
Loading...
Error loading review requests:

Attachments

(1 attachment)

Talos has detected a Firefox performance regression from push:

https://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?changeset=b84a4439260322c260fc272e3b8a3537b616d3f1

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

Regressions:

 32%  glvideo Mean tick time across 100 ticks:  osx-cross opt e10s     20.40 -> 27.01


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

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
Blocks: 1378658
:glandium This is related to bug 1378658. Could you look over it and estimate a fix time for the regression?
Flags: needinfo?(mh+mozilla)

Updated

5 months ago
Component: Untriaged → Memory Allocator
Product: Firefox → Core
Another regression showed up and it's related to this bug:

 11%  basic_compositor_video summary osx-cross opt e10s                10.89 -> 12.11

For up to date results, see: https://treeherder.mozilla.org/perf.html#/alerts?id=7737
Comment hidden (mozreview-request)
(Assignee)

Updated

5 months ago
Assignee: nobody → mh+mozilla
Flags: needinfo?(mh+mozilla)

Comment 4

5 months ago
mozreview-review
Comment on attachment 8885052 [details]
Bug 1379139 - Instead of not recycling huge chunks, zero them.

https://reviewboard.mozilla.org/r/155918/#review161032

::: memory/mozjemalloc/mozjemalloc.cpp:1927
(Diff revision 1)
>          MOZ_ASSERT(ret);
>          return (ret);
>  }
>  
>  bool
> -pages_purge(void *addr, size_t length)
> +pages_purge(void *addr, size_t length, bool force)

Please add a comment about the meaning of |force|. And would |force_zero| be a better name?
Attachment #8885052 - Flags: review?(n.nethercote) → review+
(Assignee)

Comment 5

5 months ago
(In reply to Nicholas Nethercote [:njn] from comment #4)
> Comment on attachment 8885052 [details]
> Bug 1379139 - Instead of not recycling huge chunks, zero them.
> 
> https://reviewboard.mozilla.org/r/155918/#review161032
> 
> ::: memory/mozjemalloc/mozjemalloc.cpp:1927
> (Diff revision 1)
> >          MOZ_ASSERT(ret);
> >          return (ret);
> >  }
> >  
> >  bool
> > -pages_purge(void *addr, size_t length)
> > +pages_purge(void *addr, size_t length, bool force)
> 
> Please add a comment about the meaning of |force|. And would |force_zero| be
> a better name?

I'll change the variable name before landing, and will leave the comment to a rebased bug 1360772.
Comment hidden (mozreview-request)

Comment 7

5 months ago
Pushed by mh@glandium.org:
https://hg.mozilla.org/integration/autoland/rev/729a49478e7a
Instead of not recycling huge chunks, zero them. r=njn

Comment 8

5 months ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/729a49478e7a
Status: NEW → RESOLVED
Last Resolved: 5 months ago
status-firefox56: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla56
Great news: the last pushed resolved the regression. Thank you!

== Change summary for alert #7822 (as of July 11 2017 04:01 UTC) ==

Improvements:

 23%  glvideo Mean tick time across 100 ticks:  osx-cross opt e10s     27.34 -> 20.95

For up to date results, see: https://treeherder.mozilla.org/perf.html#/alerts?id=7822
:glandium Please also look over the 2nd regression I mentioned in comment 2.
Flags: needinfo?(mh+mozilla)
:glandium There is alot of noise for the basic_compositor_video alert. I want to know if your push has anything to do with it, or actually the previous changeset, from bug 1367900, right before bug 1378658, may be the source of the regression.

I did many retriggers now, and looks like I may have mistaken comment 2.
Setting Beta/ESR52 to affected for now so it doesn't fall off the radar should bug 1378658 end up getting uplifted.
status-firefox54: --- → unaffected
status-firefox55: --- → affected
status-firefox-esr52: --- → affected
(Assignee)

Comment 13

5 months ago
(In reply to Ionuț Goldan [:igoldan], Performance Sheriffing from comment #10)
> :glandium Please also look over the 2nd regression I mentioned in comment 2.

It went down with the same push.
Flags: needinfo?(mh+mozilla)
:glandium Great to see! Thanks
Just for the record, bug 1378658 caused this AWSY regresssion also:

== Change summary for alert #7739 (as of July 07 2017 00:22 UTC) ==

Regressions:

108%  Resident Memory summary osx-cross opt      691,257,509.89 -> 1,434,526,909.55

For up to date results, see: https://treeherder.mozilla.org/perf.html#/alerts?id=7739


But the fix pushed (from comment 7), resolved it too:

== Change summary for alert #7823 (as of July 11 2017 04:01 UTC) ==

Improvements:

 53%  Resident Memory summary osx-cross opt      1,437,716,343.72 -> 682,446,716.37

For up to date results, see: https://treeherder.mozilla.org/perf.html#/alerts?id=7823
(Assignee)

Updated

5 months ago
Duplicate of this bug: 1380231

Updated

5 months ago
Duplicate of this bug: 1379527
status-firefox55: affected → unaffected
status-firefox-esr52: affected → unaffected
Whiteboard: [MemShrink:P1]
Duplicate of this bug: 1379043
You need to log in before you can comment on or make changes to this bug.