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

Status: RESOLVED FIXED

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

Description

[Boris Zbarsky [:bzbarsky]]

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.

Comment 1

[Mats Palmgren (inactive)]

Attachment: fix

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.

Comment 2

[Xidorn Quan [:xidorn] UTC+11]

Comment on attachment 9005271 [details] [diff] [review]
fix

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.

Comment 3

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 9005271 [details] [diff] [review]
fix

r=me

Comment 4

[Boris Zbarsky [:bzbarsky]]

And my apologies for the horrible lag.  I ended up with some unexpected PTO...

Comment 5

[Mats Palmgren (inactive)]

No worries, this isn't urgent at all.

Comment 6

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/108817297fb1
Add memory reporting for nsLineBox::mFrames.  r=bz

Comment 7

[Narcis Beleuzu [:NarcisB]]

https://hg.mozilla.org/mozilla-central/rev/108817297fb1