1607854 - Prepare to defer allocation of JSScript to just before JSScript::fullyInitFromEmitter

Status: RESOLVED FIXED

(Core :: JavaScript Engine, task, P2)

Description

[Matthew Gaudet (he/him) [:mgaudet]]

By the end of this bug I think JSScript::create and JSScript::fullyInitFromEmitter ought to collapse together.

This is surprisingly within reach already (not to say there are no blockers, but it's not as far away as I believed) because it turns out that the BytecodeEmitter does relatively few things with the passed in JSScript:

• BytecodeEmitter::checkSingletonContext
• Field-initalizers are set as part of emitFunctionScript
• Checking script->noScriptRval()
• setTreatAsRunOnce.

Comment 1

[Matthew Gaudet (he/him) [:mgaudet]]

Attachment: Bug 1607854 - Use parser-options for noScriptRval where possible r?tcampbell

Most of the complexity of this patch is trying to preserve the same level of
assertion in the future case where a BytecodeEmitter may or may not have a
pre-allocated script.

Comment 2

[Matthew Gaudet (he/him) [:mgaudet]]

Attachment: Bug 1607854 - Set field initializers as part of FunctionScriptEmitter::initScript r?djvj

Depends on D60712

Comment 3

[Matthew Gaudet (he/him) [:mgaudet]]

Attachment: Bug 1607854 - Don't use Script to access ScriptSource object in BytecodeCompiler r?djvj

Depends on D60713

Comment 4

[Matthew Gaudet (he/him) [:mgaudet]]

Attachment: Bug 1607854 - Centralize forwarding of LazyScript flags to CreateFromLazy r?djvj

Depends on D60714

Comment 5

[Matthew Gaudet (he/him) [:mgaudet]]

Marking as leave open as the first batch won't finish this.

Comment 6

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/9cffcfb914d7
Use parser-options for noScriptRval where possible r=tcampbell
https://hg.mozilla.org/integration/autoland/rev/04f020697732
Set field initializers as part of FunctionScriptEmitter::initScript r=djvj
https://hg.mozilla.org/integration/autoland/rev/24fd4240a3cf
Don't use Script to access ScriptSource object in BytecodeCompiler r=djvj
https://hg.mozilla.org/integration/autoland/rev/03c30bc4fb01
Centralize forwarding of LazyScript flags to CreateFromLazy r=djvj

Comment 7

[Razvan Maries]

https://hg.mozilla.org/mozilla-central/rev/9cffcfb914d7
https://hg.mozilla.org/mozilla-central/rev/04f020697732
https://hg.mozilla.org/mozilla-central/rev/24fd4240a3cf
https://hg.mozilla.org/mozilla-central/rev/03c30bc4fb01

Comment 9

[Matthew Gaudet (he/him) [:mgaudet]]

Attachment: Bug 1607854 - Create ScriptAllocationStencil to represent the data required to allocate a script

Also use it to allocate the JSScript inside of BytecodeEmitter::emitFunction

Comment 10

[Matthew Gaudet (he/him) [:mgaudet]]

Attachment: Bug 1607854 - Use ScriptAllocationStencil to power InternalCreateScript

Depends on D64789

Comment 11

[Matthew Gaudet (he/him) [:mgaudet]]

Attachment: Bug 1607854 - Use ScriptAllocationStencil in CompileGlobalBinASTScriptImpl

Depends on D64790

Comment 12

[Matthew Gaudet (he/him) [:mgaudet]]

Attachment: Bug 1607854 - Use ScriptAllocationStencil to handle LazyScript creation.

This is a prepatory step to deferring JSScript allocation.

Depends on D64793

Comment 13

[Matthew Gaudet (he/him) [:mgaudet]]

Attachment: Bug 1607854 - Make input and output aspects of the script management in CompilationInfo clearer.

Depends on D64794

Comment 14

[Matthew Gaudet (he/him) [:mgaudet]]

Attachment: Bug 1607854 - Defer allocation of JSScript to just before initialization

Use ScriptAllocationStencil inside of BytecodeEmitter instead of Script, and allocate on demand.

Depends on D64796

Comment 15

[Matthew Gaudet (he/him) [:mgaudet]]

Uploaded my current-state-of-the-world patches.

Honestly would probably fold this stack for landing, as the intermediate patches change GC handling three times.

But I wanted this up to use as a discussion point (and also, if someone else starts looking at this they have a starting point)

Comment 16

[Matthew Gaudet (he/him) [:mgaudet]]

Attachment: Bug 1607854 - Merge BCEScriptStencil and ScriptAllocationStencil r?tcampbell

One thing this patch makes pretty clear is that the story for manipulating
immutable flags is very confusing in the front end. For example, we have
flags on the functionbox, but also flags on the BCEScriptStencil, and they
need to get unioned together at script allocation time.

Comment 17

[Matthew Gaudet (he/him) [:mgaudet]]

Attachment: Bug 1607854 - Create ScriptAllocationStencil to represent the data required to allocate a script r?tcampbell

Also use it to allocate the JSScript inside of BytecodeEmitter::emitFunction

Comment 18

[Matthew Gaudet (he/him) [:mgaudet]]

Attachment: Bug 1607854 - Make input and output aspects of the script management in CompilationInfo clearer. r?tcampbell

Depends on D68253

Comment 19

[Matthew Gaudet (he/him) [:mgaudet]]

Attachment: Bug 1607854 - Defer allocation of JSScript to just before initialization r?tcampbell

Use ScriptAllocationStencil inside of BytecodeEmitter instead of Script, and allocate on demand.

Depends on D68254

Comment 20

[Matthew Gaudet (he/him) [:mgaudet]]

Attachment: Bug 1607854 - Minimize dependency on BytecodeEmitter member of BCEScriptStencil

This is preparation for disconnecting BCEScriptStencil from the BytecodeEmitter to allow subsequent batch initialization

Comment 21

[Phabricator Automation]

Comment on attachment 9135794 [details]
Bug 1607854 - Minimize dependency on BytecodeEmitter member of BCEScriptStencil

Revision D68266 was moved to bug 1618631. Setting attachment 9135794 [details] to obsolete.

Comment 22

[Matthew Gaudet (he/him) [:mgaudet]]

Attachment: Bug 1607854 - Add operator== support to SourceExtent r?tcampbell

Note: This stack doesn't add any uses of this, but this was invaluable in the
development of this stack, and it was the second time I had implmented this and
not landed it.

Comment 23

[Matthew Gaudet (he/him) [:mgaudet]]

Attachment: Bug 1607854 - Add a creation extent to SharedContext to drive JSScript allocation r?tcampbell

Depends on D71997

Comment 24

[Matthew Gaudet (he/him) [:mgaudet]]

Attachment: Bug 1607854 - Use shared context immutable flags for doing JSScript::Create r?tcampbell

Depends on D71998

Comment 25

[Matthew Gaudet (he/him) [:mgaudet]]

Attachment: Bug 1607854 - Clarify BytecodeEmitter input output a bit r?tcampbell

Depends on D71999

Comment 26

[Matthew Gaudet (he/him) [:mgaudet]]

Attachment: Bug 1607854 - Defer allocation of JSScript to just before JSScript::fullyInitFromEmitter r?tcampbell

Depends on D72000

Comment 27

[Matthew Gaudet (he/him) [:mgaudet]]

Splitting this bug to avoid patches going into multiple releases under the same bug.

Retitle as "Prepare -"

Comment 28

[Phabricator Automation]

Comment on attachment 9142456 [details]
Bug 1607854 - Add operator== support to SourceExtent r?tcampbell

Revision D71997 was moved to bug 1632260. Setting attachment 9142456 [details] to obsolete.

Comment 29

[Phabricator Automation]

Comment on attachment 9142457 [details]
Bug 1607854 - Add a creation extent to SharedContext to drive JSScript allocation r?tcampbell

Revision D71998 was moved to bug 1632260. Setting attachment 9142457 [details] to obsolete.

Comment 30

[Phabricator Automation]

Comment on attachment 9142459 [details]
Bug 1607854 - Use shared context immutable flags for doing JSScript::Create r?tcampbell

Revision D71999 was moved to bug 1632260. Setting attachment 9142459 [details] to obsolete.

Comment 31

[Phabricator Automation]

Comment on attachment 9142460 [details]
Bug 1607854 - Clarify BytecodeEmitter input output a bit r?tcampbell

Revision D72000 was moved to bug 1632260. Setting attachment 9142460 [details] to obsolete.

Comment 32

[Phabricator Automation]

Comment on attachment 9142461 [details]
Bug 1607854 - Defer allocation of JSScript to just before JSScript::fullyInitFromEmitter r?tcampbell

Revision D72001 was moved to bug 1632260. Setting attachment 9142461 [details] to obsolete.