The default bug view has changed. See this FAQ.

Another memory reporter for the startup cache

RESOLVED FIXED in mozilla11

Status

()

Core
XPCOM
RESOLVED FIXED
6 years ago
5 years ago

People

(Reporter: njn, Assigned: njn)

Tracking

(Blocks: 1 bug)

unspecified
mozilla11
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [MemShrink:P2])

Attachments

(2 attachments)

(Assignee)

Description

6 years ago
+++ 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.
(Assignee)

Updated

6 years ago
Blocks: 563700
(Assignee)

Updated

6 years ago
Whiteboard: [MemShrink] → [MemShrink:P2]
(Assignee)

Comment 1

5 years ago
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)
(Assignee)

Comment 2

5 years ago
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()).
Assignee: nobody → nnethercote
Status: NEW → ASSIGNED
Attachment #579541 - Flags: review?(bzbarsky)
(Assignee)

Comment 3

5 years ago
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.
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 5

5 years ago
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+
(Assignee)

Updated

5 years ago
Depends on: 707865
(Assignee)

Comment 6

5 years ago
> 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

5 years ago
(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?
(Assignee)

Comment 8

5 years ago
> 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.)
(Assignee)

Comment 9

5 years ago
https://hg.mozilla.org/integration/mozilla-inbound/rev/e255fa32719c
https://hg.mozilla.org/integration/mozilla-inbound/rev/692d80735b7e
https://hg.mozilla.org/mozilla-central/rev/e255fa32719c
https://hg.mozilla.org/mozilla-central/rev/692d80735b7e
Status: ASSIGNED → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla11
You need to log in before you can comment on or make changes to this bug.