1054321 - Need memory reporter for FrameLayerBuilder

Status: RESOLVED INVALID

(Core :: Layout: Images, Video, and HTML Frames, defect)

Description

[Nicholas Nethercote [inactive]]

There's a heap-unclassified spike while viewing the PDF at https://github.com/mozilla/pdf.js/issues/2541 with pdf.js. It happens while the text layer is being drawn -- that creates thousands of small, transparent divs.

DMD says it's all related to FrameLayerBuilder. See the top 9 records in the attached file. Perhaps this stuff is short-lived? That would explain why I haven't seen it come up previously.

Nb: if I apply the patch in bug 1054161, which skips a lot of the frame building stuff for transparent text nodes, then the heap-unclassified spike doesn't happen.

Comment 1

[Nicholas Nethercote [inactive]]

Attachment: DMD output

Comment 2

[Matt Woodrow (:mattwoodrow)]

From a quick look, this appear to be the Layers themselves, the retained data FrameLayerBuilder uses to track changes between paints (DisplayItemGeometry, DisplayItemData, LayerManagerData), and some short lived things used during a paint.

Most of these things are pretty small, which is why it hasn't shown up before.

PDF.js creating thousands of individually transformed elements (each of which requires a new layer manager) means that just the sheer number of these objects adds up to a decent amount of memory.

I expect roc's patch to not build display items for invisible text makes this all go away :)

I'm not sure it's worth adding trackers for all these small items, since it's only rare cases that pages do crazy enough things for it to show up.

Comment 3

[Nicholas Nethercote [inactive]]

> I expect roc's patch to not build display items for invisible text makes
> this all go away :)

It does.

> I'm not sure it's worth adding trackers for all these small items, since
> it's only rare cases that pages do crazy enough things for it to show up.

Fair enough.

Comment 4

[Nathan Froyd [:froydnj]]

(In reply to Matt Woodrow (:mattwoodrow) from comment #2)
> I'm not sure it's worth adding trackers for all these small items, since
> it's only rare cases that pages do crazy enough things for it to show up.

But isn't that one of the great things about adding memory reporters, is that they shine light on pages doing unexpected things?  (I don't know about how these proposed memory reporters might tie in to the memory work the Developer Tools team has been doing, but if the proposed reporters *did* tie in to that reporting, I'd imagine web developers would welcome the idea of being alerted when their approach was memory-hungry...)

Comment 5

[Matt Woodrow (:mattwoodrow)]

(In reply to Nathan Froyd (:froydnj) from comment #4)
> But isn't that one of the great things about adding memory reporters, is
> that they shine light on pages doing unexpected things?  (I don't know about
> how these proposed memory reporters might tie in to the memory work the
> Developer Tools team has been doing, but if the proposed reporters *did* tie
> in to that reporting, I'd imagine web developers would welcome the idea of
> being alerted when their approach was memory-hungry...)

Sure, I just don't think the win is big enough to justify doing the work to implement them.

Comment 6

[Gregory Pappas [:gregp]]

FrameLayerBuilder was removed in bug 1726291