Note: There are a few cases of duplicates in user autocompletion which are being worked on.

add memory reporting for attribute nodes and attribute maps

RESOLVED FIXED in mozilla18

Status

()

Core
DOM
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: Gavin, Assigned: khuey)

Tracking

(Blocks: 1 bug)

Trunk
mozilla18
Points:
---
Dependency tree / graph
Bug Flags:
in-testsuite -

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [MemShrink])

Attachments

(1 attachment)

These aren't hooked up, and apparently some sites use them a lot.
Blocks: 563700
The logs in bug 701443 had 28,000 attribute maps in them.
Blocks: 701443

Updated

5 years ago
Whiteboard: [MemShrink]
Assignee: nobody → khuey
Created attachment 665687 [details] [diff] [review]
Patch
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+
Attachment #665687 - Flags: review?(bugs)

Comment 6

5 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+
https://hg.mozilla.org/integration/mozilla-inbound/rev/96f3db193c75
https://hg.mozilla.org/mozilla-central/rev/96f3db193c75
Status: NEW → RESOLVED
Last Resolved: 5 years ago
Flags: in-testsuite-
Resolution: --- → FIXED
Target Milestone: --- → mozilla18
You need to log in before you can comment on or make changes to this bug.