Closed Bug 1565945 Opened 5 years ago Closed 5 years ago

Prepare to merge JSScript and LazyScript

Categories

(Core :: JavaScript Engine, enhancement, P2)

Product:
Core
Component:
JavaScript Engine
Type:
enhancement

Tracking

()

Status:
RESOLVED FIXED
Milestone:
mozilla70
Tracking Flags:
Tracking Status
firefox70 --- fixed

People

(Reporter: tcampbell, Assigned: tcampbell)

References

Details

Attachments

(5 files)

Bug 1565945 - Add JSScript::CreateFromLazy method. r?jandem
(deleted), text/x-phabricator-request
Details
Bug 1565945 - Fix PrivateScriptData::trace handling of ClearEdgesTracer. r?jonco
(deleted), text/x-phabricator-request
Details
Bug 1565945 - Compute LazyScript::TreatAsRunOnce flag the same as JSScript. r?jandem
(deleted), text/x-phabricator-request
Details
Bug 1565945 - Remove JSScript HasSingletons flag. r?jandem
(deleted), text/x-phabricator-request
Details
Bug 1565945 - Make ImmutableFlags consistent between LazyScript and JSScript. r?jandem
(deleted), text/x-phabricator-request
Details

Here are a few patches needed for Bug 1529456 that bring LazyScript and JSScript closer together in our current system.

Decouple the JSScript creation for delazification from needing
CompileOptions. When JSScript and LazyScript become unified, this
operation will become a no-op.

When using ClearEdgesTracer on PrivateScriptData, we must erase the
types of the GCCellPtr to avoid a number of internal assertions. This is
needed to be able to release PrivateScriptData outside of the JSScript
finalizer.

Depends on D38023

This has equivalent overall behavior as before, but computes the
precise LazyScript::TreatAsRunOnce flag rather than checking the extra
conditions during delazification. The heuristics can be computed using
just the LazyScript information so the result is the same.

Depends on D38024

Keywords: leave-open
Pushed by tcampbell@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/cee2f9e4e7c1 Add JSScript::CreateFromLazy method. r=jandem https://hg.mozilla.org/integration/autoland/rev/61e233285734 Compute LazyScript::TreatAsRunOnce flag the same as JSScript. r=jandem

This flag is only checked by the eval-cache which currently disallows
any inner function/object. If the HasSingletons flag was set, we'd
already have inner objects so the effect is still the same without.

When flags are derived from the FunctionBox, we should consistently set
them for both LazyScript and JSScript even if their primary use it
hinting delazification.

Pushed by tcampbell@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/6a9f494b6961 Remove JSScript HasSingletons flag. r=jandem https://hg.mozilla.org/integration/autoland/rev/c8886b864823 Make ImmutableFlags consistent between LazyScript and JSScript. r=jandem
Pushed by tcampbell@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/c8100315938b Fix PrivateScriptData::trace handling of ClearEdgesTracer. r=jonco

That's enough pieces for this bug.

Keywords: leave-open

Arai, one of the remaining JSScript::ImmutableFlags inconsistencies is the IsBinAST flag. This is currently only used for debugging and only valid on LazyScript. Should we remove the flag in favour of checking ScriptSource->hasBinASTSource()? Or is it straightforward for BinAST JSScript to set the flag as well during frontend?

Flags: needinfo?(arai.unmht)

Removing IsBinAST flag and using ScriptSource->hasBinASTSource() instead sounds reasonable.

Flags: needinfo?(arai.unmht)

https://hg.mozilla.org/mozilla-central/rev/c8100315938b

Status: ASSIGNED → RESOLVED
Closed: 5 years ago
status-firefox70: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla70

(In reply to Tooru Fujisawa [:arai] from comment #13)

Removing IsBinAST flag and using ScriptSource->hasBinASTSource() instead sounds reasonable.

I opened Bug 1567221 to do that.

You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: