Closed Bug 747508 Opened 12 years ago Closed 12 years ago

high {Frame,ns}PropertyTable usage in DMD stacks for HTML5 spec

Categories

(Core :: Layout, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla15

People

(Reporter: froydnj, Assigned: froydnj)

References

(Blocks 1 open bug, )

Details

(Whiteboard: [MemShrink:P2])

Attachments

(1 file)

Viewing the single-page HTML5 spec (~1.1MB of HTML, never mind styles and whatnot; see the linked URL) is rather slow (bug 475606).  Looking at about:memory after quiescence, there's a high percentage of heap-unclassified (25% here on my Linux/x86-64 box).  Going through with DMD, the top hits look like:

 ==28808== Unreported: 1 block(s) in record 1 of 15608
==28808==  16,781,312 bytes (16,777,220 requested / 4,092 slop)
==28808==  7.60% of the heap (7.60% cumulative unreported)
==28808==    at 0x4C27193: malloc (vg_replace_malloc.c:263)
==28808==    by 0x6A5A1C9: moz_malloc (mozalloc.cpp:97)
==28808==    by 0x9C4FCED: PL_DHashAllocTable (pldhash.cpp:117)
==28808==    by 0x9C50710: ChangeTable(PLDHashTable*, int) (pldhash.cpp:560)
==28808==    by 0x9C50AB5: PL_DHashTableOperate (pldhash.cpp:645)
==28808==    by 0x876D3BA: nsTHashtable<mozilla::FramePropertyTable::Entry>::PutEntry(nsIFrame*) (nsTHashtable.h:198)
==28808==    by 0x876CA9F: mozilla::FramePropertyTable::Set(nsIFrame*, mozilla::FramePropertyDescriptor const*, void*) (FramePropertyTable.cpp:52)
==28808==    by 0x8831384: nsBidiPresUtils::ResolveParagraph(nsBlockFrame*, BidiParagraphData*) (nsBidiPresUtils.cpp:757)
==28808==    by 0x8830DE4: nsBidiPresUtils::Resolve(nsBlockFrame*) (nsBidiPresUtils.cpp:634)
==28808==    by 0x887AC5A: nsBlockFrame::ResolveBidi() (nsBlockFrame.cpp:7139)
==28808==    by 0x8866F77: nsBlockFrame::Reflow(nsPresContext*, nsHTMLReflowMetrics&, nsHTMLReflowState const&, unsigned int&) (nsBlockFrame.cpp:1028)
==28808==    by 0x887C246: nsBlockReflowContext::ReflowBlock(nsRect const&, bool, nsCollapsingMargin&, int, bool, nsLine
Box*, nsHTMLReflowState&, unsigned int&, nsBlockReflowState&) nsBlockReflowContext.cpp:295)
==28808== 
==28808== Unreported: 111,027 block(s) in record 2 of 15608
==28808==  8,882,160 bytes (7,993,944 requested / 888,216 slop)
==28808==  4.02% of the heap (11.62% cumulative unreported)
==28808==    at 0x4C27193: malloc (vg_replace_malloc.c:263)
==28808==    by 0x6A5A16B: moz_xmalloc (mozalloc.cpp:87)
==28808==    by 0x84CDABA: nsTArrayInfallibleAllocator::Malloc(unsigned long) (nsTArray.h:88)
==28808==    by 0x84D4858: nsTArray_base<nsTArrayDefaultAllocator>::EnsureCapacity(unsigned int, unsigned int) (nsTArray-inl.h:151)
==28808==    by 0x876D74C: nsTArray<mozilla::FramePropertyTable::PropertyValue, nsTArrayDefaultAllocator>::SetCapacity(unsigned int) (nsTArray.h:1038)
==28808==    by 0x876D3FF: nsTArray<mozilla::FramePropertyTable::PropertyValue, nsTArrayDefaultAllocator>::nsTArray(unsigned int) (nsTArray.h:472)
==28808==    by 0x876CB78: mozilla::FramePropertyTable::Set(nsIFrame*, mozilla::FramePropertyDescriptor const*, void*) (FramePropertyTable.cpp:75)
==28808==    by 0x88313B7: nsBidiPresUtils::ResolveParagraph(nsBlockFrame*, BidiParagraphData*) (nsBidiPresUtils.cpp:759)
==28808==    by 0x8830DE4: nsBidiPresUtils::Resolve(nsBlockFrame*) (nsBidiPresUtils.cpp:634)
==28808==    by 0x887AC5A: nsBlockFrame::ResolveBidi() (nsBlockFrame.cpp:7139)
==28808==    by 0x8866F77: nsBlockFrame::Reflow(nsPresContext*, nsHTMLReflowMetrics&, nsHTMLReflowState const&, unsigned int&) (nsBlockFrame.cpp:1028)
==28808==    by 0x887C246: nsBlockReflowContext::ReflowBlock(nsRect const&, bool, nsCollapsingMargin&, int, bool, nsLineBox*, nsHTMLReflowState&, unsigned int&, nsBlockReflowState&) (nsBlockReflowContext.cpp:295)
==28808== 
==28808== Unreported: 1 block(s) in record 3 of 15608
==28808==  6,295,552 bytes (6,291,460 requested / 4,092 slop)
==28808==  2.85% of the heap (14.48% cumulative unreported)
==28808==    at 0x4C27193: malloc (vg_replace_malloc.c:263)
==28808==    by 0x6A5A1C9: moz_malloc (mozalloc.cpp:97)
==28808==    by 0x9C4FCED: PL_DHashAllocTable (pldhash.cpp:117)
==28808==    by 0x9C50710: ChangeTable(PLDHashTable*, int) (pldhash.cpp:560)
==28808==    by 0x9C50AB5: PL_DHashTableOperate (pldhash.cpp:645)
==28808==    by 0x8BCEACC: nsPropertyTable::SetPropertyInternal(nsPropertyOwner, nsIAtom*, void*, void (*)(void*, nsIAtom*, void*, void*), void*, bool, void**) (nsPropertyTable.cpp:260)
==28808==    by 0x8B8A96F: nsPropertyTable::SetProperty(nsPropertyOwner, nsIAtom*, void*, void (*)(void*, nsIAtom*, void*, void*), void*, bool, void**) (nsPropertyTable.h:137)
==28808==    by 0x8B8E5BB: nsINode::SetProperty(unsigned short, nsIAtom*, void*, void (*)(void*, nsIAtom*, void*, void*), bool, void**) (nsGenericElement.cpp:231)
==28808==    by 0x8770742: nsINode::SetProperty(nsIAtom*, void*, void (*)(void*, nsIAtom*, void*, void*), bool) (nsINode.h:634)
==28808==    by 0x891157B: nsTextFrame::GetInFlowContentLength() (nsTextFrameThebes.cpp:633)
==28808==    by 0x8925760: nsTextFrame::ReflowText(nsLineLayout&, int, nsRenderingContext*, bool, nsHTMLReflowMetrics&, unsigned int&) (nsTextFrameThebes.cpp:7258)
==28808==    by 0x88E5F25: nsLineLayout::ReflowFrame(nsIFrame*, unsigned int&, nsHTMLReflowMetrics*, bool&) (nsLineLayout.cpp:872)
==28808== 
==28808== Unreported: 28,475 block(s) in record 4 of 15608
==28808==  2,278,000 bytes (2,050,200 requested / 227,800 slop)
==28808==  1.03% of the heap (15.51% cumulative unreported)
==28808==    at 0x4C27193: malloc (vg_replace_malloc.c:263)
==28808==    by 0x6A5A16B: moz_xmalloc (mozalloc.cpp:87)
==28808==    by 0x84CDABA: nsTArrayInfallibleAllocator::Malloc(unsigned long) (nsTArray.h:88)
==28808==    by 0x84D4858: nsTArray_base<nsTArrayDefaultAllocator>::EnsureCapacity(unsigned int, unsigned int) (nsTArray
-inl.h:151)
==28808==    by 0x876D74C: nsTArray<mozilla::FramePropertyTable::PropertyValue, nsTArrayDefaultAllocator>::SetCapacity(unsigned int) (nsTArray.h:1038)
==28808==    by 0x876D3FF: nsTArray<mozilla::FramePropertyTable::PropertyValue, nsTArrayDefaultAllocator>::nsTArray(unsigned int) (nsTArray.h:472)
==28808==    by 0x876CB78: mozilla::FramePropertyTable::Set(nsIFrame*, mozilla::FramePropertyDescriptor const*, void*) (FramePropertyTable.cpp:75)
==28808==    by 0x88313B7: nsBidiPresUtils::ResolveParagraph(nsBlockFrame*, BidiParagraphData*) (nsBidiPresUtils.cpp:759)
==28808==    by 0x883240A: nsBidiPresUtils::ResolveParagraphWithinBlock(nsBlockFrame*, BidiParagraphData*) (nsBidiPresUtils.cpp:1173)
==28808==    by 0x883223C: nsBidiPresUtils::TraverseFrames(nsBlockFrame*, nsBlockInFlowLineIterator*, nsIFrame*, BidiPar
agraphData*) (nsBidiPresUtils.cpp:1116)
==28808==    by 0x8830D88: nsBidiPresUtils::Resolve(nsBlockFrame*) (nsBidiPresUtils.cpp:627)
==28808==    by 0x887AC5A: nsBlockFrame::ResolveBidi() (nsBlockFrame.cpp:7139)
==28808==
==28808== Unreported: 16,090 block(s) in record 5 of 15608
==28808==  1,287,200 bytes (1,158,480 requested / 128,720 slop)
==28808==  0.58% of the heap (16.09% cumulative unreported)
==28808==    at 0x4C27193: malloc (vg_replace_malloc.c:263)
==28808==    by 0x6A5A16B: moz_xmalloc (mozalloc.cpp:87)
==28808==    by 0x84CDABA: nsTArrayInfallibleAllocator::Malloc(unsigned long) (nsTArray.h:88)
==28808==    by 0x84D4858: nsTArray_base<nsTArrayDefaultAllocator>::EnsureCapacity(unsigned int, unsigned int) (nsTArray-inl.h:151)
==28808==    by 0x876D74C: nsTArray<mozilla::FramePropertyTable::PropertyValue, nsTArrayDefaultAllocator>::SetCapacity(unsigned int) (nsTArray.h:1038)
==28808==    by 0x876D3FF: nsTArray<mozilla::FramePropertyTable::PropertyValue, nsTArrayDefaultAllocator>::nsTArray(unsigned int) (nsTArray.h:472)
==28808==    by 0x876CB78: mozilla::FramePropertyTable::Set(nsIFrame*, mozilla::FramePropertyDescriptor const*, void*) (FramePropertyTable.cpp:75)
==28808==    by 0x88313B7: nsBidiPresUtils::ResolveParagraph(nsBlockFrame*, BidiParagraphData*) (nsBidiPresUtils.cpp:759)
==28808==    by 0x8830DE4: nsBidiPresUtils::Resolve(nsBlockFrame*) (nsBidiPresUtils.cpp:634)
==28808==    by 0x887AC5A: nsBlockFrame::ResolveBidi() (nsBlockFrame.cpp:7139)
==28808==    by 0x8865D42: nsBlockFrame::GetMinWidth(nsRenderingContext*) (nsBlockFrame.cpp:721)
==28808==    by 0x87EF324: nsLayoutUtils::IntrinsicForContainer(nsRenderingContext*, nsIFrame*, nsLayoutUtils::IntrinsicWidthType) (nsLayoutUtils.cpp:2399)

There are more stacks with similarities to the FramePropertyTable stacks above in the top 20; a notable exception is:

==28808== Unreported: 12,464 block(s) in record 10 of 15608
==28808==  997,120 bytes (897,408 requested / 99,712 slop)
==28808==  0.45% of the heap (18.52% cumulative unreported)
==28808==    at 0x4C27193: malloc (vg_replace_malloc.c:263)
==28808==    by 0x6A5A16B: moz_xmalloc (mozalloc.cpp:87)
==28808==    by 0x84CDABA: nsTArrayInfallibleAllocator::Malloc(unsigned long) (nsTArray.h:88)
==28808==    by 0x84D4858: nsTArray_base<nsTArrayDefaultAllocator>::EnsureCapacity(unsigned int, unsigned int) (nsTArray-inl.h:151)
==28808==    by 0x876D74C: nsTArray<mozilla::FramePropertyTable::PropertyValue, nsTArrayDefaultAllocator>::SetCapacity(unsigned int) (nsTArray.h:1038)
==28808==    by 0x876D3FF: nsTArray<mozilla::FramePropertyTable::PropertyValue, nsTArrayDefaultAllocator>::nsTArray(unsigned int) (nsTArray.h:472)
==28808==    by 0x876CB78: mozilla::FramePropertyTable::Set(nsIFrame*, mozilla::FramePropertyDescriptor const*, void*) (FramePropertyTable.cpp:75)
==28808==    by 0x875E2CA: mozilla::FrameProperties::Set(mozilla::FramePropertyDescriptor const*, void*) const (FramePropertyTable.h:256)
==28808==    by 0x88A437F: nsIFrame::GetOverflowAreasProperty() (nsFrame.cpp:6616)
==28808==    by 0x88A4700: nsIFrame::SetOverflowAreas(nsOverflowAreas const&) (nsFrame.cpp:6673)
==28808==    by 0x88A531E: nsIFrame::FinishAndStoreOverflow(nsOverflowAreas&, nsSize) (nsFrame.cpp:6832)
==28808==    by 0x88470D9: nsIFrame::FinishAndStoreOverflow(nsHTMLReflowMetrics*) (nsIFrame.h:2322)

I was going to say that FramePropertyTable is not properly measuring its memory consumption, but after reflecting on the comment:

http://mxr.mozilla.org/mozilla-central/source/layout/base/FramePropertyTable.h#186

I think that it is.  The specific FramePropertyTable in those stacks coming through nsBidiPresUtils should be measured here:

http://mxr.mozilla.org/mozilla-central/source/layout/base/nsPresContext.cpp#2365

so I'm not really sure what's going on.
Oh, and:

==28808== Unreported: 11,294 block(s) in record 12 of 15608
==28808==  903,520 bytes (813,168 requested / 90,352 slop)
==28808==  0.40% of the heap (18.93% cumulative unreported)
==28808==    at 0x4C27193: malloc (vg_replace_malloc.c:263)
==28808==    by 0x6A5A16B: moz_xmalloc (mozalloc.cpp:87)
==28808==    by 0x84CDABA: nsTArrayInfallibleAllocator::Malloc(unsigned long) (nsTArray.h:88)
==28808==    by 0x84D4858: nsTArray_base<nsTArrayDefaultAllocator>::EnsureCapacity(unsigned int, unsigned int) (nsTArray-inl.h:151)
==28808==    by 0x876D74C: nsTArray<mozilla::FramePropertyTable::PropertyValue, nsTArrayDefaultAllocator>::SetCapacity(unsigned int) (nsTArray.h:1038)
==28808==    by 0x876D3FF: nsTArray<mozilla::FramePropertyTable::PropertyValue, nsTArrayDefaultAllocator>::nsTArray(unsigned int) (nsTArray.h:472)
==28808==    by 0x876CB78: mozilla::FramePropertyTable::Set(nsIFrame*, mozilla::FramePropertyDescriptor const*, void*) (FramePropertyTable.cpp:75)
==28808==    by 0x88313B7: nsBidiPresUtils::ResolveParagraph(nsBlockFrame*, BidiParagraphData*) (nsBidiPresUtils.cpp:759)
==28808==    by 0x883240A: nsBidiPresUtils::ResolveParagraphWithinBlock(nsBlockFrame*, BidiParagraphData*) (nsBidiPresUtils.cpp:1173)
==28808==    by 0x88320C1: nsBidiPresUtils::TraverseFrames(nsBlockFrame*, nsBlockInFlowLineIterator*, nsIFrame*, BidiParagraphData*) (nsBidiPresUtils.cpp:1082)
==28808==    by 0x8830D88: nsBidiPresUtils::Resolve(nsBlockFrame*) (nsBidiPresUtils.cpp:627)
==28808==    by 0x887AC5A: nsBlockFrame::ResolveBidi() (nsBlockFrame.cpp:7139)
==28808== 
==28808== Unreported: 9,501 block(s) in record 14 of 15608
==28808==  760,080 bytes (684,072 requested / 76,008 slop)
==28808==  0.34% of the heap (19.27% cumulative unreported)
==28808==    at 0x4C27193: malloc (vg_replace_malloc.c:263)
==28808==    by 0x6A5A16B: moz_xmalloc (mozalloc.cpp:87)
==28808==    by 0x84CDABA: nsTArrayInfallibleAllocator::Malloc(unsigned long) (nsTArray.h:88)
==28808==    by 0x84D4858: nsTArray_base<nsTArrayDefaultAllocator>::EnsureCapacity(unsigned int, unsigned int) (nsTArray-inl.h:151)
==28808==    by 0x876D74C: nsTArray<mozilla::FramePropertyTable::PropertyValue, nsTArrayDefaultAllocator>::SetCapacity(unsigned int) (nsTArray.h:1038)
==28808==    by 0x876D3FF: nsTArray<mozilla::FramePropertyTable::PropertyValue, nsTArrayDefaultAllocator>::nsTArray(unsigned int) (nsTArray.h:472)
==28808==    by 0x876CB78: mozilla::FramePropertyTable::Set(nsIFrame*, mozilla::FramePropertyDescriptor const*, void*) (FramePropertyTable.cpp:75)
==28808==    by 0x891B76B: nsContinuingTextFrame::Init(nsIContent*, nsIFrame*, nsIFrame*) (nsTextFrameThebes.cpp:3981)
==28808==    by 0x8787E2C: nsCSSFrameConstructor::CreateContinuingFrame(nsPresContext*, nsIFrame*, nsIFrame*, nsIFrame**, bool) (nsCSSFrameConstructor.cpp:8444)
==28808==    by 0x8870558: nsBlockFrame::CreateContinuationFor(nsBlockReflowState&, nsLineBox*, nsIFrame*, bool&) (nsBlockFrame.cpp:3969)
==28808==    by 0x887032B: nsBlockFrame::ReflowInlineFrame(nsBlockReflowState&, nsLineLayout&, nsLineList_iterator, nsIFrame*, LineReflowStatus*) (nsBlockFrame.cpp:3934)
==28808==    by 0x886F352: nsBlockFrame::DoReflowInlineFrames(nsBlockReflowState&, nsLineLayout&, nsLineList_iterator, nsFlowAreaRect&, int&, nsFloatManager::SavedState*, bool*, LineReflowStatus*, bool) (nsBlockFrame.cpp:3627)
==28808== 
==28808== Unreported: 5,511 block(s) in record 15 of 15608
==28808==  716,800 bytes (716,800 requested / 0 slop)
==28808==  0.32% of the heap (19.59% cumulative unreported)
==28808==    at 0x4C2728B: realloc (vg_replace_malloc.c:632)
==28808==    by 0x6A5A28A: moz_xrealloc (mozalloc.cpp:119)
==28808==    by 0x84CDADF: nsTArrayInfallibleAllocator::Realloc(void*, unsigned long) (nsTArray.h:92)
==28808==    by 0x84D49D1: nsTArray_base<nsTArrayDefaultAllocator>::EnsureCapacity(unsigned int, unsigned int) (nsTArray-inl.h:199)
==28808==    by 0x876D786: mozilla::FramePropertyTable::PropertyValue* nsTArray<mozilla::FramePropertyTable::PropertyValue, nsTArrayDefaultAllocator>::AppendElements<mozilla::FramePropertyTable::PropertyValue>(mozilla::FramePropertyTable::PropertyValue const*, unsigned int) (nsTArray.h:899)
==28808==    by 0x876D429: mozilla::FramePropertyTable::PropertyValue* nsTArray<mozilla::FramePropertyTable::PropertyValue, nsTArrayDefaultAllocator>::AppendElement<mozilla::FramePropertyTable::PropertyValue>(mozilla::FramePropertyTable::PropertyValue const&) (nsTArray.h:916)
==28808==    by 0x876CC30: mozilla::FramePropertyTable::Set(nsIFrame*, mozilla::FramePropertyDescriptor const*, void*) (FramePropertyTable.cpp:89)
==28808==    by 0x875E2CA: mozilla::FrameProperties::Set(mozilla::FramePropertyDescriptor const*, void*) const (FramePropertyTable.h:256)
==28808==    by 0x88A437F: nsIFrame::GetOverflowAreasProperty() (nsFrame.cpp:6616)
==28808==    by 0x88A4700: nsIFrame::SetOverflowAreas(nsOverflowAreas const&) (nsFrame.cpp:6673)
==28808==    by 0x88A531E: nsIFrame::FinishAndStoreOverflow(nsOverflowAreas&, nsSize) (nsFrame.cpp:6832)
==28808==    by 0x88470D9: nsIFrame::FinishAndStoreOverflow(nsHTMLReflowMetrics*) (nsIFrame.h:2322)
Whiteboard: [MemShrink]
Blocks: DarkMatter
Whiteboard: [MemShrink] → [MemShrink:P2]
Attached patch patchSplinter Review
Well, we were counting FramePropertyTables all right...but we were never counting the nsPresContext they were attached to.

This patch fixes that oversight.  I don't know whether it's appropriate to give prescontexts their own leaf in the tree, but there's no layout-general leaf to put them in otherwise.

On the HTML5 spec testcase, prescontexts account for 13%+ of the total heap; heap-unclassified is now down to 14%.  I haven't DMD'd to see what else is lurking in that 14%; I suspect there's not many big wins left...
Attachment #621238 - Flags: feedback?(n.nethercote)
It looks like this is a regression after bug 730181 (part 1) was committed; we used to count the prescontext in with the arenas (which seems odd, but hey...).
Comment on attachment 621238 [details] [diff] [review]
patch

Review of attachment 621238 [details] [diff] [review]:
-----------------------------------------------------------------

Well there you go.  I assume you've run a build with your patch applied under DMD to check there's no double-reporting?
Attachment #621238 - Flags: feedback?(n.nethercote) → feedback+
Comment on attachment 621238 [details] [diff] [review]
patch

(In reply to Nicholas Nethercote [:njn] from comment #4)
> Well there you go.  I assume you've run a build with your patch applied
> under DMD to check there's no double-reporting?

Yes, no double-reporting.

Requesting formal r+; I am unsure of whether this sort of thing needs a layout peer to approve.  Please bounce r? around appropriately.
Attachment #621238 - Flags: review?(n.nethercote)
Comment on attachment 621238 [details] [diff] [review]
patch

Review of attachment 621238 [details] [diff] [review]:
-----------------------------------------------------------------

No need for a review from a layout peer, IMO;  this is all code that either I wrote or I was last to modify.

Thanks!  It's nice to have someone else using DMD :)

::: dom/base/nsWindowMemoryReporter.cpp
@@ +210,4 @@
>           "tree, within a window.");
>    aWindowTotalSizes->mLayoutTextRuns += windowSizes.mLayoutTextRuns;
>  
> +  REPORT("/layout/presContexts", windowSizes.mLayoutPresContext,

Can you change this to "/layout/pres-contexts" for consistency with all the other memory reporters?
Attachment #621238 - Flags: review?(n.nethercote) → review+
https://hg.mozilla.org/integration/mozilla-inbound/rev/a607eec50772
Assignee: nobody → nfroyd
Status: NEW → ASSIGNED
Target Milestone: --- → mozilla15
https://hg.mozilla.org/mozilla-central/rev/a607eec50772
Status: ASSIGNED → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
I just realized something that I should have caught in my review.  In about:memory's "Other Measurements" section we have these:

    800,960 B ── window-objects-layout-arenas
  1,934,224 B ── window-objects-layout-style-sets
        352 B ── window-objects-layout-text-runs

which are the sums of all the windows' individual entries.  It would be nice to have a window-objects-layout-pres-contexts entry too.  nfroyd, are you happy to do that in a follow-up?  Should be pretty easy.
Blocks: 754241
You need to log in before you can comment on or make changes to this bug.