Last Comment Bug 697335 - Another memory reporter for the startup cache
: Another memory reporter for the startup cache
Status: RESOLVED FIXED
[MemShrink:P2]
:
Product: Core
Classification: Components
Component: XPCOM (show other bugs)
: unspecified
: All All
: -- normal with 1 vote (vote)
: mozilla11
Assigned To: Nicholas Nethercote [:njn] (on vacation until July 11)
:
Mentors:
Depends on: DMD 696690 707865
Blocks: DarkMatter
  Show dependency treegraph
 
Reported: 2011-10-25 19:40 PDT by Nicholas Nethercote [:njn] (on vacation until July 11)
Modified: 2011-12-19 05:46 PST (History)
8 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
patch 1 (2.98 KB, patch)
2011-12-06 17:22 PST, Nicholas Nethercote [:njn] (on vacation until July 11)
bzbarsky: review+
Details | Diff | Review
patch 2 (5.69 KB, patch)
2011-12-06 17:23 PST, Nicholas Nethercote [:njn] (on vacation until July 11)
taras.mozilla: review+
Details | Diff | Review

Description Nicholas Nethercote [:njn] (on vacation until July 11) 2011-10-25 19:40:58 PDT
+++ This bug was initially created as a clone of Bug #696690 +++

I also saw the following, which isn't terribly big:

==1586== Unreported: 126,976 (cumulative: 548,427,584) bytes in 2 heap block(s) in record 79 of 15105:
==1586==  Requested bytes unreported: 122,444 / 122,444
==1586==  Slop      bytes unreported: 4,532 / 4,532
==1586==    at 0x402A063: malloc (vg_replace_malloc.c:263) 
==1586==    by 0x403BFFA: moz_xmalloc (mozalloc.cpp:103)
==1586==    by 0x68660B8: mozilla::scache::StartupCache::PutBuffer(char const*, char const*, unsigned int) (mozalloc.h:242)
==1586==    by 0x74A4088: WriteCachedScript(mozilla::scache::StartupCache*, nsACString_internal&, JSContext*, JSScript*) (moz
JSLoaderUtils.cpp:189)
==1586==    by 0x749F1D5: mozJSComponentLoader::GlobalForLocation(nsILocalFile*, nsIURI*, JSObject**, char**, JS::Value*) (mo
zJSComponentLoader.cpp:1003) 
==1586==    by 0x749D77F: mozJSComponentLoader::LoadModuleImpl(nsILocalFile*, nsAString_internal&, nsIURI*) (mozJSComponentLo
ader.cpp:607)
==1586==    by 0x749D3A4: mozJSComponentLoader::LoadModule(nsILocalFile*) (mozJSComponentLoader.cpp:547)
==1586==    by 0x7A42B50: nsComponentManagerImpl::KnownModule::Load() (nsComponentManager.cpp:951)

I just saw a case where this stack trace was responsible for over 1MB of allocations.
Comment 1 Nicholas Nethercote [:njn] (on vacation until July 11) 2011-12-05 19:34:57 PST
Some more interesting ones:

==5227== Unreported: 10 block(s) in record 7 of 15307
==5227==  786,912 bytes (769,800 requested / 17,112 slop)
==5227==  1.35% of the heap (14.91% cumulative unreported)
==5227==    at 0x402A063: malloc (vg_replace_malloc.c:263)
==5227==    by 0x403C05A: moz_xmalloc (mozalloc.cpp:103)
==5227==    by 0x6892748: mozilla::scache::StartupCache::PutBuffer(char const*, char const*, unsigned int) (mozalloc.h:241)
==5227==    by 0x6F43EFD: nsXULPrototypeCache::FinishOutputStream(nsIURI*) (nsXULPrototypeCache.cpp:509)
==5227==    by 0x729F097: nsXULPrototypeScript::SerializeOutOfLine(nsIObjectOutputStream*, nsIScriptGlobalObject*) (nsXULElement.cpp:2977)
==5227==    by 0x729DFB3: nsXULPrototypeElement::Serialize(nsIObjectOutputStream*, nsIScriptGlobalObject*, nsCOMArray<nsINodeInfo> const*) (n
sXULElement.cpp:2678)
==5227==    by 0x6F479CF: nsXULPrototypeDocument::Write(nsIObjectOutputStream*) (nsXULPrototypeDocument.cpp:480)
==5227==    by 0x6F43806: nsXULPrototypeCache::WritePrototype(nsXULPrototypeDocument*) (nsXULPrototypeCache.cpp:420)


==5227== Unreported: 18 block(s) in record 10 of 15307
==5227==  559,104 bytes (527,190 requested / 31,914 slop)
==5227==  0.96% of the heap (18.16% cumulative unreported)
==5227==    at 0x402A063: malloc (vg_replace_malloc.c:263)
==5227==    by 0x403C05A: moz_xmalloc (mozalloc.cpp:103)
==5227==    by 0x6892748: mozilla::scache::StartupCache::PutBuffer(char const*, char const*, unsigned int) (mozalloc.h:241)
==5227==    by 0x6F0FDCB: nsXBLDocumentInfo::WritePrototypeBindings() (nsXBLDocumentInfo.cpp:697)
==5227==    by 0x6F21F79: nsXBLService::LoadBindingDocumentInfo(nsIContent*, nsIDocument*, nsIURI*, nsIPrincipal*, bool, nsXBLDocumentInfo**)
 (nsXBLService.cpp:1105)
==5227==    by 0x6F20E1F: nsXBLService::GetBinding(nsIContent*, nsIURI*, bool, nsIPrincipal*, bool*, nsXBLBinding**, nsTArray<nsIURI*, nsTArr
ayDefaultAllocator>&) (nsXBLService.cpp:838)
==5227==    by 0x6F20C92: nsXBLService::GetBinding(nsIContent*, nsIURI*, bool, nsIPrincipal*, bool*, nsXBLBinding**) (nsXBLService.cpp:811)
==5227==    by 0x6F2004C: nsXBLService::LoadBindings(nsIContent*, nsIURI*, nsIPrincipal*, bool, nsXBLBinding**, bool*) (nsXBLService.cpp:561)
==5227==

==5227== Unreported: 5 block(s) in record 27 of 15307
==5227==  161,792 bytes (147,308 requested / 14,484 slop)
==5227==  0.27% of the heap (23.49% cumulative unreported)
==5227==    at 0x402A063: malloc (vg_replace_malloc.c:263)
==5227==    by 0x403C05A: moz_xmalloc (mozalloc.cpp:103)
==5227==    by 0x6892748: mozilla::scache::StartupCache::PutBuffer(char const*, char const*, unsigned int) (mozalloc.h:241)
==5227==    by 0x6F43EFD: nsXULPrototypeCache::FinishOutputStream(nsIURI*) (nsXULPrototypeCache.cpp:509)
==5227==    by 0x729F097: nsXULPrototypeScript::SerializeOutOfLine(nsIObjectOutputStream*, nsIScriptGlobalObject*) (nsXULElement.cpp:2977)
==5227==    by 0x6F3C71A: nsXULDocument::OnStreamComplete(nsIStreamLoader*, nsISupports*, unsigned int, unsigned int, unsigned char const*) (
nsXULDocument.cpp:3570)
==5227==    by 0x6758A7D: nsStreamLoader::OnStopRequest(nsIRequest*, nsISupports*, unsigned int) (nsStreamLoader.cpp:125)
==5227==    by 0x671CA6C: nsBaseChannel::OnStopRequest(nsIRequest*, nsISupports*, unsigned int) (nsBaseChannel.cpp:745)
Comment 2 Nicholas Nethercote [:njn] (on vacation until July 11) 2011-12-06 17:22:41 PST
Created attachment 579541 [details] [diff] [review]
patch 1

Before I could implement the startup cache reporter, I had to implement nsBaseHashtable::SizeOfExcludingThis(), which was heavily modelled on nsTHashtable::SizeOfExcludingThis()  (Nb: you currently need to look at the patch in bug 701210 to see the most up-to-date version of nsTHashtable::SizeOfExcludingThis()).
Comment 3 Nicholas Nethercote [:njn] (on vacation until July 11) 2011-12-06 17:23:52 PST
Created attachment 579542 [details] [diff] [review]
patch 2

This patch renames the "explicit/startup-cache" reporter to
"explicit/startup-cache/mapping" and adds the "explicit/startup-cache/data"
reporter.  The new reporter measures the StartupCache object itself and
StartupCache::mTable, which easily gets up to 3 or 4 MB at
startup (a sizeable percentage of "explicit", e.g. 5% or more), but drops
down to much smaller values after some time passes.
Comment 4 Boris Zbarsky [:bz] (Out June 25-July 6) 2011-12-06 19:51:06 PST
Comment on attachment 579541 [details] [diff] [review]
patch 1

>+++ b/xpcom/glue/nsBaseHashtable.h
>+  typedef size_t
>+    (* SizeOfEntryFun)(KeyType           aKey,
>+                       DataType          &aData,

can aData be |const DataType&|?  If so, I'd really prefer that.

>+  s_SizeOfArgs* eargs = (s_SizeOfArgs*) arg;

static_cast?

r=me with those.
Comment 5 (dormant account) 2011-12-07 09:45:04 PST
Comment on attachment 579542 [details] [diff] [review]
patch 2

Raw nsIMemoryReporter* are unsettling. I would be much happier to see a RAII wrapper for NS_[Un]RegisterMemoryReporter.
Comment 6 Nicholas Nethercote [:njn] (on vacation until July 11) 2011-12-10 03:59:52 PST
> Raw nsIMemoryReporter* are unsettling. I would be much happier to see a RAII
> wrapper for NS_[Un]RegisterMemoryReporter.

I can't quite imagine what that would look like.  Can you give more details?  Would this just be for the startup cache or would it apply to other memory reporters?
Comment 7 (dormant account) 2011-12-12 12:13:38 PST
(In reply to Nicholas Nethercote [:njn] from comment #6)
> > Raw nsIMemoryReporter* are unsettling. I would be much happier to see a RAII
> > wrapper for NS_[Un]RegisterMemoryReporter.
> 
> I can't quite imagine what that would look like.  Can you give more details?
> Would this just be for the startup cache or would it apply to other memory
> reporters?

In general.
proposed pseudocode:
nsIMemoryReporter* mDataMemoryReporter; -> nsAutoPtr<nsIMemoryReporter> mDataMemoryReporter;
and then assign that with mDataMemoryReporter =  NS_RegisterMemoryReporter(new NS_MEMORY_REPORTER_NAME(StartupCacheMapping)

Is there a reason some sort of raii like this is not realistic for memory reporters?
Comment 8 Nicholas Nethercote [:njn] (on vacation until July 11) 2011-12-12 17:07:18 PST
> In general.
> proposed pseudocode:
> nsIMemoryReporter* mDataMemoryReporter; -> nsAutoPtr<nsIMemoryReporter>
> mDataMemoryReporter;
> and then assign that with mDataMemoryReporter = 
> NS_RegisterMemoryReporter(new NS_MEMORY_REPORTER_NAME(StartupCacheMapping)
> 
> Is there a reason some sort of raii like this is not realistic for memory
> reporters?

When you register a memory reporter, a strong reference to it is held by the MemoryReporterManager, and that reference is only released when NS_UnregisterMemoryReporter is called.

Maybe I've misunderstood your proposal, but it seems like the explicit NS_UnregisterMemoryReporter is necessary with this design.

(BTW, that explains why the raw nsIMemoryReporter* pointer is present -- it could be a nsCOMPtr/nsRefPtr, but its destruction coincides exactly with the NS_UnregisterMemoryReporter call so it's not a big deal.  Some places use a raw pointer and some places use a COM/Ref pointer.)

Note You need to log in before you can comment on or make changes to this bug.