Closed Bug 795128 Opened 12 years ago Closed 12 years ago

add memory reporting for attribute nodes and attribute maps

Categories

(Core :: DOM: Core & HTML, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla18

People

(Reporter: Gavin, Assigned: khuey)

References

(Blocks 1 open bug)

Details

(Whiteboard: [MemShrink])

Attachments

(1 file)

These aren't hooked up, and apparently some sites use them a lot.
The logs in bug 701443 had 28,000 attribute maps in them.
Blocks: 701443
Whiteboard: [MemShrink]
Assignee: nobody → khuey
Attached patch Patch (deleted) — Splinter Review
Attachment #665687 - Flags: review?(continuation)
Comment on attachment 665687 [details] [diff] [review] Patch Review of attachment 665687 [details] [diff] [review]: ----------------------------------------------------------------- Thanks for following the existing style, e.g. documenting fields that you're not measuring.
Attachment #665687 - Flags: feedback+
I had good examples to steal from ;-)
Comment on attachment 665687 [details] [diff] [review] Patch Review of attachment 665687 [details] [diff] [review]: ----------------------------------------------------------------- Looks okay to me, but somebody else should review it, given that I know nothing about what an attribute or an attribute map is. Sorry. ::: content/base/src/FragmentOrElement.cpp @@ +617,5 @@ > + // - mChildrenList > + // - mClassList > + > + // The following members are not measured: > + // - mBindingParent / mControllers: because they're non-owning Extra spaces between "they're" and "non-owning".
Attachment #665687 - Flags: review?(continuation) → feedback+
Comment on attachment 665687 [details] [diff] [review] Patch >diff --git a/content/base/public/FragmentOrElement.h b/content/base/public/FragmentOrElement.h >--- a/content/base/public/FragmentOrElement.h >+++ b/content/base/public/FragmentOrElement.h >@@ -343,16 +343,18 @@ public: > { > public: > nsDOMSlots(); > virtual ~nsDOMSlots(); > > void Traverse(nsCycleCollectionTraversalCallback &cb, bool aIsXUL); > void Unlink(bool aIsXUL); > >+ size_t SizeOfIncludingThis(nsMallocSizeOfFun aMallocSizeOf) const; >+ > /** > * The .style attribute (an interface that forwards to the actual > * style rules) > * @see nsGenericHTMLElement::GetStyle > */ > nsCOMPtr<nsICSSDeclaration> mStyle; > > /** >diff --git a/content/base/src/FragmentOrElement.cpp b/content/base/src/FragmentOrElement.cpp >--- a/content/base/src/FragmentOrElement.cpp >+++ b/content/base/src/FragmentOrElement.cpp >@@ -593,16 +593,40 @@ FragmentOrElement::nsDOMSlots::Unlink(bo > NS_IF_RELEASE(mControllers); > mChildrenList = nullptr; > if (mClassList) { > mClassList->DropReference(); > mClassList = nullptr; > } > } > >+size_t >+FragmentOrElement::nsDOMSlots::SizeOfIncludingThis(nsMallocSizeOfFun aMallocSizeOf) const >+{ >+ size_t n = aMallocSizeOf(this); >+ >+ if (mAttributeMap) { >+ n += mAttributeMap->SizeOfIncludingThis(aMallocSizeOf); >+ } >+ >+ // Measurement of the following members may be added later if DMD finds it is >+ // worthwhile: >+ // - Superclass members (nsINode::nsSlots) >+ // - mStyle >+ // - mDataSet >+ // - mSMILOverrideStyle >+ // - mSMILOverrideStyleRule >+ // - mChildrenList >+ // - mClassList >+ >+ // The following members are not measured: >+ // - mBindingParent / mControllers: because they're non-owning >+ return n; >+} >+ > FragmentOrElement::FragmentOrElement(already_AddRefed<nsINodeInfo> aNodeInfo) > : nsIContent(aNodeInfo) > { > } > > FragmentOrElement::~FragmentOrElement() > { > NS_PRECONDITION(!IsInDoc(), >@@ -1933,11 +1957,19 @@ FragmentOrElement::FireNodeRemovedForChi > child = child->GetNextSibling()) { > nsContentUtils::MaybeFireNodeRemoved(child, this, doc); > } > } > > size_t > FragmentOrElement::SizeOfExcludingThis(nsMallocSizeOfFun aMallocSizeOf) const > { >- return nsIContent::SizeOfExcludingThis(aMallocSizeOf) + >- mAttrsAndChildren.SizeOfExcludingThis(aMallocSizeOf); >+ size_t n = 0; >+ n += nsIContent::SizeOfExcludingThis(aMallocSizeOf); >+ n += mAttrsAndChildren.SizeOfExcludingThis(aMallocSizeOf); >+ >+ nsDOMSlots* slots = GetExistingDOMSlots(); >+ if (slots) { >+ n += slots->SizeOfIncludingThis(aMallocSizeOf); >+ } >+ >+ return n; > } >diff --git a/content/base/src/nsDOMAttributeMap.cpp b/content/base/src/nsDOMAttributeMap.cpp >--- a/content/base/src/nsDOMAttributeMap.cpp >+++ b/content/base/src/nsDOMAttributeMap.cpp >@@ -496,8 +496,28 @@ nsDOMAttributeMap::Count() const > } > > uint32_t > nsDOMAttributeMap::Enumerate(AttrCache::EnumReadFunction aFunc, > void *aUserArg) const > { > return mAttributeCache.EnumerateRead(aFunc, aUserArg); > } >+ >+size_t >+AttrCacheSizeEnumerator(const nsAttrKey& aKey, >+ const nsRefPtr<nsDOMAttribute>& aValue, >+ nsMallocSizeOfFun aMallocSizeOf, >+ void* aUserArg) >+{ >+ return aMallocSizeOf(aValue.get()); >+} >+ >+size_t >+nsDOMAttributeMap::SizeOfIncludingThis(nsMallocSizeOfFun aMallocSizeOf) const >+{ >+ size_t n = aMallocSizeOf(this); >+ n += mAttributeCache.SizeOfExcludingThis(AttrCacheSizeEnumerator, >+ aMallocSizeOf); >+ >+ // NB: mContent is non-owning and thus not counted. >+ return n; >+} >diff --git a/content/base/src/nsDOMAttributeMap.h b/content/base/src/nsDOMAttributeMap.h >--- a/content/base/src/nsDOMAttributeMap.h >+++ b/content/base/src/nsDOMAttributeMap.h >@@ -154,16 +154,18 @@ public: > } > #endif > > return static_cast<nsDOMAttributeMap*>(aSupports); > } > > NS_DECL_CYCLE_COLLECTION_CLASS(nsDOMAttributeMap) > >+ size_t SizeOfIncludingThis(nsMallocSizeOfFun aMallocSizeOf) const; >+ > private: > Element *mContent; // Weak reference > > /** > * Cache of nsDOMAttributes. > */ > AttrCache mAttributeCache; >
Attachment #665687 - Flags: review?(bugs) → review+
Status: NEW → RESOLVED
Closed: 12 years ago
Flags: in-testsuite-
Resolution: --- → FIXED
Target Milestone: --- → mozilla18
Component: DOM → DOM: Core & HTML
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: