1578730 - Remove code-coverage dependence on JSScript::realm() during finalization

Status: RESOLVED FIXED

(Core :: JavaScript Engine, task, P2)

Description

[Ted Campbell [:tcampbell]]

Bug 1575350 removed most of the uses of JSScript::realm() during JSScript finalization, but the lcov stuff still using a LCovRealm. This is currently complicating Bug 1568245.

Comment 1

[Ted Campbell [:tcampbell]]

The approach that I am going with it to replace ScriptNameMap with a HashMap<JSScript*, LCovSource*>. This moves the LCovSource allocation earlier, but simplifies what we do in the JSScript::finalizer. It will also stop us from cloning the filename for each inner function while running code coverage.

Comment 2

[Ted Campbell [:tcampbell]]

Attachment: Bug 1578730 - Add js::GetCodeCovergeSummaryAll API. r?nbp,marco

Split the GetCodeCoverageSummary API into a variant for a specific realm
vs checking all realms. This restores the original behaviour of the
getLcovInfo testing function to only return info on current realm. This
makes testing OOM behaviour much more predictable.

Comment 3

[Ted Campbell [:tcampbell]]

Attachment: Bug 1578730 - Run JSScript::initScriptName on main-thread only. r?nbp

Avoid creating the ScriptNameMap in the off-thread parse zone. Instead
perform JSScript::initScriptName when merging the parse result to main
thread. This will avoid the need to migrate coverage initialization data
between Realms.

Depends on D46167

Comment 4

[Ted Campbell [:tcampbell]]

Attachment: Bug 1578730 - Allocate LCovRealm on first use. r?nbp

Save a little bit of memory when code coverage is not in use. Add a
collectCodeCoverage method to the realm so it can allocate LCovRealm on
demand. Also compute the realm name string from the constructor instead
of in the lookup code (this still happens at same time).

Depends on D46168

Comment 5

[Ted Campbell [:tcampbell]]

Attachment: Bug 1578730 - Use vector of pointers for LCovRealm::sources. r?nbp

Now that LCovRealm is allocated on demand, use a Vector directly instead
of allocating that vector on demand. To ensure pointer stability of
LCovSources, allocated them in the LifoAlloc and only store pointers in
the sources vector. They are still fully owned by the LCovRealm.

Depends on D46169

Comment 6

[Ted Campbell [:tcampbell]]

Attachment: Bug 1578730 - Defer LCovSource::writeScript OOM flag. r?nbp

To prepare for later patches, track writeScript OOMs as a flag inside
LCovSource. The objective is to be able to call LCovSource::writeScript
directly from the JSScript finalizer without needing access to the
current realm.

Depends on D46170

Comment 7

[Ted Campbell [:tcampbell]]

Attachment: Bug 1578730 - Move ScriptNameMap to js/src/vm/CodeCoverage files. r?nbp

Remove LCovRealm::collectCodeCoverageInfo and directly call
LCovSource::writeScript from JSScript::finalize and others. In later
patches we will replace the script-name map with a LCovSource map.

Depends on D46171

Comment 8

[Ted Campbell [:tcampbell]]

Attachment: Bug 1578730 - Track script-associated LCovSource instead of filename. r?nbp

Rename the ScriptNameMap to ScriptLCovMap and associate LCovSource* with
each script rather than a private clone of filename. This removes the
need for JSScript::finalize to need access to JSScript::realm().

Depends on D46172

Comment 9

[Ted Campbell [:tcampbell]]

The overall objective here is to remove the calls to JSScript::realm() from JSScript::finalize. The result of this patch stack is for a JSScript to be associated with an LCovSource* during initialization. During finalization, the coverage counts for a script are accumulated into the LCovSource.

The first patch (Add js::GetCodeCoverageSummaryAll API) fixes an issue where coverage/bug1214548.js would timeout. The issue was that js::GetCodeCoverageSummary was collecting coverage data beyond just the correct realm.

The second patch (Run JSScript::initScriptName on main-thread only) tries to avoid generating any of the coverage tables in the off-thread parse Zone and defers calling initScriptName until we merge the off-thread parse task back to main thread.

The rest of the patches cleanup LCovRealm/LCovSource so that having a LCovSource* is sufficient to finalize a script. I kept it in parts to make it easier to see progression, but I can also squash if that is clearer.

Comment 10

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/7581d5468864
Add js::GetCodeCovergeSummaryAll API. r=nbp,marco
https://hg.mozilla.org/integration/autoland/rev/985febe1e1a7
Run JSScript::initScriptName on main-thread only. r=nbp
https://hg.mozilla.org/integration/autoland/rev/19172ad2dea7
Allocate LCovRealm on first use. r=nbp
https://hg.mozilla.org/integration/autoland/rev/910a125e41dc
Use vector of pointers for LCovRealm::sources. r=nbp

Comment 11

[Stefan Hindli [:stefan_hindli]]

https://hg.mozilla.org/mozilla-central/rev/7581d5468864
https://hg.mozilla.org/mozilla-central/rev/985febe1e1a7
https://hg.mozilla.org/mozilla-central/rev/19172ad2dea7
https://hg.mozilla.org/mozilla-central/rev/910a125e41dc

Comment 12

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/8d80982a429f
Defer LCovSource::writeScript OOM flag. r=nbp
https://hg.mozilla.org/integration/autoland/rev/b194fe05c555
Move ScriptNameMap to js/src/vm/CodeCoverage files. r=nbp
https://hg.mozilla.org/integration/autoland/rev/8a7f8a71391f
Track script-associated LCovSource instead of filename. r=nbp

Comment 13

[Cosmin Sabou [:CosminS]]

https://hg.mozilla.org/mozilla-central/rev/8d80982a429f
https://hg.mozilla.org/mozilla-central/rev/b194fe05c555
https://hg.mozilla.org/mozilla-central/rev/8a7f8a71391f