I attempted to DMD firefox with the given URL and got quite a number of double reports: ==14443== Double report of heap block 0x3F62BE20: ==14443== Allocated ==14443== at 0x4C284AF: malloc (vg_replace_malloc.c:267) ==14443== by 0x74630D8: moz_xmalloc (mozalloc.cpp:54) ==14443== by 0x7F6E76C: nsCSSStyleSheet::nsCSSStyleSheet() (mozalloc.h:200) ==14443== by 0x7F6E7B2: NS_NewCSSStyleSheet(nsCSSStyleSheet**) (nsCSSStyleSheet.cpp:2158) ==14443== by 0x7F46304: mozilla::css::Loader::CreateSheet(nsIURI*, nsIContent*, nsIPrincipal*, bool, bool, nsAString_internal const&, mozilla::css::StyleSheetState&, bool*, nsCSSStyleSheet**) (Loader.cpp:1202) ==14443== by 0x7F483A1: mozilla::css::Loader::LoadStyleLink(nsIContent*, nsIURI*, nsAString_internal const&, nsAString_internal const&, bool, nsICSSLoaderObserver*, bool*) (Loader.cpp:1884) ==14443== by 0x80BEEF6: nsStyleLinkElement::DoUpdateStyleSheet(nsIDocument*, nsICSSLoaderObserver*, bool*, bool*, bool) (nsStyleLinkElement.cpp:280) ==14443== by 0x80BEF63: nsStyleLinkElement::UpdateStyleSheetInternal(nsIDocument*, bool) (nsStyleLinkElement.cpp:190) ==14443== by 0x81B73DC: nsStyleLinkElement::UpdateStyleSheetInternal() (nsStyleLinkElement.h:58) ==14443== by 0x81B6ECF: nsRunnableMethodImpl<void (nsHTMLLinkElement::*)(), true>::Run() (nsThreadUtils.h:349) ==14443== by 0x804A914: nsContentUtils::RemoveScriptBlocker() (nsContentUtils.cpp:4879) ==14443== by 0x8079BF4: nsDocument::EndUpdate(unsigned int) (nsDocument.cpp:3956) ==14443== Previously reported by 'windows' ==14443== at 0x82EEDC3: DOMStyleMallocSizeOf(void const*) (nsWindowMemoryReporter.cpp:102) ==14443== by 0x7F6DBFC: nsCSSStyleSheetInner::SizeOfIncludingThis(unsigned long (*)(void const*)) const (nsCSSStyleSheet.cpp:978) ==14443== by 0x7F6DC7A: nsCSSStyleSheet::SizeOfIncludingThis(unsigned long (*)(void const*)) const (nsCSSStyleSheet.cpp:849) ==14443== by 0x8061B12: SizeOfStyleSheetsElementIncludingThis(nsIStyleSheet*, unsigned long (*)(void const*), void*) (nsDocument.cpp:9635) ==14443== by 0x8A70A12: SizeOfElementIncludingThisEnumerator(void const*, void*) (nsVoidArray.cpp:737) ==14443== by 0x8A71406: nsVoidArray::EnumerateForwards(bool (*)(void const*, void*), void*) const (nsVoidArray.cpp:704) ==14443== by 0x8A71500: nsVoidArray::SizeOfExcludingThis(unsigned long (*)(void const*, unsigned long (*)(void const*), void*), unsigned long (*)(void const*), void*) const (nsVoidArray.cpp:755) ==14443== by 0x80639D0: nsDocument::DocSizeOfExcludingThis(nsWindowSizes*) const (nsCOMArray.h:98) ==14443== by 0x8215C0C: nsXMLDocument::DocSizeOfExcludingThis(nsWindowSizes*) const (nsXMLDocument.cpp:574) ==14443== by 0x8061AF5: nsIDocument::DocSizeOfIncludingThis(nsWindowSizes*) const (nsDocument.cpp:9627) ==14443== by 0x82BA622: nsGlobalWindow::SizeOfIncludingThis(nsWindowSizes*) const (nsGlobalWindow.cpp:10167) ==14443== by 0x82EF445: CollectWindowReports(nsGlobalWindow*, nsWindowSizes*, nsTHashtable<nsUint64HashKey>*, nsIMemoryMultiReporterCallback*, nsISupports*) (nsWindowMemoryReporter.cpp:157) ==14443== Now reported by 'windows' ==14443== at 0x82EEDC3: DOMStyleMallocSizeOf(void const*) (nsWindowMemoryReporter.cpp:102) ==14443== by 0x7F6DBFC: nsCSSStyleSheetInner::SizeOfIncludingThis(unsigned long (*)(void const*)) const (nsCSSStyleSheet.cpp:978) ==14443== by 0x7F6DC7A: nsCSSStyleSheet::SizeOfIncludingThis(unsigned long (*)(void const*)) const (nsCSSStyleSheet.cpp:849) ==14443== by 0x7F6DC34: nsCSSStyleSheetInner::SizeOfIncludingThis(unsigned long (*)(void const*)) const (nsCSSStyleSheet.cpp:981) ==14443== by 0x7F6DC7A: nsCSSStyleSheet::SizeOfIncludingThis(unsigned long (*)(void const*)) const (nsCSSStyleSheet.cpp:849) ==14443== by 0x8061B12: SizeOfStyleSheetsElementIncludingThis(nsIStyleSheet*, unsigned long (*)(void const*), void*) (nsDocument.cpp:9635) ==14443== by 0x8A70A12: SizeOfElementIncludingThisEnumerator(void const*, void*) (nsVoidArray.cpp:737) ==14443== by 0x8A71406: nsVoidArray::EnumerateForwards(bool (*)(void const*, void*), void*) const (nsVoidArray.cpp:704) ==14443== by 0x8A71500: nsVoidArray::SizeOfExcludingThis(unsigned long (*)(void const*, unsigned long (*)(void const*), void*), unsigned long (*)(void const*), void*) const (nsVoidArray.cpp:755) ==14443== by 0x80639D0: nsDocument::DocSizeOfExcludingThis(nsWindowSizes*) const (nsCOMArray.h:98) ==14443== by 0x8215C0C: nsXMLDocument::DocSizeOfExcludingThis(nsWindowSizes*) const (nsXMLDocument.cpp:574) ==14443== by 0x8061AF5: nsIDocument::DocSizeOfIncludingThis(nsWindowSizes*) const (nsDocument.cpp:9627) ==14443== Double report of heap block 0x15E68730: ==14443== Allocated ==14443== at 0x4C284AF: malloc (vg_replace_malloc.c:267) ==14443== by 0x74630D8: moz_xmalloc (mozalloc.cpp:54) ==14443== by 0x7F574CB: (anonymous namespace)::CSSParserImpl::ParseImportRule(void (*)(mozilla::css::Rule*, void*), void*) (mozalloc.h:200) ==14443== by 0x7F50C3D: (anonymous namespace)::CSSParserImpl::ParseAtRule(void (*)(mozilla::css::Rule*, void*), void*, bool) (nsCSSParser.cpp:1510) ==14443== by 0x7F5B967: nsCSSParser::ParseSheet(nsAString_internal const&, nsIURI*, nsIURI*, nsIPrincipal*, unsigned int, bool) (nsCSSParser.cpp:901) ==14443== by 0x7F48972: mozilla::css::Loader::ParseSheet(nsAString_internal const&, mozilla::css::SheetLoadData*, bool&) (Loader.cpp:1611) ==14443== by 0x7F48EBE: mozilla::css::SheetLoadData::OnStreamComplete(nsIUnicharStreamLoader*, nsISupports*, unsigned int, nsAString_internal const&) (Loader.cpp:959) ==14443== by 0x7CFD2B4: nsUnicharStreamLoader::OnStopRequest(nsIRequest*, nsISupports*, unsigned int) (nsUnicharStreamLoader.cpp:92) ==14443== by 0x7CD7889: nsBaseChannel::OnStopRequest(nsIRequest*, nsISupports*, unsigned int) (nsBaseChannel.cpp:713) ==14443== by 0x7CE0AB5: nsInputStreamPump::OnStateStop() (nsInputStreamPump.cpp:555) ==14443== by 0x7CE0D83: nsInputStreamPump::OnInputStreamReady(nsIAsyncInputStream*) (nsInputStreamPump.cpp:373) ==14443== by 0x8A96589: nsInputStreamReadyEvent::Run() (nsStreamUtils.cpp:82) ==14443== Previously reported by 'windows' ==14443== at 0x82EEDC3: DOMStyleMallocSizeOf(void const*) (nsWindowMemoryReporter.cpp:102) ==14443== by 0x7F64F3B: mozilla::css::ImportRule::SizeOfIncludingThis(unsigned long (*)(void const*)) const (nsCSSRules.cpp:479) ==14443== by 0x7F64E0D: mozilla::css::Rule::SizeOfCOMArrayElementIncludingThis(mozilla::css::Rule*, unsigned long (*)(void const*), void*) (nsCSSRules.cpp:93) ==14443== by 0x8A70A12: SizeOfElementIncludingThisEnumerator(void const*, void*) (nsVoidArray.cpp:737) ==14443== by 0x8A71406: nsVoidArray::EnumerateForwards(bool (*)(void const*, void*), void*) const (nsVoidArray.cpp:704) ==14443== by 0x8A71500: nsVoidArray::SizeOfExcludingThis(unsigned long (*)(void const*, unsigned long (*)(void const*), void*), unsigned long (*)(void const*), void*) const (nsVoidArray.cpp:755) ==14443== by 0x7F6DC17: nsCSSStyleSheetInner::SizeOfIncludingThis(unsigned long (*)(void const*)) const (nsCOMArray.h:98) ==14443== by 0x7F6DC7A: nsCSSStyleSheet::SizeOfIncludingThis(unsigned long (*)(void const*)) const (nsCSSStyleSheet.cpp:849) ==14443== by 0x8061B12: SizeOfStyleSheetsElementIncludingThis(nsIStyleSheet*, unsigned long (*)(void const*), void*) (nsDocument.cpp:9635) ==14443== by 0x8A70A12: SizeOfElementIncludingThisEnumerator(void const*, void*) (nsVoidArray.cpp:737) ==14443== by 0x8A71406: nsVoidArray::EnumerateForwards(bool (*)(void const*, void*), void*) const (nsVoidArray.cpp:704) ==14443== by 0x8A71500: nsVoidArray::SizeOfExcludingThis(unsigned long (*)(void const*, unsigned long (*)(void const*), void*), unsigned long (*)(void const*), void*) const (nsVoidArray.cpp:755) ==14443== Now reported by 'windows' ==14443== at 0x82EEDC3: DOMStyleMallocSizeOf(void const*) (nsWindowMemoryReporter.cpp:102) ==14443== by 0x7F64F3B: mozilla::css::ImportRule::SizeOfIncludingThis(unsigned long (*)(void const*)) const (nsCSSRules.cpp:479) ==14443== by 0x7F64E0D: mozilla::css::Rule::SizeOfCOMArrayElementIncludingThis(mozilla::css::Rule*, unsigned long (*)(void const*), void*) (nsCSSRules.cpp:93) ==14443== by 0x8A70A12: SizeOfElementIncludingThisEnumerator(void const*, void*) (nsVoidArray.cpp:737) ==14443== by 0x8A71406: nsVoidArray::EnumerateForwards(bool (*)(void const*, void*), void*) const (nsVoidArray.cpp:704) ==14443== by 0x8A71500: nsVoidArray::SizeOfExcludingThis(unsigned long (*)(void const*, unsigned long (*)(void const*), void*), unsigned long (*)(void const*), void*) const (nsVoidArray.cpp:755) ==14443== by 0x7F6DC17: nsCSSStyleSheetInner::SizeOfIncludingThis(unsigned long (*)(void const*)) const (nsCOMArray.h:98) ==14443== by 0x7F6DC7A: nsCSSStyleSheet::SizeOfIncludingThis(unsigned long (*)(void const*)) const (nsCSSStyleSheet.cpp:849) ==14443== by 0x7F6DC34: nsCSSStyleSheetInner::SizeOfIncludingThis(unsigned long (*)(void const*)) const (nsCSSStyleSheet.cpp:981) ==14443== by 0x7F6DC7A: nsCSSStyleSheet::SizeOfIncludingThis(unsigned long (*)(void const*)) const (nsCSSStyleSheet.cpp:849) ==14443== by 0x8061B12: SizeOfStyleSheetsElementIncludingThis(nsIStyleSheet*, unsigned long (*)(void const*), void*) (nsDocument.cpp:9635) ==14443== by 0x8A70A12: SizeOfElementIncludingThisEnumerator(void const*, void*) (nsVoidArray.cpp:737) ...and more like this, but I think those illustrate the point well enough: we have memory shared by nsCSSStyleSheetInner and we're double-counting that. Looks like the style memory reporting has to become a bit more sophisticated.

I've seen these also when the error console is open, and (I think?) when multiple browser windows are open.

Looks like nsCSSStyleSheet::SizeOfIncludingThis assumes that it owns its mInner. In fact multiple style sheets can share an mInner (which is then copy-on-write if there are multiple owners) -- that's why there's a sheet vs. inner separation.

(It should probably just check if it is the inner's mSheets[0].)

> Looks like nsCSSStyleSheet::SizeOfIncludingThis assumes that it owns its > mInner. In fact multiple style sheets can share an mInner (which is then > copy-on-write if there are multiple owners) -- that's why there's a sheet > vs. inner separation. > > (It should probably just check if it is the inner's mSheets[0].) So nsCSSStyleSheetInner::mSheets holds pointers to each of the nsCSSStyleSheets that share the inner? I see. There will be a bit of arbitrariness in terms of who gets blamed for the memory consumption of the inner, but that seems hard to avoid and double-counting is worse.

This simple patch follows dbaron's suggestion. It removes all of DMD's double-counting warnings for both (a) the two-window desktop browser case, and (b) B2G. Hurray.

Comment on attachment 669799 [details] [diff] [review] Don't double-count shared nsCSSStyleSheetInners. s->mInner in the if condition, as long as we're using "s" everywhere here, and r=me

One note, though: How do we decide which nsCSSStyleSheets to measure right now? If we're not doing the ones in the caches in the css::Loader for the document, this patch can cause us to miss sheets altogether in cases when we prefetched... I wonder whether we should avoid sticking the prefetch sheets into the table, since that can cause use to use more memory when CSSOM is used. Maybe a followup bug on that?

And definitely a followup on memory-reporting those hashtables in the loader, please!

> And definitely a followup on memory-reporting those hashtables in the > loader, please! Bug 799796. > One note, though: How do we decide which nsCSSStyleSheets to measure right > now? If we're not doing the ones in the caches in the css::Loader for the > document, this patch can cause us to miss sheets altogether in cases when we > prefetched... We just measure sheets in documents and the style-sheet-cache, IIRC. > I wonder whether we should avoid sticking the prefetch sheets into the > table, since that can cause use to use more memory when CSSOM is used. > Maybe a followup bug on that? Can you file that bug? I don't understand the idea enough to write something sensible.

> Can you file that bug? I don't understand the idea enough to write something sensible. Filed bug 799816.

Comment on attachment 669799 [details] [diff] [review] Don't double-count shared nsCSSStyleSheetInners. [Approval Request Comment] Bug caused by (feature/regressing bug #): memory reporting. User impact if declined: misleading memory reporting which will hurt understanding of B2G memory consumption. Testing completed (on m-c, etc.): has been on m-c for 16 days without problem. Risk to taking this patch (and alternatives if risky): minimal; very simple patch and the code only runs when viewing about:memory or triggering a memory report dump. String or UUID changes made by this patch: none.

Comment on attachment 669799 [details] [diff] [review] Don't double-count shared nsCSSStyleSheetInners. Approving for aurora as it is low risk and help understanding memory consumption better