Integrate JavaScript SharedArrayBuffers and agents/agent clusters #2361
Conversation
domenic
added
the
needs tests
| data-x="dom-MessagePort-postMessage">postMessage()</code> on a <code>MessagePort</code> | ||
| whose messages are initially paused.) This subtlety is unfortunately obfuscated by the | ||
| current <span>StructuredCloneWithTransfer</span> algorithm; see <a | ||
| href="https://github.com/whatwg/html/issues/935">issue #935</a>.</p> |
There was a problem hiding this comment.
Is this really how we want to signal errors? Wouldn't it be much better if the message contained an indication of the error in some way? (Note that we also need to solve this for ArrayBuffer, as its allocation can fail.)
There was a problem hiding this comment.
It's also a little sad that we don't solve the underlying issue of the model being wrong first. It makes it harder to see the correct solutions.
There was a problem hiding this comment.
My understanding is that this is how it was shipped in all browsers already, based on the spec this is integrating, but maybe I'm wrong.
Last time this was discussed in #936 the idea was to not mix up error messages and data.
The advantage of this design, by the way, is that it allows you to get all the other data you cloned/transferred, instead of throwing it away (in the transferred case, forever).
There was a problem hiding this comment.
So is this how browsers deal with ArrayBuffer allocation failure?
There was a problem hiding this comment.
In that case I think they mostly just crash.
There was a problem hiding this comment.
Really? JavaScript prescribes a TypeError and I've seen that in code.
| with this specification's concept of an <span>event loop</span>.</p> | ||
|
|
||
| <p>To determine the manifestation of JavaScript's <span>agent cluster</span> concept in user | ||
| agents, we first define the set of <dfn>directly share-to-able event loops</dfn> for a given |
There was a problem hiding this comment.
share-to-able read a little awkward.
There was a problem hiding this comment.
Maybe "connected event loops" and "transitive-connected event loops"?
There was a problem hiding this comment.
I like including something about "sharing" in the name since this is largely about shared memory but that is indeed less awkward so I'm happy to switch to "connected".
There was a problem hiding this comment.
"Directly-shared event loops" and "shared event loops" seems okay too. I like it a little better as it has a shorter term for the thing that maps to JavaScript.
| <dd>The set containing <var>loop</var>, plus the worker's <span>environment settings | ||
| object</span>'s <span>responsible browsing context</span>'s <span>event loop</span>, plus the | ||
| <a href="#worker-event-loop">worker event loops</a> of each of <span>the worker's | ||
| workers</span></dd> |
There was a problem hiding this comment.
What if this worker was inside a service worker or a shared worker?
There was a problem hiding this comment.
My understanding is that all workers spawned from a given shared/service worker are share-to-able.
There was a problem hiding this comment.
Sure, but what about its responsible browsing context then? That's either null or it should not share with that.
There was a problem hiding this comment.
I see, yeah. Hopefully it is null, in which case this is a wording tweak, but I'll need to check.
|
Nice to see this integrated! |
|
Hi everyone! Sorry to be late to this discussion. I'm going to try to document what WebKit does right now. I wrote it before I heard all of these arguments, so these semantics are just what seemed like a good idea at the time. The main concept in WebKit's SharedArrayBuffer is that they behave like an ArrayBuffer for structured cloning. This means that all cases of postMessage except dedicatedWorker.postMessage() will clone the SharedArrayBuffer. For example, window.postMessage(sab) passes a copy, not the original. The copy on the other end is an instance of SharedArrayBuffer, and so the receiver could then share it with its dedicated workers. I don't feel strongly about these semantics. Note that this behavior is currently enabled in WebKit trunk and it "ships" in Safari Technology Preview. |
|
@pizlonator, that's a variant I hadn't really considered much. I have had (probably still have) the mindset that if it's shared memory you really don't want to copy it because that breaks the semantics of shared memory. True, a copy will have to be made for serialization purposes (XHR, IDB) but in those cases it seems like serialization is being requested explicitly, so that seems OK to me. What you're proposing is also a little strange (to me, anyhow) in that if you have a same-origin nested iframe and you postMessage the SAB to it then you get a copy, but if the SAB becomes shared through other program actions (one doc reaching into the other) then they have shared memory. |
|
@domenic are you also fixing the IDL situation? Should we maybe block this on that? E.g., I was pretty sure the plan was that SAB could not be used by XMLHttpRequest / IDB/ etc. ( |
I am, but it's orthogonal to this. The IDL annotations are only needed by Web GL.
No, there's no such decision. For things that just accept
Yes, that will be good. |
|
XMLHttpRequest doesn't accept either object or any though. IDB would be affected. We'd then need to update IDB it seems to either throw or make it do a copy (at a defined-point-in-time). |
|
Sure, those will be good things to work on/get automatically when we update IDL. But they're orthogonal to this. I'm eager to hear more from implementers on whether postMessaging within a process should always share or should sometimes copy. (For that matter, we still need to decide what postMessaging across a process does---the discussion in #936 is assuming some kind of error, but maybe just copying is fine.) |
|
I don't see how they are orthogonal. IDB directly invokes StructuredClone. You end up breaking IDB by landing this. I guess we could say that we don't care about dependencies, but I'm not entirely convinced that's a good idea. |
|
No, IDB does not break by landing this; it works perfectly. Please open another issue to discuss if you don't see why after tracing through the IDB algorithms; I'd like to keep this focused on the questions that have been raised so far about the current PR text. |
|
@binji pointed me to Chrome's code and, although we haven't written/run tests yet, it looks like we currently do the same thing for SharedArrayBuffers as for ImageBitmaps: if you go outside the process, we do a copy. I think what we need to figure out is where we stand on copy-vs.-error. As @lars-t-hansen said above:
I tend to agree with @lars-t-hansen's intuition here, that SABs (as opposed to, e.g. ImageBitmaps) really want sharing, not copying, so erroring if you can't share makes sense. But I don't feel strongly. I also think that sharing should happen within a "process", not just in the main-thread-to-worker case. That would be different from @pizlonator's semantics, but he seems OK with changing? So, let me try to narrow it down to two options:
The "error" version has the minor virtue that it could be extended to also cover failure in the case of normal ABs where the destination runs out of memory; right now we don't have an interoperable or specced story for that (see #936). What are peoples' preferences? I think so far myself and @lars-t-hansen have weak preferences for "error". |
|
One point you did not mention is that with |
|
I first want to reiterate the (uber-pedantic) point that "process" is short-hand for unit of same-origin browsing contexts, ie, contexts that share a run loop. Just so that there's no confusion about that. No OS process abstraction should be implied. IMO, though I'm not really in a position to have much of an opinion about the HTML spec, sharing parent->worker only and by workerObject.postMessage only without accounting for channels, broadcasts, etc, seems like a partial solution that is not forward-looking for the web. I have the sense from Filip's message that he's not even allowing worker-to-parent sharing: parents have to control everything. I agree this system still covers many use cases but it seems strangely limiting. I also think that whether we should copy or error out when trying to share across the boundary is about equal in implementation complexity. In both cases, it requires tracking the "process" identity of the sender in the structured clone data so that we can compare it to the receiver's identity. At the reception point, if we need to copy the data we need to basically rewrite the serialized data (at least in Firefox, where we're using a flat serialized representation), which is probably no easier than setting up an error event. |
|
@lars-t-hansen @pizlonator @binji I've started some work on the tests to help things along: web-platform-tests/wpt#5003. So far I've only done the success cases, as we're still not completely decided on the error cases I guess. Interesting notes from my testing with Firefox Dev Edition and Chrome Canary (will be able to check Safari Tech Preview tomorrow):
Tests can be perused at http://w3c-test.org/submissions/5003/html/infrastructure/safe-passing-of-structured-data/shared-array-buffers/, although window-domain-success.sub.html won't work with that URL since it relies on some absolute URL construction that isn't happy about the |
|
The code in Firefox is a trifle hacky. MessageChannel transmission has been disabled for the sake of sanity until we can agree on the semantics, and also because Firefox is becoming multiprocess and disabling MessageChannel transmission reduces risk while we're figuring out the implications of that. Additionally, I don't uplift obscure bugfixes from Nightly to Dev Ed so testing Nightly is probably more reliable generally. Chakra has SAB+Atomics too: https://github.com/Microsoft/ChakraCore/blob/master/lib/Runtime/Library/SharedArrayBuffer.h. /cc @akroshg |
This rewrites most of the cloneable and transferable object infrastructure to better reflect the reality that structured cloning requires separate serialization and deserialization steps, instead of a single operation that creates a new object in the target Realm. This is most evident in the case of MessagePorts, as noted in #2277. It also allows us to avoid awkward double-cloning with an intermediate "user-agent defined Realm", as seen in e.g. history.state or IndexedB; instead we can simply store the serialized form and later deserialize. Concretely, this: * Replaces the concept of cloneable objects with serializable objects. For platform objects, instead of defining a [[Clone]]() internal method, serializable platform objects are annotated with the new [Serializable] IDL attribute, and include serialization and deserialization steps in their definition. * Updates the concept of transferable objects. For platform objects, instead of defining a [[Transfer]]() internal method, transferable platform objects are annotated with the new [Transferable] IDL attribute, and include transfer and transfer-receiving steps. Additionally, the [[Detached]] internal slot for such objects is now managed more automatically. * Removes the StructuredClone() abstract operation in favor of separate StructuredSerialize() and StructuredDeserialize() abstract operations. In practice we found that performing a structured clone alone is never necessary in specs. It is always either coupled with a transfer list, for which StructuredCloneWithTransfer() can be used, or it is best expressed as separate serialization and deserialization steps. * Removes IsTransferable() and Transfer() abstract operations. When defined more properly, these became less useful by themselves, so they were inlined into the rest of the machinery. * Introduces StructuredSerialzieWithTransfer() and StructuredDeserializeWithTransfer(), which can be used by other specifications which need to define their own postMessage()-style algorithm but for which StructuredCloneWithTransfer() is not sufficient. Closes #785. Closes #935. Closes #2277. Closes #1162. Sets the stage for #936 and #2260/#2361.
|
As a heads up, this PR is effectively replaced by these PRs:
Please take into account that these PRs build upon each other to some extent, so there's some overlap in commits. That'll be reduced as they start landing on master. Also the above list is in order of stability. |
|
Closing this to avoid confusion. Work has moved to the aforementioned PRs, the first of which has landed. |
This rewrites most of the cloneable and transferable object infrastructure to better reflect the reality that structured cloning requires separate serialization and deserialization steps, instead of a single operation that creates a new object in the target Realm. This is most evident in the case of MessagePorts, as noted in whatwg#2277. It also allows us to avoid awkward double-cloning with an intermediate "user-agent defined Realm", as seen in e.g. history.state or IndexedB; instead we can simply store the serialized form and later deserialize. Concretely, this: * Replaces the concept of cloneable objects with serializable objects. For platform objects, instead of defining a [[Clone]]() internal method, serializable platform objects are annotated with the new [Serializable] IDL attribute, and include serialization and deserialization steps in their definition. * Updates the concept of transferable objects. For platform objects, instead of defining a [[Transfer]]() internal method, transferable platform objects are annotated with the new [Transferable] IDL attribute, and include transfer and transfer-receiving steps. Additionally, the [[Detached]] internal slot for such objects is now managed more automatically. * Removes the StructuredClone() abstract operation in favor of separate StructuredSerialize() and StructuredDeserialize() abstract operations. In practice we found that performing a structured clone alone is never necessary in specs. It is always either coupled with a transfer list, for which StructuredCloneWithTransfer() can be used, or it is best expressed as separate serialization and deserialization steps. * Removes IsTransferable() and Transfer() abstract operations. When defined more properly, these became less useful by themselves, so they were inlined into the rest of the machinery. * Introduces StructuredSerialzieWithTransfer() and StructuredDeserializeWithTransfer(), which can be used by other specifications which need to define their own postMessage()-style algorithm but for which StructuredCloneWithTransfer() is not sufficient. Closes whatwg#785. Closes whatwg#935. Closes whatwg#2277. Closes whatwg#1162. Sets the stage for whatwg#936 and whatwg#2260/whatwg#2361.
This integrates much of https://tc39.github.io/ecmascript_sharedmem/dom_shmem.html, with the modifications discussed in #2260 around the "process boundary" (called here a "unit of share-within-able event loops"). Regarding that discussion, it also includes nested workers, as they turned out to be fairly easy to spec.
Closes #2260. Closes tc39/proposal-ecmascript-sharedmem#144.
This builds upon a slightly-shaky foundation, as noted in #935. In particular it needs to take special care to call out allowances made for the current algorithm structure, which does not separate serialization and deserialization.
In general I am pretty happy with the normative content here. The verbiage in "Integration with the JavaScript agent formalism" could use some tweaks though. The terminology I invented, viz. "directly share-to-able event loops" and "unit of share-within-able event loops", is pretty comical, and I'm happy to discuss some replacements. (Maybe simply "event loop cluster" for the latter.) And I'm not so sure if the example in that section is helpful, or maybe should be expanded or something.
/cc @lars-t-hansen @bradnelson
Test plan moved to web-platform-tests/wpt#5003