1268863 - Memory taken up by script-sources are not sometimes reported in about:memory (detected via DMD)

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[Jukka Jylänki]

STR:

1. Visit https://dl.dropboxusercontent.com/u/40949268/emcc/2016-04-29-StrategyGame_separate_asm_shipping/StrategyGame-HTML5-Shipping.html
2. Run an about:memory log.

Observed:

689.89 MB (100.0%) -- explicit
├──416.03 MB (60.30%) -- window-objects/top(file:///D:/UnrealProjects/StrategyGame%204.11/html5_build_separate_asm_shipping/HTML5/StrategyGame-HTML5-Shipping.html, id=4294967297)
│  ├──415.58 MB (60.24%) -- active/window(file:///D:/UnrealProjects/StrategyGame%204.11/html5_build_separate_asm_shipping/HTML5/StrategyGame-HTML5-Shipping.html)
│  │  ├──413.94 MB (60.00%) -- js-compartment(file:///D:/UnrealProjects/StrategyGame%204.11/html5_build_separate_asm_shipping/HTML5/StrategyGame-HTML5-Shipping.html)
│  │  │  ├──413.43 MB (59.93%) -- classes
│  │  │  │  ├──322.01 MB (46.67%) -- class(ArrayBuffer)/objects
│  │  │  │  │  ├──320.00 MB (46.38%) ── non-heap/elements/asm.js
│  │  │  │  │  └────2.01 MB (00.29%) ++ (2 tiny)
│  │  │  │  ├───89.80 MB (13.02%) -- class(WasmModuleObject)/objects
│  │  │  │  │   ├──55.54 MB (08.05%) ── non-heap/code/asm.js
│  │  │  │  │   ├──34.25 MB (04.97%) ── malloc-heap/misc
│  │  │  │  │   └───0.00 MB (00.00%) ── gc-heap
│  │  │  │  └────1.63 MB (00.24%) ++ (9 tiny)
│  │  │  └────0.51 MB (00.07%) ++ (7 tiny)
│  │  └────1.64 MB (00.24%) ++ (4 tiny)
│  └────0.45 MB (00.06%) ++ js-zone(0x2bba2c24800)
├──220.19 MB (31.92%) ── heap-unclassified
├───26.48 MB (03.84%) -- dmd
│   ├──24.00 MB (03.48%) ── live-block-table
│   └───2.48 MB (00.36%) ++ (2 tiny)
├───15.65 MB (02.27%) -- js-non-window
│   ├───9.10 MB (01.32%) -- zones
│   │   ├──7.49 MB (01.09%) ++ zone(0x2bb97a79000)
│   │   └──1.61 MB (00.23%) ++ (3 tiny)
│   └───6.55 MB (00.95%) ++ (2 tiny)
└───11.55 MB (01.67%) ++ (19 tiny)

Running the page through DMD gives the source of heap-unclassified as 

Unreported {
  1 block in heap block record 1 of 2,974
  190,566,400 bytes (190,562,906 requested / 3,494 slop)
  66.57% of the heap (66.57% cumulative)
  82.16% of unreported (82.16% cumulative)
  Allocated at {
    #01: js::MallocProvider<JS::Zone>::maybe_pod_malloc<char16_t> (d:\gecko-dev\js\src\vm\mallocprovider.h:57)
    #02: js::MallocProvider<JS::Zone>::pod_malloc<char16_t> (d:\gecko-dev\js\src\vm\mallocprovider.h:91)
    #03: js::ScriptSource::ensureOwnsSource (d:\gecko-dev\js\src\jsscript.cpp:2075)
    #04: js::ScriptSource::setSourceCopy (d:\gecko-dev\js\src\jsscript.cpp:2128)
    #05: BytecodeCompiler::maybeCompressSource (d:\gecko-dev\js\src\frontend\bytecodecompiler.cpp:205)
    #06: BytecodeCompiler::compileScript (d:\gecko-dev\js\src\frontend\bytecodecompiler.cpp:500)
    #07: js::frontend::CompileScript (d:\gecko-dev\js\src\frontend\bytecodecompiler.cpp:742)
    #08: js::ScriptParseTask::parse (d:\gecko-dev\js\src\vm\helperthreads.cpp:278)
    #09: js::HelperThread::handleParseWorkload (d:\gecko-dev\js\src\vm\helperthreads.cpp:1541)
    #10: js::HelperThread::threadLoop (d:\gecko-dev\js\src\vm\helperthreads.cpp:1737)
    #11: _PR_NativeRunThread (d:\gecko-dev\nsprpub\pr\src\threads\combined\pruthr.c:419)
    #12: pr_root (d:\gecko-dev\nsprpub\pr\src\md\windows\w95thred.c:96)
    #13: crt_at_quick_exit[C:\WINDOWS\SYSTEM32\ucrtbase.dll +0x6be1d]
    #14: BaseThreadInitThunk[C:\WINDOWS\system32\KERNEL32.DLL +0x18102]
    #15: RtlUserThreadStart[C:\WINDOWS\SYSTEM32\ntdll.dll +0x5c5b4]
  }
}

Comment 1

[Luke Wagner [:luke]]

Ah hah; I think I was hitting this too earlier on http://github.com/lukewagner/AngryBotsPacked and I assumed it was some weird Blob interaction.

Comment 2

[Luke Wagner [:luke]]

Ah hah!  What's happening here is that we visit ScriptSources reachable via JSScript.  In these only-asm.js scripts (like I had also in AngryBotsPacked), the outer JSScript becomes unreachable/finalized and the ScriptSource is only kept alive via an AsmJSModule refcount.  With the obvious fix, I can see a new blob in script-sources that goes from 181MiB (before bug 1219098) to 23MiB on trunk.  Thanks for reporting!

Comment 3

[Luke Wagner [:luke]]

Oops, not sure how javascript-core-security got checked...

Comment 4

[Luke Wagner [:luke]]

Attachment: fix-asmjs-script-source-reporting

Comment 5

[Nicholas Nethercote [inactive]]

Comment on attachment 8747149 [details] [diff] [review]
fix-asmjs-script-source-reporting

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

Nice.

::: js/src/vm/MemoryMetrics.cpp
@@ +466,5 @@
> +
> +        if (obj->is<WasmModuleObject>()) {
> +            if (ScriptSource* ss = obj->as<WasmModuleObject>().module().maybeScriptSource())
> +                CollectScriptSourceStats<granularity>(closure, ss);
> +        }

This block needs a comment explaining why it won't lead to double-counting, i.e. why this ScriptSource is not reachable via a JSScript. Something like what you wrote in comment 2.

Comment 6

[Luke Wagner [:luke]]

(In reply to Nicholas Nethercote [:njn] from comment #5)
> This block needs a comment explaining why it won't lead to double-counting,
> i.e. why this ScriptSource is not reachable via a JSScript. Something like
> what you wrote in comment 2.

I'm happy to, but, just to make sure: even ignoring this new path via AsmJSModule, a single ScriptSource can be reached by multiple JSScripts and that's why there is that seenSources HashSet in CollectScriptSourceStats.  Do you still think it needs a comment?

Comment 7

[Nicholas Nethercote [inactive]]

> I'm happy to, but, just to make sure: even ignoring this new path via
> AsmJSModule, a single ScriptSource can be reached by multiple JSScripts and
> that's why there is that seenSources HashSet in CollectScriptSourceStats. 
> Do you still think it needs a comment?

Oh, in that case, probably not. Thank you for checking.

Comment 8

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/5f92a2190262

Comment 9

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/5d2a22f7aa39

Comment 10

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/5f92a2190262
https://hg.mozilla.org/mozilla-central/rev/5d2a22f7aa39