2.19% Heap Unclassified (linux64) regression on push c31c7c6520240d32ac26741022d89b4f407885a9 (Mon Sep 4 2017)

REOPENED
Unassigned

Status

()

Core
Layout: Text
P3
normal
REOPENED
3 months ago
2 months ago

People

(Reporter: igoldan, Unassigned)

Tracking

(Depends on: 1 bug, Blocks: 1 bug, {perf, regression})

18 Branch
perf, regression
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox55 unaffected, firefox56 unaffected, firefox57 fix-optional)

Details

We have detected an awsy regression from push:

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

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

Regressions:

  2%  Heap Unclassified summary linux64 opt      54,341,833.72 -> 55,533,260.62


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

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 jobs in a pushlog format.

To learn more about the regressing test(s), please see: https://developer.mozilla.org/en-US/docs/Mozilla/Performance/AWSY
Component: Untriaged → Layout: Text
Product: Firefox → Core
Jonathan, can you take a look at this AWSY regression?
Flags: needinfo?(jfkthame)
I think the increase here is in line with expectations.

The change in bug 835204 means that we are likely to access the 'name' tables of a bunch of fonts (on an as-needed basis, depending on what is requested by CSS) to identify additional font-family names that should be matched, and in some cases we'll end up creating additional font-family and -face records for our font lists (so that font-family searches will directly find the expected entries). Even when we don't actually find additional legacy names to add to our lists, the fact that we've inspected the 'name' tables means they'll end up in the gfxFontEntry table caches.

So some increase in memory use is natural here: we are (in effect) loading more fonts, because we're loading copies of them under different names; and we're loading font data that we previously didn't examine, in order to support this.
Flags: needinfo?(jfkthame)

Updated

3 months ago
status-firefox55: --- → unaffected
status-firefox56: --- → unaffected
status-firefox57: --- → affected
:jfkthame, is there work to reduce this, or should we accept this?
I think we should accept it; I don't currently see a good way to reduce the memory footprint without impacting performance or behavior. (FWIW, I expect the memory use should be significantly less for the macOS and Win/DWrite font backends, as we are able to access font tables there without copying them into our own cache. Linux is the worst-case here.)
Status: NEW → RESOLVED
Last Resolved: 3 months ago
Resolution: --- → WONTFIX
(In reply to Jonathan Kew (:jfkthame) from comment #4)
> I think we should accept it; I don't currently see a good way to reduce the
> memory footprint without impacting performance or behavior. (FWIW, I expect
> the memory use should be significantly less for the macOS and Win/DWrite
> font backends, as we are able to access font tables there without copying
> them into our own cache. Linux is the worst-case here.)

Regressions in heap-unclassifed are two fold: a) we regressed memory b) we're not measuring it. You decided we're okay with a) but I'd saw we still need to fix b). Can we add reporters for this cache?
Status: RESOLVED → REOPENED
Flags: needinfo?(jfkthame)
Resolution: WONTFIX → ---
(In reply to Eric Rahm [:erahm] (please no mozreview requests) from comment #5)
> (In reply to Jonathan Kew (:jfkthame) from comment #4)
> > I think we should accept it; I don't currently see a good way to reduce the
> > memory footprint without impacting performance or behavior. (FWIW, I expect
> > the memory use should be significantly less for the macOS and Win/DWrite
> > font backends, as we are able to access font tables there without copying
> > them into our own cache. Linux is the worst-case here.)
> 
> Regressions in heap-unclassifed are two fold: a) we regressed memory b)
> we're not measuring it. You decided we're okay with a) but I'd saw we still
> need to fix b). Can we add reporters for this cache?

Hmm. We do have reporters for the font table cache in gfxFontEntry:

https://dxr.mozilla.org/mozilla-central/rev/f9a5e9ed62103c84e4cde915f4d08f1ce71be83e/gfx/thebes/gfxFontEntry.cpp#486
https://dxr.mozilla.org/mozilla-central/rev/f9a5e9ed62103c84e4cde915f4d08f1ce71be83e/gfx/thebes/gfxFontEntry.cpp#971
https://dxr.mozilla.org/mozilla-central/rev/f9a5e9ed62103c84e4cde915f4d08f1ce71be83e/gfx/thebes/gfxFontEntry.cpp#994-997

So that shouldn't show up as unclassified. My mention of the gfxFontEntry table caches, therefore, was slightly mistaken. I think the heap-unclassified memory most probably indicates something happening within fontconfig (can we get any further insight via tools like DMD?), which is out of our control and not readily measurable/reportable, AFAIK.
Flags: needinfo?(jfkthame)

Updated

3 months ago
status-firefox57: affected → ---
status-firefox57: --- → fix-optional
(In reply to Jonathan Kew (:jfkthame) from comment #6)
> So that shouldn't show up as unclassified. My mention of the gfxFontEntry
> table caches, therefore, was slightly mistaken. I think the
> heap-unclassified memory most probably indicates something happening within
> fontconfig (can we get any further insight via tools like DMD?), which is
> out of our control and not readily measurable/reportable, AFAIK.

Okay I'm going to try to hack in DMD support to AWSY so we can get a better sense of what's going on.
That sounds awesome, thanks!
Flags: needinfo?(erahm)
Duplicate of this bug: 1400245
Depends on: 1395540
(In reply to Eric Rahm [:erahm] (please no mozreview requests) from comment #7)
> (In reply to Jonathan Kew (:jfkthame) from comment #6)
> > So that shouldn't show up as unclassified. My mention of the gfxFontEntry
> > table caches, therefore, was slightly mistaken. I think the
> > heap-unclassified memory most probably indicates something happening within
> > fontconfig (can we get any further insight via tools like DMD?), which is
> > out of our control and not readily measurable/reportable, AFAIK.
> 
> Okay I'm going to try to hack in DMD support to AWSY so we can get a better
> sense of what's going on.

I'm working on this in bug 1395540.
Flags: needinfo?(erahm)

Updated

2 months ago
Priority: -- → P3
You need to log in before you can comment on or make changes to this bug.