1669784 - Remove TypedObject code not required for GC-MVP

Status: RESOLVED FIXED

(Core :: JavaScript: WebAssembly, task, P3)

Description

[Ryan Hunt [:rhunt]]

We want to use the TypedObject API for the basis of our GC-MVP JS-API, but there's a bunch of features we don't need or will need to change. This bug is for cutting all features not required by our current GC prototype. This will make it much easier to turn it into what we need going forward.

Comment 1

[Ryan Hunt [:rhunt]]

The major changes I have so far:

1. Remove all notions of opaque vs. transparent. Most of this code was already gone.
2. Remove all helper methods from prototype chains (.array, .equivalent, .forEach, .toSource)
3. Simplify prototype chain so all StructType/ArrayType's don't share a prototype
4. Disallow directly nested struct/arrays without indirection
5. Disallow setting fields/elements from JS
6. Move implementation of getting fields/elements to native code, removing last need for self-hosted code
7. Disallow creating StructType/ArrayType from JS
8. Remove TypedObject namespace disallowing direct access to .uint8, .int8, .object, .StructType, .ArrayType, etc.
9. Remove support for types not needed by Wasm, such as string, uint32, uint64
10. Remove JS_HAS_TYPED_OBJECTS feature code and use Wasm GC feature code
11. Remove all typed object API tests, update Wasm GC tests for slight incompatibilities

I think this is a good starting point, so I'll post the patches here soon.

Comment 2

[Ryan Hunt [:rhunt]]

Attachment: Bug 1669784 - Remove opaque/transparent split from TypedObjects. r?jandem

We want all TypedObjects to be opaque so kill all code related to transparent
TypedObjects.

Comment 3

[Ryan Hunt [:rhunt]]

Attachment: Bug 1669784 - Remove helper methods from TypedObjects. r?jandem

We may want some of these in the future, but for now they're a maintenance burden.

Depends on D92849

Comment 4

[Ryan Hunt [:rhunt]]

Attachment: Bug 1669784 - Disallow nested TypedObjects without an indirection. r?jandem

A future extension to the GC proposal may allow flattened data structures and
interior pointers, but for now let's remove support for this by disallowing it
at the type level.

The reading/writing code will be rewritten/removed in a later commit so no
significant simplifications are made there.

Depends on D92850

Comment 5

[Ryan Hunt [:rhunt]]

Attachment: Bug 1669784 - Disallow setting fields/elements of TypedObjects from JS. r?jandem

For now it's sufficient to only support mutating these objects from
Wasm. This will make it so we only need to support type checks on entry
to Wasm functions for now.

Depends on D92851

Comment 6

[Ryan Hunt [:rhunt]]

Attachment: Bug 1669784 - Don't share prototypes between all StructType/ArrayTypes. r?jandem

Depends on D92852

Comment 7

[Ryan Hunt [:rhunt]]

Attachment: Bug 1669784 - Don't allow constructing a StructType/ArrayType from JS. r?jandem

This will allow us to only worry about creating these types from WebAssembly.

Depends on D92853

Comment 8

[Ryan Hunt [:rhunt]]

Attachment: Bug 1669784 - Replace self-hosted TypedObject read field/element code with native implementation. r?jandem

The code for reading fields/elements is the only remaining self-hosted code. Replace it with
a much smaller native implementation.

Depends on D92854

Comment 9

[Ryan Hunt [:rhunt]]

Attachment: Bug 1669784 - Move TypedObject implementation from builtin/ to wasm/. r?lth

Follow-up work is going to integrate this code more extensively with WebAssembly's
type system and runtime, so I think it makes sense for the implementation to
live in wasm/.

Depends on D92855

Comment 10

[Ryan Hunt [:rhunt]]

Attachment: Bug 1669784 - Merge TypedObject namespace into WebAssembly namespace, gated on GC feature. r?lth

This commit merges the TypedObject namespace into the WebAssembly namespace, and
only exposes the TypedObject definitions when the GC feature is enabled.

A future commit will remove the TypedObject definitions from the namespace, but we
still need the infrastructure from this commit for storing the TypedObject definitions
in private slots in the namespace.

Depends on D92856

Comment 11

[Ryan Hunt [:rhunt]]

Attachment: Bug 1669784 - Don't expose TypedObject definitions on WebAssembly namespace. r?lth,jandem

There's no point in exposing the TypedObject definitions in the WebAssembly namespace
until we allow the types to be constructed by JS again.

This commit also removes support for TypedObject types that aren't needed by
WebAssembly, such as 'string', 'uint32', 'uint64'.

Depends on D92857

Comment 12

[Ryan Hunt [:rhunt]]

Attachment: Bug 1669784 - Fix wasm/gc tests for changes to TypedObject API. r?lth

Fix tests for:

• disallowing construction of TypedObject types
• disallowing JS to set fields of struct types

Depends on D92858

Comment 13

[Ryan Hunt [:rhunt]]

Attachment: Bug 1669784 - Remove TypedObject tests, split part 3. r?jandem

This commit removes all tests that used the old TypedObject API.

We may be able to salvage some of these tests in the future when
the Wasm GC API is clearer, but for now remove them.

Depends on D92859

Comment 14

[Ryan Hunt [:rhunt]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=d4cf272b2ea6e790292c68dba84cb98eca9a4aab

Comment 15

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/89e5f4b08331
Remove opaque/transparent split from TypedObjects. r=jandem
https://hg.mozilla.org/integration/autoland/rev/792089710c63
Remove helper methods from TypedObjects. r=jandem
https://hg.mozilla.org/integration/autoland/rev/62ad099fe894
Disallow nested TypedObjects without an indirection. r=jandem
https://hg.mozilla.org/integration/autoland/rev/18cdaa6614cf
Disallow setting fields/elements of TypedObjects from JS. r=jandem
https://hg.mozilla.org/integration/autoland/rev/248d39d51318
Don't share prototypes between all StructType/ArrayTypes. r=jandem
https://hg.mozilla.org/integration/autoland/rev/331a512fee82
Don't allow constructing a StructType/ArrayType from JS. r=jandem
https://hg.mozilla.org/integration/autoland/rev/506ef415351b
Replace self-hosted TypedObject read field/element code with native implementation. r=jandem
https://hg.mozilla.org/integration/autoland/rev/aa2da8a6cc39
Move TypedObject implementation from builtin/ to wasm/. r=lth
https://hg.mozilla.org/integration/autoland/rev/481b2e98e24b
Merge TypedObject namespace into WebAssembly namespace, gated on GC feature. r=lth
https://hg.mozilla.org/integration/autoland/rev/01f901ce054b
Don't expose TypedObject definitions on WebAssembly namespace. r=jandem
https://hg.mozilla.org/integration/autoland/rev/59d7cd2a604c
Fix wasm/gc tests for changes to TypedObject API. r=lth

Comment 16

[Ryan Hunt [:rhunt]]

I was not able to push the last commit, removing all typed object tests, as lando was giving me an error. My guess is that it was too big.

I'm going to try to land it separately. This is okay, because all TypedObject tests do feature detection and so they should not run when the TypedObject namespace is no longer available.

Comment 17

[Razvan Maries]

Backed out for perma failures.

Push with failures: https://treeherder.mozilla.org/#/jobs?repo=autoland&selectedTaskRun=NkXB91ENSU-KG8LaFVi0xw.0&resultStatus=testfailed%2Cbusted%2Cexception&revision=59d7cd2a604c7fb3f21f1c90169d957a869f4ffb&failure_classification_id=2

Logs:
https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=318190423&repo=autoland&lineNumber=3931
https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=318189734&repo=autoland&lineNumber=5513

Backout: https://hg.mozilla.org/integration/autoland/rev/8ef9eb372189ebba7f583baafa5022e1d1853f23

Comment 18

[Ryan Hunt [:rhunt]]

Ah, some browser tests are asserting that TypedObjects are defined in nightly.

Comment 19

[Ryan Hunt [:rhunt]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=3bab2f6a978d2360564eddea52a7f3f4e9f72af1

Comment 20

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/69cd2f4baa75
Remove opaque/transparent split from TypedObjects. r=jandem
https://hg.mozilla.org/integration/autoland/rev/d075286992c2
Remove helper methods from TypedObjects. r=jandem
https://hg.mozilla.org/integration/autoland/rev/476165c19fa2
Disallow nested TypedObjects without an indirection. r=jandem
https://hg.mozilla.org/integration/autoland/rev/15e9d163baba
Disallow setting fields/elements of TypedObjects from JS. r=jandem
https://hg.mozilla.org/integration/autoland/rev/34d573a984fc
Don't share prototypes between all StructType/ArrayTypes. r=jandem
https://hg.mozilla.org/integration/autoland/rev/921a5bd4b0c5
Don't allow constructing a StructType/ArrayType from JS. r=jandem
https://hg.mozilla.org/integration/autoland/rev/ac1e917a452b
Replace self-hosted TypedObject read field/element code with native implementation. r=jandem
https://hg.mozilla.org/integration/autoland/rev/970f38ba9b25
Move TypedObject implementation from builtin/ to wasm/. r=lth
https://hg.mozilla.org/integration/autoland/rev/42ef5b507f58
Merge TypedObject namespace into WebAssembly namespace, gated on GC feature. r=lth
https://hg.mozilla.org/integration/autoland/rev/6f7124a9d2f2
Don't expose TypedObject definitions on WebAssembly namespace. r=jandem
https://hg.mozilla.org/integration/autoland/rev/601765c9159a
Fix wasm/gc tests for changes to TypedObject API. r=lth

Comment 21

[Natalia Csoregi [:nataliaCs]]

https://hg.mozilla.org/mozilla-central/rev/69cd2f4baa75
https://hg.mozilla.org/mozilla-central/rev/d075286992c2
https://hg.mozilla.org/mozilla-central/rev/476165c19fa2
https://hg.mozilla.org/mozilla-central/rev/15e9d163baba
https://hg.mozilla.org/mozilla-central/rev/34d573a984fc
https://hg.mozilla.org/mozilla-central/rev/921a5bd4b0c5
https://hg.mozilla.org/mozilla-central/rev/ac1e917a452b
https://hg.mozilla.org/mozilla-central/rev/970f38ba9b25
https://hg.mozilla.org/mozilla-central/rev/42ef5b507f58
https://hg.mozilla.org/mozilla-central/rev/6f7124a9d2f2
https://hg.mozilla.org/mozilla-central/rev/601765c9159a

Comment 22

[Ryan Hunt [:rhunt]]

Attachment: Bug 1669784 - Remove TypedObject tests, split part 1.

Comment 23

[Ryan Hunt [:rhunt]]

Attachment: Bug 1669784 - Remove TypedObject tests, split part 2.

Depends on D93520

Comment 24

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/687ca86e0d13
Remove TypedObject tests, split part 1.
https://hg.mozilla.org/integration/autoland/rev/ac3e2a0fb832
Remove TypedObject tests, split part 2.
https://hg.mozilla.org/integration/autoland/rev/3a9738afa9d0
Remove TypedObject tests, split part 3. r=jandem

Comment 25

[Andreea Pavel [:apavel]]

https://hg.mozilla.org/mozilla-central/rev/687ca86e0d13
https://hg.mozilla.org/mozilla-central/rev/ac3e2a0fb832
https://hg.mozilla.org/mozilla-central/rev/3a9738afa9d0

Comment 26

[Florin Strugariu [:Bebe]]

== Change summary for alert #27244 (as of Fri, 16 Oct 2020 07:28:25 GMT) ==

Improvements:

Ratio	Suite	Test	Platform	Options	Absolute values (old vs new)
0.36%	Base Content JS		windows7-32-shippable		2,277,687.00 -> 2,269,477.33
0.32%	Base Content JS		windows7-32-shippable		2,276,536.00 -> 2,269,138.67
0.31%	Base Content JS		linux1804-64-shippable		2,895,450.29 -> 2,886,586.67
0.30%	Base Content JS		windows10-64-shippable-qr		2,900,150.29 -> 2,891,510.00
0.29%	Base Content JS		linux1804-64-shippable-qr		2,895,199.43 -> 2,886,745.33
0.28%	Base Content JS		windows10-64-shippable		2,890,893.71 -> 2,882,934.67

For up to date results, see: https://treeherder.mozilla.org/perf.html#/alerts?id=27244