Closed Bug 697335 Opened 13 years ago Closed 12 years ago

Another memory reporter for the startup cache

Categories

(Core :: XPCOM, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla11

People

(Reporter: n.nethercote, Assigned: n.nethercote)

References

(Blocks 1 open bug)

Details

(Whiteboard: [MemShrink:P2])

Attachments

(2 files)

+++ 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.
Blocks: DarkMatter
Whiteboard: [MemShrink] → [MemShrink:P2]
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)
Attached patch patch 1Splinter Review
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()).
Assignee: nobody → nnethercote
Status: NEW → ASSIGNED
Attachment #579541 - Flags: review?(bzbarsky)
Attached patch patch 2Splinter Review
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.
Attachment #579542 - Flags: review?(tglek)
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.
Attachment #579541 - Flags: review?(bzbarsky) → review+
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.
Attachment #579542 - Flags: review?(mozilla) → review+
Depends on: 707865
> 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 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?
> 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.)
https://hg.mozilla.org/mozilla-central/rev/e255fa32719c
https://hg.mozilla.org/mozilla-central/rev/692d80735b7e
Status: ASSIGNED → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla11
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: