This is things like the name and ID tables on Document, say. See bug 678376 comment 10, where this comes up.

What about this, Boris?

A couple of points about memory reporters in general, which I might as well put here. - In the JS engine, I'm moving towards using moz_malloc_usable_size to measure heap blocks, and falling back to another computation if it returns 0 (as it does on platforms that don't have malloc_usable_size or equivalent). Bug 676732 has a patch that shows how I do this. We need to do this because slop can be 10% or more of total heap usage. So it would be worthwhile starting to do this in all other reporters too. - Another thing I do in bug 676732 is to pass in a pointer to moz_malloc_usable_size to the code that takes the memory measurements. This is necessary in the JS engine because you can't call moz_malloc_usable_size from within it (due to various linking/separation annoyances). But it turns out to be really good for DMD, because I can provide wrapped versions of moz_malloc_usable_size that tell DMD about the reported blocks. I wonder if doing this for other memory reporters is also a good idea. - I suspect we'll want to split DOM reporter at some point by category, and possibly by URI (or something similar). Similar to bug 683149.

Comment on attachment 556815 [details] [diff] [review] Patch v1 A followup to change mSubDocuments to an nsTHashtable might be worthwhile.... > + // TODO: maybe add a SizeOf() method to nsIdentifierMapEntry? Yes; those can be reasonably large. > + size += mRadioGroups.Size(); You probably also want to add a SizeOf() to nsRadioGroupStruct, right? > + // TODO: add mExternalResourceMap? Yes, please file followup! >+ inline PRInt64 SizeOfPLDHashTable(const PLDHashTable* const aHashTable) We have existing code in nsCSSRuleProcessor that could use this too, right? But why do you need the EntryType template anyway? PLDHashTable::entrySize should be the same thing, no? If ops is null, then doing PL_DHASH_TABLE_SIZE is wrong, because it will return random values. In particular, the change to nsHTMLStyleSheet.cpp is wrong too.

It's probably good to use the memory reporter's SizeOfPLDHashTable in nsTHashtable too, because of the ops issue.

Should fix all previous comments. I prefer not to call dom::MemoryReporter::SizeOFPLDHashTable() in nsTHashTable::SizeOf(). I agree that duplicating the code here isn't a good idea. We could probably add a method to pldhash.h to make things easier?

Comment on attachment 558276 [details] [diff] [review] Patch v2 >+++ b/content/base/src/nsContentList.h >+ // Returns the size taken in memory by the object. >+ PRInt64 SizeOf() const { >+ return mElements.Size(); Add a comment that this is OK because no one is instantiating nsBaseContentList directly (so it's not important to count its members here; the subclasses can handle that)? And make it protected to make sure that only subclasses can call it. You'll need to add a SizeOf() on nsSimpleContentList that includes sizeof(*this). >+++ b/content/base/src/nsDocument.cpp >+ChangeCallbackSizeOfEnumerator(nsIdentifierMapEntry::ChangeCallbackEntry* aEntry, So this adds up the size of all the ChangeCallbackEntry objects in the hashtable, but just using sizeof() to compute it.... >+ size += mChangeCallbacks->Size(); And this adds the size of the mChangeCallbacks entry store. Doesn't that double-count the memory for the entries? >+ mChangeCallbacks->EnumerateEntries(ChangeCallbackSizeOfEnumerator, &size); In other words, I don't think we need this enumeration at all. > struct nsRadioGroupStruct >+ PRInt64 SizeOf() const { >+ return sizeof(*this) - sizeof(mRadioButtons) + mRadioButtons.Size(); Hmm... Does .Size() on an nsCOMArray count its own sizeof (unlike the .Size() on an nsTArray)? If not, why is this right? If it does, can we fix it? If not, comments are needed where mRadioButtons is declared pointing out that if its type changes to nsTArray the SizeOf needs to change.... >+++ b/xpcom/glue/nsTHashtable.h >+ return PL_DHASH_TABLE_SIZE(&mTable) * sizeof(mTable.entrySize); That second factor should be just mTable.entrySize, without the sizeof. I haven't double-checked the void array Size() impls yet, pending answers to my questions above.

> Hmm... Does .Size() on an nsCOMArray count its own sizeof (unlike the > .Size() on an nsTArray)? If not, why is this right? If it does, can we fix > it? FWIW, bug 698968 will make things clearer on this front by starting to rename all SizeOf() functions either SizeOfIncludingThis() or SizeOfExcludingThis() (and some classes will have both).

We can reopen this or create a new bug if DMD finds these are significant.