Closed Bug 1585372 Opened 5 years ago Closed 5 years ago

Compute LCov function name before finalization

Categories

(Core :: JavaScript Engine, task, P2)

task

Tracking

()

RESOLVED FIXED
mozilla72
Tracking Status
firefox72 --- fixed

People

(Reporter: tcampbell, Assigned: tcampbell)

References

Details

Attachments

(2 files)

Currently we access script->functionNonDelazifying()->displayAtom() during finalization with code-coverage enabled. This is currently safe because we enforce GC ordering that JSScript is torn-down before js::Scope. In order to remove the dependency we should format the script name when run coverage::InitScriptCoverage instead of coverage::CollectScriptCoverage.

This will increase memory usage slightly running coverage to store the function name, but we recently removed the copy of filename() per-function that we were storing during coverage. The net impact should still be a memory savings versus a month ago.

An oversight I made here is that the displayAtom() isn't available when we call coverage::InitScriptCoverage. I'm considering handling this similarly to how I fixed off-thread recently in [1]. This would also get away from having to handle uncompleted scripts in coverage.

[1] https://searchfox.org/mozilla-central/rev/f372e8a46ef7659ef61be9938ec2a3ea34d343c6/js/src/vm/HelperThreads.cpp#1902-1936

Blocks: 1591405
No longer blocks: 1583816

To allow InitScriptCoverage to be more flexible, we need to delay the call
until the JSScript is actually initialized. This moves the calls from
JSScript constructor until the JSScript is actually initialized. This is
similar to the debugger onNewScript call, but more cleanup would be needed to
unify these.

Depends on D51472

To avoid calling JSScript::function() from finalizer, we need to read the
name earlier in process.

Depends on D51473

Pushed by tcampbell@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/b3a76c2b11b3
Run InitScriptCoverage after initialization. r=jandem,nbp
https://hg.mozilla.org/integration/autoland/rev/dfd48b94b745
Compute script name in InitScriptCoverage. r=nbp
Status: ASSIGNED → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla72
Regressions: 1597997
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: