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

RESOLVED FIXED in Firefox 64

Status

()

defect
RESOLVED FIXED
11 months ago
10 months ago

People

(Reporter: bzbarsky, Assigned: mats)

Tracking

(Blocks 1 bug)

unspecified
mozilla64
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox64 fixed)

Details

(Whiteboard: [overhead:noted])

Attachments

(1 attachment)

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.
Posted 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]
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.
Whiteboard: [overhead:noted]
Comment on attachment 9005271 [details] [diff] [review]
fix

r=me
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 mpalmgren@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/108817297fb1
Add memory reporting for nsLineBox::mFrames.  r=bz
https://hg.mozilla.org/mozilla-central/rev/108817297fb1
Status: NEW → RESOLVED
Closed: 10 months ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla64
You need to log in before you can comment on or make changes to this bug.