Closed
Bug 1130988
Opened 10 years ago
Closed 8 years ago
Xrays support for SharedArrayBuffer
Categories
(Core :: XPConnect, defect)
Tracking
()
RESOLVED
FIXED
mozilla52
Tracking | Status | |
---|---|---|
firefox52 | --- | fixed |
People
(Reporter: lth, Assigned: evilpie)
References
(Blocks 2 open bugs)
Details
Attachments
(3 files)
2.37 KB,
patch
|
bzbarsky
:
review+
|
Details | Diff | Splinter Review |
5.68 KB,
patch
|
bzbarsky
:
review+
|
Details | Diff | Splinter Review |
5.02 KB,
patch
|
bzbarsky
:
review+
|
Details | Diff | Splinter Review |
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•10 years ago
|
||
(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.
Reporter | ||
Comment 2•10 years ago
|
||
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•10 years ago
|
||
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.
Reporter | ||
Comment 4•10 years ago
|
||
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•10 years ago
|
||
(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.
Assignee | ||
Comment 6•8 years ago
|
||
SharedTypedArrays are gone, do we really need Xray support for Atomis or is SharedTypedArray good enough?
Reporter | ||
Comment 7•8 years ago
|
||
I seriously doubt this is interesting for Atomics. SharedArrayBuffer should have whatever support ArrayBuffer has, however.
Reporter | ||
Updated•8 years ago
|
Summary: Xrays support for SharedArrayBuffer, SharedTypedArray, and maybe the Atomics object → Xrays support for SharedArrayBuffer
Assignee | ||
Comment 8•8 years ago
|
||
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.
Assignee | ||
Comment 9•8 years ago
|
||
Assignee | ||
Comment 10•8 years ago
|
||
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.
Reporter | ||
Comment 12•8 years ago
|
||
(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•8 years ago
|
||
Comment on attachment 8805200 [details] [diff] [review]
Actually support creating TypedArrays with Xray SharedArrayBuffer
r=me
Attachment #8805200 -
Flags: review+
Comment 14•8 years ago
|
||
Comment on attachment 8805201 [details] [diff] [review]
Change SharedArrayBuffer to use ClassSpec
r=me
Attachment #8805201 -
Flags: review+
Comment 15•8 years ago
|
||
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.
Flags: needinfo?(bzbarsky)
Attachment #8805203 -
Flags: review+
Comment 16•8 years ago
|
||
Pushed by evilpies@gmail.com:
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•8 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/d27de07adaae
https://hg.mozilla.org/mozilla-central/rev/0b9e245f787d
https://hg.mozilla.org/mozilla-central/rev/40cd4eb145ff
Status: NEW → RESOLVED
Closed: 8 years ago
status-firefox52:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla52
You need to log in
before you can comment on or make changes to this bug.
Description
•