1495919 - Many live WebRender font allocations detected by DMD

Status: RESOLVED FIXED

(Core :: Graphics: WebRender, enhancement, P1)

Description

[Bobby Holley (:bholley)]

I ran my normal browsing under DMD today and got a couple of interesting stacks under WebRender. I'm filing the large font-related ones here.

Technically all this means is that memory reporters don't find this memory (so that it currently shows up as heap-unclassified). But given that we're looking at hundreds of live unreported allocations, I'm somewhat suspicious that we have a leak here.

This may be the same issue as bug 1495661. Getting it on file independently for now, feel free to dupe.

Comment 1

[Bobby Holley (:bholley)]

Attachment: Allocation stacks

Comment 2

[Bobby Holley (:bholley)]

This measurement was done on mac, so this is all in the parent process, but the stacks all come out of the WR worker pool.

Note that there are ~800 of each, which suggests that we're looking at ~800 live fonts.

Comment 3

[Bobby Holley (:bholley)]

Attachment: UpdateResources stacks

There are also these stacks in UpdateResources. There are about 800 of them, and it seems plausible that they're for fonts. If you think they aren't, let me know and I can file another bug.

Comment 4

[Bobby Holley (:bholley)]

We'd hoped this was fixed by bug 1495661, but looks like it wasn't. Ryan saw his GPU process balloon to 10GB of ram today. He ran with DMD and got stacks in add_font.

Comment 5

[Bobby Holley (:bholley)]

Attachment: Ryan's DMD log

Ryan said this corresponded to about 180MB of memory in the GPU process. He'd previously seen it at 10GB, but had to restart to enable DMD. Apparently opening new tabs was a consistent way to grow it.

Comment 6

[Maire Reavy [:mreavy]]

Lee -- I need you to take bug as your top priority as soon as you see this.  It's a P1 which means "beta blocker."
Jonathan -- to stay on in Beta, we need to get this fixed asap.  ANYTHING you can do is much appreciated.  It's more important than anything else you're working. 
Thanks, both!

Comment 7

[Ryan VanderMeulen [:RyanVM]]

Attachment: my about:support

In case, here's my system info. As Bobby said, it seems to grow pretty consistently as I open new tabs and navigate around. Doesn't go down when I close them after. The 180MB log posted here was after less than an hour.

Comment 8

[Ryan VanderMeulen [:RyanVM]]

Attachment: another dmd log

Ran my local builds with DMD enabled today (which was thankfully a LOT easier to symbolicate). This is after ~45min uptime and GPU process currently over 1GB of heap-unclassified.

Comment 9

[Jonathan Kew [:jfkthame]]

Ryan, are there particular websites you visit that tend to make the GPU process heap-unclassified grow so much? I'm having trouble reproducing anything nearly as alarming as your reports, although I'm trying to visit sites that use a bunch of fonts (both webfonts and system-installed ones).

Comment 10

[Ryan VanderMeulen [:RyanVM]]

Nothing in particular - Treeherder, Slack, GMail (two tabs), Bugzilla, GDocs/GSheets, etc.

Comment 11

[Lee Salzman [:lsalzman]]

This is a problem with the dwrote-rs library. It looks like there was some hastily written code to manage DWriteFontFileStreams that, while seemingly working, was not conscientious about freeing memory.

I put up a PR to resolve this: https://github.com/servo/dwrote-rs/pull/22

Some small tweaks in WR will be further necessary to use this.

Comment 12

[Lee Salzman [:lsalzman]]

WR PR to use the fixed version of dwrote: https://github.com/servo/webrender/pull/3217

Comment 13

[Lee Salzman [:lsalzman]]

Attachment: ensure IDWriteFontFileStream stays alive with NativeFontResourceDWrite

It isn't guaranteed to stay alive, so we need to hold it. Noticed this in the process of debugging the leak.

Comment 14

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/11f359e1b038
ensure IDWriteFontFileStream stays alive with NativeFontResourceDWrite. r=jrmuizel

Comment 15

[Glenn Watson [:gw]]

The WR patch (https://github.com/servo/webrender/pull/3217) has landed now, and will be in the next WR update in Gecko.

Comment 16

[Cosmin Sabou [:CosminS]]

https://hg.mozilla.org/mozilla-central/rev/11f359e1b038

Comment 17

[Maire Reavy [:mreavy]]

Ryan -- Can you retest this once bug 1500233 lands and mark this fixed if you can't repro?  (Thanks for your help with this one!)

Comment 18

[Ryan VanderMeulen [:RyanVM]]

Confirmed, GPU process heap-unclassified is stable now for me in the low 20s of MB. No signs of unbounded growth after extensive usage :)

Comment 19

[Maire Reavy [:mreavy]]

\o/ - Thanks, Ryan.   And many thanks, Lee and Jonathan, for all your work on getting fonts working in WR (including in particular this bug).