1355600 - nsFont destructor takes 9ms to free up memory.

Status: RESOLVED DUPLICATE of bug 1355595

(Core :: Layout, enhancement, P3)

Description

[Evelyn Hung]

From profile [1] it spends time on 

nsTArray_base<nsTArrayInfallibleAllocator, nsTArray_CopyWithMemutils>::ShrinkCapacity(unsigned long, unsigned long). 

I'm not sure if there is anything to improve. File this bug for a record.

Is this related to bug 1113069?

[1] https://perfht.ml/2p4IE6S

Comment 1

[Jeff Muizelaar [:jrmuizel]]

9ms / 8.1 seconds is only .1% of the time. It's not obvious to me that this problem is worth dealing with.

Comment 2

[Ben Kelly [:bkelly, not reviewing]]

(In reply to Jeff Muizelaar [:jrmuizel] from comment #1)
> 9ms / 8.1 seconds is only .1% of the time. It's not obvious to me that this
> problem is worth dealing with.

It's more than half our frame budget, though.

Comment 3

[Jeff Muizelaar [:jrmuizel]]

(In reply to Ben Kelly [not reviewing due to deadline][:bkelly] from comment #2)
> (In reply to Jeff Muizelaar [:jrmuizel] from comment #1)
> > 9ms / 8.1 seconds is only .1% of the time. It's not obvious to me that this
> > problem is worth dealing with.
> 
> It's more than half our frame budget, though.

This isn't happening all at once. It's a bunch of individual samples spread out over 8.1 seconds.

Comment 4

[Daniel Holbert [:dholbert]]

The profile shows ~nsFont being called by "nsTextFrame::ComputeSelectionUnderlineHeight".  I was just looking at that function, trying to figure out how it could possibly be instantiating / destructing a nsFont.  There's no obvious nsFont involved if you look at the current version of this code.

And then I realized, the function was rewritten at some point since that profile was taken.  Specifically, it was rewritten in bug 1355595, in this changeset:
https://hg.mozilla.org/mozilla-central/rev/abfece312d74

That removed this code:
-      int32_t defaultFontSize =
-        aPresContext->AppUnitsToDevPixels(nsStyleFont(aPresContext).mFont.size);
-      gfxFloat fontSize = std::min(gfxFloat(defaultFontSize),
-                                 aFontMetrics.emHeight);

The temporary nsStyleFont instance that we create there has a nsFont member-variable. So that's the font that was getting destructed here.

In the new version of this code, we don't instantiate a nsStyleFont (and hence don't instantiate a nsFont) -- we just use a persistent one instead, via the aPresContext->GetDefaultFont() API.

So, this is just a version of bug 1355595, basically. I'm duping this over to that bug.

(Note: while taking a look at this and trying to catch the ~nsFont in a debugger, I ran into a different unnecessary-nsFont-instantiation/destruction bug, and I filed bug 1383925 on that.)