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)
Core
DOM: Core & HTML
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.
Updated•12 years ago
|
Blocks: DarkMatter
Reporter | ||
Updated•12 years ago
|
Assignee: nobody → khuey
Assignee | ||
Comment 2•12 years ago
|
||
Attachment #665687 -
Flags: review?(continuation)
Comment 3•12 years ago
|
||
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+
Assignee | ||
Comment 4•12 years ago
|
||
I had good examples to steal from ;-)
Comment 5•12 years ago
|
||
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+
Assignee | ||
Updated•12 years ago
|
Attachment #665687 -
Flags: review?(bugs)
Comment 6•12 years ago
|
||
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+
Assignee | ||
Comment 7•12 years ago
|
||
Comment 8•12 years ago
|
||
Status: NEW → RESOLVED
Closed: 12 years ago
Flags: in-testsuite-
Resolution: --- → FIXED
Target Milestone: --- → mozilla18
Updated•6 years ago
|
Component: DOM → DOM: Core & HTML
You need to log in
before you can comment on or make changes to this bug.
Description
•