Need memory reporter for FrameLayerBuilder

NEW
Unassigned

Status

()

4 years ago
4 years ago

People

(Reporter: njn, Unassigned)

Tracking

(Blocks: 1 bug)

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [MemShrink:P2])

Attachments

(1 attachment)

(Reporter)

Description

4 years ago
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.
(Reporter)

Comment 1

4 years ago
Created attachment 8473712 [details]
DMD output
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.
(Reporter)

Comment 3

4 years ago
> 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.
(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...)
(Reporter)

Updated

4 years ago
Whiteboard: [MemShrink] → [MemShrink:P2]
(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.
You need to log in before you can comment on or make changes to this bug.