Closed
Bug 1054321
Opened 10 years ago
Closed 1 year ago
Need memory reporter for FrameLayerBuilder
Categories
(Core :: Layout: Images, Video, and HTML Frames, defect)
Core
Layout: Images, Video, and HTML Frames
Tracking
()
RESOLVED
INVALID
People
(Reporter: n.nethercote, Unassigned)
References
(Blocks 1 open bug)
Details
(Whiteboard: [MemShrink:P2])
Attachments
(1 file)
370.29 KB,
text/plain
|
Details |
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•10 years ago
|
||
Comment 2•10 years ago
|
||
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•10 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.
Comment 4•10 years ago
|
||
(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•10 years ago
|
Whiteboard: [MemShrink] → [MemShrink:P2]
Comment 5•10 years ago
|
||
(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.
Updated•6 years ago
|
Product: Core → Core Graveyard
Assignee | ||
Updated•6 years ago
|
Component: Layout: HTML Frames → Layout: Images
Product: Core Graveyard → Core
Updated•2 years ago
|
Severity: normal → S3
Comment 6•1 year ago
|
||
FrameLayerBuilder was removed in bug 1726291
Status: NEW → RESOLVED
Closed: 1 year ago
Resolution: --- → INVALID
You need to log in
before you can comment on or make changes to this bug.
Description
•