1580020 - Defer GC allocation of BigInts until afer parse is finished.

Status: RESOLVED FIXED

(Core :: JavaScript Engine, task, P1)

Description

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

Currently BigInts are allocated as they are parsed, but it would be good to defer them till after parse to help create a pure-parser.

Comment 1

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

Attachment: Bug 1580020 - Defer BigInt allocation in deferred mode r?tcampbell

This is a rough draft: There are some issues that need to be decided upon
(below) but I wanted to get this up to ensure the larger level design pieces
pass muster before going too far down the road.

1. It's not clear to me how this is going to interact with the parser error
   handling system. It seems to me that this should succeed based on the
   comment above ParseBigIntLiteral that says "Called from parser with already
   trimmed and validated token.", along with the release assert that there's no
   parse error 1

2. This will raise the high water mark of a parse, because we hold onto a copy
   of the CharBuffer. There's a hard upper limit of the memory increase
   possible of 2x, but, nevertheless, this could be impactful on code that
   uses BigInt literals extensively. I don't know that it's avoidable with the
   current tokenizer design, as it seems that this buffer already has all the
   escapes processed; the only alternative would be to save source coordinates
   however, I couldn't seem to figure out how we could get that data from our
   current token implementation.

3. This is my first time attempting to write move constructors. I'll be honest,
   I mostly scattered && and std::move() until it worked, but I have no
   idea what the idiomatic way would be to express the moves I am trying to
   express.

Comment 2

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

Attachment: Bug 1580020 - Mark publishDeferredItems as MOZ_MUST_USE to prevent misuse r?tcampbell

Depends on D45556

Comment 3

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

Attachment: Bug 1580020 - Switch ParseContext to consume a ParseInfo directly, rather than decomposing r?tcampbell

Also breaks UsedNameTracker into its own header

Comment 4

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

Attachment: Bug 1580020 - Simplify AsmJS handling for deferred allocation r?tcampbell

Comment 5

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/197706178905
Mark publishDeferredItems as MOZ_MUST_USE to prevent misuse r=tcampbell
https://hg.mozilla.org/integration/autoland/rev/6731b16798c1
Switch ParseContext to consume a ParseInfo directly, rather than decomposing r=tcampbell
https://hg.mozilla.org/integration/autoland/rev/14dcedb553f4
Simplify AsmJS handling for deferred allocation r=tcampbell

Comment 6

[Arthur Iakab [arthur_iakab]]

https://hg.mozilla.org/mozilla-central/rev/197706178905
https://hg.mozilla.org/mozilla-central/rev/6731b16798c1
https://hg.mozilla.org/mozilla-central/rev/14dcedb553f4

Comment 7

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/4af5b90d68a3
Defer BigInt allocation in deferred mode r=tcampbell,jwalden

Comment 8

[Stefan Hindli [:stefan_hindli]]

Backed out changeset 4af5b90d68a3 (Bug 1580020) for causing a build bustage in /builds/worker/workspace/build/src/js/src/frontend/ParseNode.h CLOSED TREE

https://treeherder.mozilla.org/#/jobs?repo=autoland&resultStatus=testfailed%2Cbusted%2Cexception&classifiedState=unclassified&selectedJob=268618737

https://treeherder.mozilla.org/logviewer.html#?job_id=268618737&repo=autoland

Comment 9

[Pulsebot]

Backout by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/858991b684ef
Backed out changeset 4af5b90d68a3 for causing a build bustage in /builds/worker/workspace/build/src/js/src/frontend/ParseNode.h CLOSED TREE

Comment 10

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/67ab1fa79ea9
Defer BigInt allocation in deferred mode r=tcampbell,jwalden

Comment 11

[Stefan Hindli [:stefan_hindli]]

https://hg.mozilla.org/mozilla-central/rev/67ab1fa79ea9