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)
Core
Layout
Tracking
()
RESOLVED
FIXED
mozilla15
People
(Reporter: froydnj, Assigned: froydnj)
References
(Blocks 1 open bug, )
Details
(Whiteboard: [MemShrink:P2])
Attachments
(1 file)
3.92 KB,
patch
|
n.nethercote
:
review+
n.nethercote
:
feedback+
|
Details | Diff | Splinter Review |
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.
Assignee | ||
Comment 1•12 years ago
|
||
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)
Updated•12 years ago
|
Whiteboard: [MemShrink]
Updated•12 years ago
|
Blocks: DarkMatter
Whiteboard: [MemShrink] → [MemShrink:P2]
Assignee | ||
Comment 2•12 years ago
|
||
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)
Assignee | ||
Comment 3•12 years ago
|
||
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 4•12 years ago
|
||
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+
Assignee | ||
Comment 5•12 years ago
|
||
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 6•12 years ago
|
||
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+
Assignee | ||
Comment 7•12 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/a607eec50772
Assignee: nobody → nfroyd
Status: NEW → ASSIGNED
Target Milestone: --- → mozilla15
Comment 8•12 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/a607eec50772
Status: ASSIGNED → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
Comment 9•12 years ago
|
||
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.
You need to log in
before you can comment on or make changes to this bug.
Description
•