Closed Bug 1399727 Opened 7 years ago Closed 7 years ago

Fix double-reporting of memory in ScriptPreloader

Categories

(Core :: XPConnect, defect)

56 Branch
defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla57
Tracking Status
firefox57 --- fixed

People

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

Details

Attachments

(1 file)

I get this regularly with DMD:

> Twice-reported {
>   16 blocks in heap block record 1 of 1
>   1,152 bytes (1,152 requested / 0 slop)
>   Individual block sizes: 128 x 2; 64 x 14
>   0.00% of the heap (0.00% cumulative)
>   100.00% of twice-reported (100.00% cumulative)
>   Allocated at {
>     #01: replace_malloc (/home/njn/moz/mc1/memory/replace/dmd/DMD.cpp:1303 (discriminator 2))
>     #02: nsStringBuffer::Alloc(unsigned long) (/home/njn/moz/mc1/xpcom/string/nsSubstring.cpp:238)
>     #03: nsTSubstring<char>::MutatePrep(unsigned int, char**, mozilla::detail::StringDataFlags*) (/home/njn/moz/mc1/o64sty/xpcom/string/../../../xpcom/string/nsTSubstring.cpp:164 (discriminator 1))
>     #04: nsTSubstring<char>::ReplacePrepInternal(unsigned int, unsigned int, unsigned int, unsigned int) (/home/njn/moz/mc1/o64sty/xpcom/string/../../../xpcom/string/nsTSubstring.cpp:227)
>     #05: nsTSubstring<char>::ReplacePrep(unsigned int, unsigned int, unsigned int) (/home/njn/moz/mc1/o64sty/xpcom/string/../../../xpcom/string/nsTSubstring.cpp:218)
>     #06: nsTSubstring<char>::Assign(char const*, unsigned int, mozilla::fallible_t const&) (/home/njn/moz/mc1/o64sty/xpcom/string/../../../xpcom/string/nsTSubstring.cpp:388)
>     #07: nsTSubstring<char>::Assign(nsTSubstring<char> const&, mozilla::fallible_t const&) (/home/njn/moz/mc1/o64sty/xpcom/string/../../../xpcom/string/nsTSubstring.cpp:483 (discriminator 2))
>     #08: nsTSubstring<char>::Assign(nsTSubstring<char> const&) (/home/njn/moz/mc1/o64sty/xpcom/string/../../../xpcom/string/nsTSubstring.cpp:439)
>     #09: nsMessageManagerScriptExecutor::TryCacheLoadAndCompileScript(nsTSubstring<char16_t> const&, bool, bool, JS::MutableHandle<JSScript*>) (/home/njn/moz/mc1/o64sty/dom/base/../../../dom/base/nsFrameMessageManager.cpp:1594 (discriminator 2))
>     #10: nsMessageManagerScriptExecutor::LoadScriptInternal(nsTSubstring<char16_t> const&, bool) (/home/njn/moz/mc1/o64sty/dom/base/../../../dom/base/nsFrameMessageManager.cpp:1563)
>   }
>   Reported at {
>     #01: mozilla::dmd::ReportHelper(void const*, bool) (/home/njn/moz/mc1/memory/replace/dmd/DMD.cpp:1676 (discriminator 1))
>     #02: mozilla::ScriptPreloader::MallocSizeOf(void const*) (/home/njn/moz/mc1/o64sty/js/xpconnect/loader/../../../dist/include/mozilla/ScriptPreloader.h:61 (discriminator 1))
>     #03: nsTSubstring<char>::SizeOfExcludingThisEvenIfShared(unsigned long (*)(void const*)) const (/home/njn/moz/mc1/o64sty/xpcom/string/../../../xpcom/string/nsTSubstring.cpp:1197)
>     #04: mozilla::ScriptPreloader::CachedScript::HeapSizeOfIncludingThis(unsigned long (*)(void const*)) (/home/njn/moz/mc1/o64sty/js/xpconnect/loader/../../../dist/include/mozilla/ScriptPreloader.h:272)
>     #05: unsigned long mozilla::ScriptPreloader::SizeOfHashEntries<(mozilla::ScriptPreloader::ScriptStatus)1>(nsClassHashtable<nsCStringHashKey, mozilla::ScriptPreloader::CachedScript>&, unsigned long (*)(void const*)) (/home/njn/moz/mc1/o64sty/js/xpconnect/loader/../../../dist/include/mozilla/ScriptPreloader.h:411)
>     #06: mozilla::ScriptPreloader::CollectReports(nsIMemoryReporterCallback*, nsISupports*, bool) (/home/njn/moz/mc1/js/xpconnect/loader/ScriptPreloader.cpp:60 (discriminator 3))
>     #07: non-virtual thunk to mozilla::ScriptPreloader::CollectReports(nsIMemoryReporterCallback*, nsISupports*, bool) (crtstuff.c:?)
>     #08: operator() (/home/njn/moz/mc1/xpcom/base/nsMemoryReporterManager.cpp:1780)
>     #09: mozilla::detail::RunnableFunction<nsMemoryReporterManager::DispatchReporter(nsIMemoryReporter*, bool, nsIMemoryReporterCallback*, nsISupports*, bool)::$_0>::Run() (/home/njn/moz/mc1/o64sty/xpcom/base/../../dist/include/nsThreadUtils.h:528)
>     #10: nsThread::ProcessNextEvent(bool, bool*) (/home/njn/moz/mc1/xpcom/threads/nsThread.cpp:1039 (discriminator 1))
>   }
>   Reported again at {
>     #01: mozilla::dmd::ReportHelper(void const*, bool) (/home/njn/moz/mc1/memory/replace/dmd/DMD.cpp:1676 (discriminator 1))
>     #02: mozilla::ScriptPreloader::MallocSizeOf(void const*) (/home/njn/moz/mc1/o64sty/js/xpconnect/loader/../../../dist/include/mozilla/ScriptPreloader.h:61 (discriminator 1))
>     #03: nsTSubstring<char>::SizeOfExcludingThisEvenIfShared(unsigned long (*)(void const*)) const (/home/njn/moz/mc1/o64sty/xpcom/string/../../../xpcom/string/nsTSubstring.cpp:1197)
>     #04: mozilla::ScriptPreloader::CachedScript::HeapSizeOfIncludingThis(unsigned long (*)(void const*)) (/home/njn/moz/mc1/o64sty/js/xpconnect/loader/../../../dist/include/mozilla/ScriptPreloader.h:27 2)
>     #05: unsigned long mozilla::ScriptPreloader::SizeOfHashEntries<(mozilla::ScriptPreloader::ScriptStatus)1>(nsClassHashtable<nsCStringHashKey, mozilla::ScriptPreloader::CachedScript>&, unsigned long (*)(void const*)) (/home/njn/moz/mc1/o64sty/js/xpconnect/loader/../../../dist/include/mozilla/ScriptPreloader.h:411)
>     #06: mozilla::ScriptPreloader::CollectReports(nsIMemoryReporterCallback*, nsISupports*, bool) (/home/njn/moz/mc1/js/xpconnect/loader/ScriptPreloader.cpp:60 (discriminator 3))
>     #07: non-virtual thunk to mozilla::ScriptPreloader::CollectReports(nsIMemoryReporterCallback*, nsISupports*, bool) (crtstuff.c:?)
>     #08: operator() (/home/njn/moz/mc1/xpcom/base/nsMemoryReporterManager.cpp:1780)
>     #09: mozilla::detail::RunnableFunction<nsMemoryReporterManager::DispatchReporter(nsIMemoryReporter*, bool, nsIMemoryReporterCallback*, nsISupports*, bool)::$_0>::Run() (/home/njn/moz/mc1/o64sty/xpcom/base/../../dist/include/nsThreadUtils.h:528)
>     #10: nsThread::ProcessNextEvent(bool, bool*) (/home/njn/moz/mc1/xpcom/threads/nsThread.cpp:1039 (discriminator 1))
>   }
> }
DMD indicates double-reporting for mURL, indicating that it is shared.
Attachment #8907936 - Flags: review?(kmaglione+bmo)
Assignee: nobody → n.nethercote
Status: NEW → ASSIGNED
Comment on attachment 8907936 [details] [diff] [review]
Fix double-reporting of memory in ScriptPreloader

Review of attachment 8907936 [details] [diff] [review]:
-----------------------------------------------------------------

Hm. I think this is a special case for scripts loaded by the message manager, since they use the same string for the cache path and the URL. I guess this is a reasonable enough solution to that problem.
Attachment #8907936 - Flags: review?(kmaglione+bmo) → review+
https://hg.mozilla.org/mozilla-central/rev/cf78e291a937
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla57
You need to log in before you can comment on or make changes to this bug.