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

Status: REOPENED

(Core :: Layout: Text and Fonts, defect, P3)

Description

[Ionuț Goldan [:igoldan]]

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

Comment 1

[Ionuț Goldan [:igoldan]]

Jonathan, can you take a look at this AWSY regression?

Comment 2

[Jonathan Kew [: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.

Comment 3

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

:jfkthame, is there work to reduce this, or should we accept this?

Comment 4

[Jonathan Kew [:jfkthame]]

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.)

Comment 5

[Eric Rahm [:erahm]]

(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?

Comment 6

[Jonathan Kew [:jfkthame]]

(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.

Comment 7

[Eric Rahm [:erahm]]

(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.

Comment 8

[Jonathan Kew [:jfkthame]]

That sounds awesome, thanks!

Comment 10

[Eric Rahm [:erahm]]

(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.