1569315 - Defer allocation of JSFunctions until after Parsing is done.

Status: RESOLVED FIXED

(Core :: JavaScript Engine, enhancement, P1)

Description

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

To further our goal of decoupling the parser for the GC, we'd like to be able to avoid creating JSFunctions until after the parser is done its business.

Comment 1

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

Attachment: Bug 1569315 - Add visitor system to FunctionTree r?tcampbell

Depends on D38699

Comment 2

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

Attachment: Bug 1569315 - Sink prototype fetching into AllocNewFunction r?tcampbell

Depends on D39769

Comment 3

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

Attachment: Bug 1569315 - Factor out FunctionCreationData from AllocNewFunction r?tcampbell

Depends on D39770

Comment 4

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

Attachment: Bug 1569315 - Factor FunctionCreationData into its own header r?tcampbell

Depends on D39771

Comment 5

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

Attachment: Bug 1569315 - Create FunctionFlags class to allow recycling function flag queries elsewhere r?tcampbell

Depends on D39772

Comment 6

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

Attachment: Bug 1569315 - Defer allocation of JSFunctions in the parser r?tcampbell

Depends on D39773

Comment 7

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

Attachment: Bug 1569315 - Defer function allocation for field initializers r?tcampbell

Depends on D39774

Comment 8

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

Attachment: Bug 1569315 - Defer mainline function allocations until after parse r?tcampbell

Remaining are the stand-alone parses, and a dummy function allocation
for Asm.JS I think

Depends on D39775

Comment 9

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

Kicked off some Performance test results (--rebuild=10)

• AWSY Results: No major memory impacts

• Raptor Results: almost all results are low confidence with the exception of two webaudio tests which see 2% degradations. It does seem like there's something real here, as there's some high confidence degradations reported in the subtests. I do think that the extreme quantization of the scores suggests the effect may be magnified by quantization though.

Comment 10

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/3f46633be59a
Sink prototype fetching into AllocNewFunction r=tcampbell
https://hg.mozilla.org/integration/autoland/rev/86c50e8added
Create FunctionFlags class to allow recycling function flag queries elsewhere r=tcampbell

Comment 11

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

Attachment: Bug 1569315 - Distinguish between AsmJS clobber function and deferred allocation initialization r?tcampbell

Depends on D39776

Comment 12

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

Attachment: Bug 1569315 - Use TraceNullableRoot for atom which can in fact be null r?tcampbell

Comment 13

[Razvan Maries]

https://hg.mozilla.org/mozilla-central/rev/3f46633be59a
https://hg.mozilla.org/mozilla-central/rev/86c50e8added

Comment 14

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/5d553fb84ecb
Factor out FunctionCreationData from AllocNewFunction r=tcampbell,jonco
https://hg.mozilla.org/integration/autoland/rev/eb0b2f561461
Factor FunctionCreationData into its own header r=tcampbell

Comment 15

[Pulsebot]

Backout by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/be22f622aeea
Backed out 2 changesets on request from mgaudet on a CLOSED TREE

Comment 16

[Andreea Pavel [:apavel]]

Backed out on request

Push with build bustages: https://treeherder.mozilla.org/#/jobs?repo=autoland&resultStatus=testfailed%2Cbusted%2Cexception&revision=eb0b2f561461461e5dde736180a463ee297e13be&selectedJob=259460806

Failure log: https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=259460806&repo=autoland&lineNumber=7406

Backout: https://hg.mozilla.org/integration/autoland/rev/be22f622aeead6c162ab348c1daec5d2401bd7d8

Comment 17

[Andreea Pavel [:apavel]]

There are also all types of test failures on this push and the next one:

https://treeherder.mozilla.org/#/jobs?repo=autoland&group_state=expanded&resultStatus=testfailed%2Cbusted%2Cexception&revision=9c430a684fe03d185f8f21d3ad23960d9e943c4a&failure_classification_id=2&selectedJob=259475660

and

https://treeherder.mozilla.org/#/jobs?repo=autoland&group_state=expanded&resultStatus=testfailed%2Cbusted%2Cexception&revision=eb0b2f561461461e5dde736180a463ee297e13be&failure_classification_id=2&selectedJob=259467906

Comment 18

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/a8a2410665d2
Factor out FunctionCreationData from AllocNewFunction r=tcampbell,jonco
https://hg.mozilla.org/integration/autoland/rev/236dacd806bf
Factor FunctionCreationData into its own header r=tcampbell

Comment 19

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

Attachment: Bug 1569315 - Make FunctionTree processing either Eager or Deferred. r?tcampbell

This makes eager processing the default as well, which makes the code
effectively work how it does before the FunctionTree patches.

Depends on D39774

Comment 20

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

Attachment: Bug 1569315 - Clarify GC status of FunctionCreationData r?tcampbell

Comment 21

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

Attachment: Bug 1569315 - Unify flags access code in FunctionFlags r?tcampbell

Comment 22

[Oana Pop-Rus]

https://hg.mozilla.org/mozilla-central/rev/a8a2410665d2
https://hg.mozilla.org/mozilla-central/rev/236dacd806bf

Comment 23

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/2a86de9ceb74
Defer allocation of JSFunctions in the parser r=tcampbell
https://hg.mozilla.org/integration/autoland/rev/82baf5ba9e0e
Make FunctionTree processing either Eager or Deferred. r=tcampbell
https://hg.mozilla.org/integration/autoland/rev/d17a7b268784
Clarify GC status of FunctionCreationData r=tcampbell
https://hg.mozilla.org/integration/autoland/rev/215ea735bc32
Unify flags access code in FunctionFlags r=tcampbell

Comment 24

[Andrei Ciure[:aciure]]

https://hg.mozilla.org/mozilla-central/rev/2a86de9ceb74
https://hg.mozilla.org/mozilla-central/rev/82baf5ba9e0e
https://hg.mozilla.org/mozilla-central/rev/d17a7b268784
https://hg.mozilla.org/mozilla-central/rev/215ea735bc32

Comment 25

[Ted Campbell [:tcampbell]]

Attachment: Bug 1569315 - Follow-up formatting nits. r?iain

Comment 26

[Ted Campbell [:tcampbell]]

Attachment: Bug 1569315 - Fix additional nits around JSFunction allocation deferral. r?iain

Depends on D41060

Comment 27

[Ted Campbell [:tcampbell]]

I'm doing some follow-up tweaking that we didn't get finished before Matt went on leave. One important detail is that the current code in mozilla-central always runs in 'eager' mode which matches the existing parser behaviour. There is no immediate win with just the parser decoupled, so we will take a little more time to iterate on it.

Comment 28

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/8dda9d1aa824
Follow-up formatting nits. r=iain
https://hg.mozilla.org/integration/autoland/rev/c27593789b12
Fix additional nits around JSFunction allocation deferral. r=iain

Comment 29

[Natalia Csoregi [:nataliaCs]]

https://hg.mozilla.org/mozilla-central/rev/8dda9d1aa824
https://hg.mozilla.org/mozilla-central/rev/c27593789b12