1642708 - Remove BinAST prototype

Status: RESOLVED FIXED

(Core :: JavaScript Engine, task, P2)

Description

[Ted Campbell [:tcampbell]]

The BinAST code in tree is a prototype that is not practical to ship in its current form and interferes with ongoing work. Until the ongoing SmooshMonkey and Stencil work ships, we should remove the BinAST prototype from tree to avoid wasting time maintaining an intermediate form.

Any future BinAST work will likely be based on the upcoming SmooshMonkey parser/emitter. The encoder and reference designs continue to live on in https://github.com/binast for the meanwhile.

Code to remove:

• js/src/frontend/BinAST* and all code guarded by JS_BUILD_BINAST
  • These will all need major rewrites before we ship them
• js/src/jsapi-test/tests/binast
  • These are mostly auto-generated with a few hand-written tests
• js/src/jit-test/tests/binast
  • These were auto-generated for normal jit-tests
• Vendored Rust code used by js/src/frontend/binast
  • These remain on external repositories

Automation to remove:

• The BinAST tests in SM(p) and SM(cgc) builds
• The Raptor Rap(inst-b), Rap(inst-b-c) performance tasks

Comment 1

[Ted Campbell [:tcampbell]]

Attachment: Bug 1642708 - Stop running BinAST tests in automation

Remove the BinAST tests from following jobs:
- SM(p)
- SM(cgc)

Remove the BinAST performance tests:
- binast-instagram

Skip the WPT BinAST tests

Comment 2

[Ted Campbell [:tcampbell]]

Attachment: Bug 1642708 - Disable BinAST prototype

Depends on D77919

Comment 3

[Ted Campbell [:tcampbell]]

Attachment: Bug 1642708 - Remove BinAST tests

Remove directories:
js/src/jsapi-tests/binast/
js/src/jit-test/tests/binast/

Remove additional tests:
js/src/fuzz-tests/testBinASTReader.cpp
js/src/jsapi-tests/testBinASTReader.cpp

Depends on D77920

Comment 4

[Ted Campbell [:tcampbell]]

Attachment: Bug 1642708 - Remove code guarded by JS_BUILD_BINAST

Also remove js/src/frontend/BinAST* sources.

Depends on D77944

Comment 5

[Ted Campbell [:tcampbell]]

Attachment: Bug 1642708 - Remove more references to BinAST code

Depends on D77945

Comment 6

[Ted Campbell [:tcampbell]]

Attachment: Bug 1642708 - Remove BinAST Rust modules

Depends on D77946

Comment 7

[Nicolas B. Pierron [:nbp]]

(CC-ing the persons who worked on BinAST)

Ted, any reason why this bug should be confidential?

Comment 8

[Ted Campbell [:tcampbell]]

I'm waiting on a few more discussions to happen and final approval that this is the best approach. I'll make sure the bug is open before any reviews actually start. These patches were to understand the scope of change.

Comment 9

[Ted Campbell [:tcampbell]]

We intend to remove the prototype code to reduce development complexity of Bug 1619282 and Bug 1601332. In future we will revisit what we will about BinAST implementation (which would likely be based on the new frontend).

Comment 10

[Ted Campbell [:tcampbell]]

Removing the jit-tests will need to happen manually (because it is too big for Lando) by removing the following paths:

js/src/jsapi-tests/binast/
js/src/jit-test/tests/binast/

I'll need a sheriff to do this manually once the rest of this lands. This does not need to happen at the same time though.

Comment 11

[Ted Campbell [:tcampbell]]

Marking leave-open until Comment 10 is handled.

Comment 12

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/ad613fa94a83
Stop running BinAST tests in automation r=perftest-reviewers,Bebe,sfink
https://hg.mozilla.org/integration/autoland/rev/1c245d4e8244
Disable BinAST prototype r=arai
https://hg.mozilla.org/integration/autoland/rev/bcc2728b5ca5
Remove code guarded by JS_BUILD_BINAST r=arai
https://hg.mozilla.org/integration/autoland/rev/d210a60ad435
Remove more references to BinAST code r=arai
https://hg.mozilla.org/integration/autoland/rev/d307b00c7e1b
Remove BinAST Rust modules r=arai,emilio

Comment 13

[Natalia Csoregi [:nataliaCs]]

Backed out for build bustages

backout: https://hg.mozilla.org/integration/autoland/rev/fa6512a9584891f866030fe0d70abbf4d86f7451

push: https://treeherder.mozilla.org/#/jobs?repo=autoland&group_state=expanded&revision=d307b00c7e1b55efa6e2279784aa838fe6f82979&searchStr=build&selectedTaskRun=cbuXU1e3RLO97Zb9QxOIeg.0

failure log: https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=306646121&repo=autoland&lineNumber=18724

Comment 14

[Ted Campbell [:tcampbell]]

The dom staticpref include in nsJSUtils.h ends up being used in other places. This was regressed by Bug 1508306, but I'll put the header back in this stack so this can be landed.

Comment 15

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/6ea7e854f8c7
Stop running BinAST tests in automation r=perftest-reviewers,Bebe,sfink
https://hg.mozilla.org/integration/autoland/rev/37322ca8d1ed
Disable BinAST prototype r=arai
https://hg.mozilla.org/integration/autoland/rev/2035099033f2
Remove code guarded by JS_BUILD_BINAST r=arai
https://hg.mozilla.org/integration/autoland/rev/0a60834b51a7
Remove more references to BinAST code r=arai
https://hg.mozilla.org/integration/autoland/rev/36790714faf2
Remove BinAST Rust modules r=arai,emilio

Comment 16

[Ted Campbell [:tcampbell]]

Attachment: Bug 1642708 - Add missing StaticPrefs_dom.h include in DocumentLoadListener

Comment 17

[Cristian Brindusan [:cbrindusan]]

https://hg.mozilla.org/mozilla-central/rev/6ea7e854f8c7
https://hg.mozilla.org/mozilla-central/rev/37322ca8d1ed
https://hg.mozilla.org/mozilla-central/rev/2035099033f2
https://hg.mozilla.org/mozilla-central/rev/0a60834b51a7
https://hg.mozilla.org/mozilla-central/rev/36790714faf2

Comment 18

[Ted Campbell [:tcampbell]]

Attachment: Bug 1642708 - Remove generated BinAST tests

Here is the patch to remove the jit-test. It removes the follow directories:

• js/src/jsapi-tests/binast/
• js/src/jit-test/tests/binast/

You should be able to r+ this on bugzilla and we will have a sheriff graft from try to autoland.

Comment 19

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/b2ce549cea27
Add missing StaticPrefs_dom.h include in DocumentLoadListener r=necko-reviewers,nika,valentin

Comment 20

[Cosmin Sabou [:CosminS]]

Backed out for causing build bustages on PermissionDelegateHandler.cpp.

Push with failures: https://treeherder.mozilla.org/#/jobs?repo=autoland&group_state=expanded&resultStatus=testfailed%2Cbusted%2Cexception&revision=b2ce549cea279b4bdbf76b6e333f4f4293d8446b&searchStr=build&selectedTaskRun=c09mTL0jTTC7aAexC5HhUQ.0

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

Backout link: https://hg.mozilla.org/integration/autoland/rev/c7fc7af7052239d58cfd6bcd6e2f2fa31de2b561

Comment 21

[Ted Campbell [:tcampbell]]

Will leave the StaticPrefs_dom.h header alone. Too many accidental uses have slipped into the unified builds depending on chunking.

Comment 22

[Tooru Fujisawa [:arai]]

Comment on attachment 9157489 [details]
Bug 1642708 - Remove generated BinAST tests

sorry for the delay.

Comment 23

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/d98c157b7e08
Remove generated BinAST tests. r=arai CLOSED TREE

Comment 24

[Ted Campbell [:tcampbell]]

After this last patch merges to m-c, this issue can be closed out.

Comment 25

[Narcis Beleuzu [:NarcisB]]

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