Closed Bug 1487221 Opened 4 years ago Closed 4 years ago

We don't have memory reporting for nsLineBox::mFrames


(Core :: Layout: Block and Inline, defect)

Not set



Tracking Status
firefox64 --- fixed


(Reporter: bzbarsky, Assigned: MatsPalmgren_bugz)


(Blocks 1 open bug)


(Whiteboard: [overhead:noted])


(1 file)

From a DMD report:

Unreported {
  1 block in heap block record 39 of 6,988
  524,288 bytes (524,288 requested / 0 slop)
  0.11% of the heap (26.94% cumulative)
  0.29% of unreported (73.18% cumulative)
  Allocated at {
    #01: replace_calloc(unsigned long, unsigned long) (DMD.cpp:1282, in libmozglue.dylib)
    #02: PLDHashTable::ChangeTable(int) (PLDHashTable.cpp:0, in XUL)
    #03: PLDHashTable::Add(void const*, std::nothrow_t const&) (PLDHashTable.cpp:0, in XUL)
    #04: PLDHashTable::Add(void const*) (PLDHashTable.cpp:619, in XUL)
    #05: NS_NewLineBox(nsIPresShell*, nsLineBox*, nsIFrame*, int) (nsLineBox.cpp:35, in XUL)
    #06: nsBlockFrame::SplitLine(mozilla::BlockReflowInput&, nsLineLayout&, nsLineList_iterator, nsIFrame*, LineReflowStatus*) (nsLineBox.h:1591, in XUL)
    #07: nsBlockFrame::ReflowInlineFrame(mozilla::BlockReflowInput&, nsLineLayout&, nsLineList_iterator, nsIFrame*, LineReflowStatus*) (nsBlockFrame.cpp:4382, in XUL)
    #08: nsBlockFrame::DoReflowInlineFrames(mozilla::BlockReflowInput&, nsLineLayout&, nsLineList_iterator, nsFlowAreaRect&, int&, nsFloatManager::SavedState*, bool*, LineReflowStatus*, bool) (nsBlockFrame.cpp:4070, in XUL)

That's a single half-a-megabyte allocation for that hashtable.  The same  log  also has 5 128-KiB allocations for this hashtable with slightly different stacks.
Attached patch fixSplinter Review
Not sure if "mLayoutFramePropertiesSize" is the right bucket,
but this data seems similar in nature.  I guess we could add
it to mArenaSizes.mLineBoxes instead, but it didn't seem right
because it's not actually allocated in the pres arena.
Assignee: nobody → mats
Attachment #9005271 - Flags: review?(bzbarsky)
Comment on attachment 9005271 [details] [diff] [review]

Review of attachment 9005271 [details] [diff] [review]:

::: layout/generic/nsLineBox.cpp
@@ +94,5 @@
>    return newLine;
>  }
>  void
> +nsLineBox::AddSizeOfExcludingThis(nsWindowSizes& aSizes) const

It seems to me for classes don't necessarily need nsWindowSizes (completely fall into one bucket and don't use other stuff like pointer table), we usually don't pass the whole nsWindowSizes in. Maybe consider make it a SizeOfExcludingThis(MallocSizeOf).

@@ +97,5 @@
>  void
> +nsLineBox::AddSizeOfExcludingThis(nsWindowSizes& aSizes) const
> +{
> +  if (mFlags.mHasHashedFrames) {
> +    aSizes.mLayoutFramePropertiesSize +=

I guess mLayoutFramePropertiesSize is probably fine, but it might be better to add another bucket maybe called "/layout/other" for this kind of stuff.
Whiteboard: [overhead:noted]
Comment on attachment 9005271 [details] [diff] [review]

Attachment #9005271 - Flags: review?(bzbarsky) → review+
And my apologies for the horrible lag.  I ended up with some unexpected PTO...
No worries, this isn't urgent at all.
Pushed by
Add memory reporting for nsLineBox::mFrames.  r=bz
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla64
You need to log in before you can comment on or make changes to this bug.