1130988 - Xrays support for SharedArrayBuffer

Status: RESOLVED FIXED

(Core :: XPConnect, defect)

Description

[Lars T Hansen [:lth]]

I'm not actually sure what's needed here but after watching bholley's talk I'm pretty sure that SAB/STA will be opaque to Xrays clients but need not be and should not be.

Comment 1

[Bobby Holley (:bholley)]

(In reply to Lars T Hansen [:lth] from comment #0)
> I'm not actually sure what's needed here but after watching bholley's talk
> I'm pretty sure that SAB/STA will be opaque to Xrays clients

Correct.

> but need not be and should not be.

Can you elaborate a bit on the use cases here?

Note that in my implementation for TypedArray Xrays, I deliberately made indexed access throw, because accessing a TypedArray that way is going to be exceedingly slow.

Comment 2

[Lars T Hansen [:lth]]

Use cases for Xrays are unclear to me - I'm new to that problem space.  Probably the use cases are analogous to those of TypedArray, certainly initially.  SharedArrayBuffer and SharedTypedArray currently behave much like ArrayBuffer and TypedArray, except that STA is not neuterable.

(The shared memory feature is described here in the event you've not seen it:
https://docs.google.com/document/d/1NDGA_gZJ7M7w1Bh8S0AoDyEqwDdRh4uSoTPSNn77PFk/edit?usp=sharing)

Comment 3

[Bobby Holley (:bholley)]

We don't have Xrays to ArrayBuffer, though you could certainly add them - it would be relatively straightforward, though I'm not aware of any use-cases it supports.

Implementing xrays to STA would probably just involve doing the analogous thing to what we do for TypedArray. This basically means providing safe access to all the simple properties, and having indexed access throw with a message saying ("This will be really slow! Use Cu.cloneInto instead!").

If you don't have support for this stuff in the structured clone algorithm, _that_ would be something worth doing.

Comment 4

[Lars T Hansen [:lth]]

SAB and STA are supported in structured clone, and that's pretty well tested at this point.  The only things not landed are the DOM bits for blocking wait (bug 1074237), and now modest Xray support.

Comment 5

[Bobby Holley (:bholley)]

(In reply to Lars T Hansen [:lth] from comment #4)
> SAB and STA are supported in structured clone, and that's pretty well tested
> at this point.  The only things not landed are the DOM bits for blocking
> wait (bug 1074237), and now modest Xray support.

Personally I don't think that Xray support here is a blocker, though I would certainly not object to adding it. Let me know if you want to and I can point you in the right direction.

Comment 6

[Tom S [:evilpie]]

SharedTypedArrays are gone, do we really need Xray support for Atomis or is SharedTypedArray good enough?

Comment 7

[Lars T Hansen [:lth]]

I seriously doubt this is interesting for Atomics.  SharedArrayBuffer should have whatever support ArrayBuffer has, however.

Comment 8

[Tom S [:evilpie]]

Attachment: Actually support creating TypedArrays with Xray SharedArrayBuffer

Calling the native function that is supposed to create the typed array with the shared array buffer would fail the "this" test. I don't think that ever worked before.

Comment 9

[Tom S [:evilpie]]

Attachment: Change SharedArrayBuffer to use ClassSpec

Comment 10

[Tom S [:evilpie]]

Attachment: Add SharedArrayBuffer to JSXray

I almost forgot about the versioning here, but we only enable SharedArrayBuffer on Aurora/Nightly at the moment. If we never want to implement SharedArrayBuffer.prototype.slice I could remove that check.

Comment 11

[Tom S [:evilpie]]

Bz, can you review this please?

Comment 12

[Lars T Hansen [:lth]]

(In reply to Tom Schuster [:evilpie] from comment #10)
> Created attachment 8805203 [details] [diff] [review]
> Add SharedArrayBuffer to JSXray
> 
> I almost forgot about the versioning here, but we only enable
> SharedArrayBuffer on Aurora/Nightly at the moment. If we never want to
> implement SharedArrayBuffer.prototype.slice I could remove that check.

SAB.prototype.slice is definitely coming, probably in FF52.

The *tentative* plan is to enable SAB in FF51 with the feature set that is currently in m-c and is in the process of being uplifted to Aurora.

Comment 13

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 8805200 [details] [diff] [review]
Actually support creating TypedArrays with Xray SharedArrayBuffer

r=me

Comment 14

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 8805201 [details] [diff] [review]
Change SharedArrayBuffer to use ClassSpec

r=me

Comment 15

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 8805203 [details] [diff] [review]
Add SharedArrayBuffer to JSXray

>+  if (!isReleaseOrBeta) {

Else, can you have this test assert that 'SharedArrayBuffer' is not a thing, so we don't forget to update the test when we ship it on release/beta?

>+      is(t.byteLength, 12, "ArrayBuffer byteLength is correct");

I'd appreciate it if the third arg here became `${c} byteLength is correct`, and similar for the other test descriptions in this function.

r=me with those fixed.

Comment 16

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/d27de07adaae
Actually support creating TypedArrays with Xray SharedArrayBuffer. r=bz
https://hg.mozilla.org/integration/mozilla-inbound/rev/0b9e245f787d
Change SharedArrayBuffer to use ClassSpec. r=bz
https://hg.mozilla.org/integration/mozilla-inbound/rev/40cd4eb145ff
Add SharedArrayBuffer to JSXray. r=bz

Comment 17

[Phil Ringnalda (:philor)]

https://hg.mozilla.org/mozilla-central/rev/d27de07adaae
https://hg.mozilla.org/mozilla-central/rev/0b9e245f787d
https://hg.mozilla.org/mozilla-central/rev/40cd4eb145ff