-
Notifications
You must be signed in to change notification settings - Fork 212
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
docs(swingset): Improve "delivery" documentation #4717
Conversation
@@ -146,7 +149,7 @@ enum CapSlot { | |||
Object(ObjectID), | |||
} | |||
struct CapData { | |||
body: Vec<u8>, | |||
body: String, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Vec<u8>
appeared to be a typo, especially with other page content referring to body as a string and providing examples of such.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hm, I believe my original attempt at a Rust implementation used Vec<u8>
(the Rust name for bytes, as opposed to unicode), because I think that's what comes out of the serde
JSON+UTF8 serializer and it's what can be sent over a wire. But String
(i.e. the Rust name for unicode strings) is more meaningful to a JS audience, and is equally plausible, and is closer to what we actually do now in JS, depending upon where you look (there's one point where syscall.send
includes a string .body
, then there's a later point where the whole syscall gets serialized into bytes to send over the wire). So String
sounds fine.
(#2589 is a long-neglected aspiration to move away from JSON and use something denser, maybe CBOR, which would also accomodate bytes
arguments better, and if/when we figure that out, .body
will probably need to be bytes
instead of String
)
packages/SwingSet/docs/delivery.md
Outdated
but occasionally a batch of mutually-referencing Promises must be resolved in | ||
a single syscall because their identifiers are aggressively retired | ||
immediately after translation). | ||
immediately after translation). \[TODO: resolve inconsistency between this claim | ||
and [Syscall/Dispatch API](#syscalldispatch-api)] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The inconsistency is documentation of trait Syscall
including fn resolve(subject: PromiseID, to: Resolution)
, which seems unable to support resolving multiple promises in a batch.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
oh yeah, when we switched to batch resolution, I didn't attempt to update the pseudo-rust writeup
The new signature is more like:
struct OneResolution {
subject: PromiseID,
rejected: bool,
resolution: CapData,
}
fn resolve(resolutions: Vec<OneResolution>) {}
and dispatch.notify
takes a batch of resolutions too
packages/SwingSet/docs/delivery.md
Outdated
generally subscribe themselves, since they are the ones causing the Promise | ||
to become resolved, so they have no need to hear about it from the kernel. | ||
about the resolution of that Promise [TODO: document ordering | ||
relevance/constraints]. The Decider vat for a Promise will not generally |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a question from me about whether the deliver order of notifications to multiple subscribing vats is deterministic, and if not whether that is guaranteed to be safe.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We don't make any particular guarantees about that. Currently we probably accidentally deliver notifies in the order of subscription, but 1: vats would be hard pressed to tell, because we don't make any guarantees about the order of vat execution, also they only talk through queues, and those are the same queues that the subscribe/notify messages travel, so unless they both have access to a device that reveals ordering differences they have no reliable way to perform a test. And 2: the ordering is about to change a bunch with #3465 .
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is that a concern w.r.t. to nondeterminism and subsequent loss of consensus, or is every instance guaranteed to deliver in the same (unpredictable) order?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
All consensus instances get the same order: they're all using the same kernel, and it's that kernel that decides how to service the queues. The term we use / property we provide is "arrival order non-determinism": until the thing arrives, you don't know the ordering, but once happens, the ordering is fixed and nobody will see any other order.
At a different level, vats shouldn't be able to sense much about other vats, and if the kernel has the flexibility to do things in various orders, its choices might be influenced by its knowledge of all vats, and thus the kernel might reveal a side-channel that we'd rather keep closed. Some of this is inevitable, since vats talk to each other. And it's probably the case that if vats A and B only talk to each other, and vats C and D only talk to each other, then neither pair's event ordering will be influenced by the other pair. So I'm not worried about our current ordering.
At an even different level, within a single vat, when it sends a bunch of messages out and is holding promises for their resolution, we're also trying to avoid making claims about the order in which they resolve on that particular vat. But those resolutions arrive as dispatch.notify
events, which go into the vat's transcript, and all consensus instances run their vats with the same transcripts. So at the point of entry to the vat (dispatch.*
), we have arrival-order non-determinism: the order is deterministic across all consensus instances, but the vat has no idea what that order is (and shouldn't depend upon any particular one), but the vat will never observe two different orders (in fact, orthogonal persistence means the vat will never even know that it was restarted and the transcript re-executed, as it has no access to anything which could prove otherwise).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good changes, minor update suggested. Land after considering them (of course squash before landing).
packages/SwingSet/docs/delivery.md
Outdated
code, which effectively perceives its memory data as eternal) and is written in | ||
the SES subset of Javascript, employing native platform Promises and making | ||
eventual-send calls to local or remote objects with either the `E()` wrapper | ||
(`resultPromise=E(x).foo(args)`) or (eventually) the wavy dot syntax |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
let's drop mentions of wavy-dot, it's not coming back any time soon
packages/SwingSet/docs/delivery.md
Outdated
Each vat is represented in the kernel as a `dispatch` object with methods that | ||
close over its internal state (cf. | ||
[Vat-Inbound Slot Translation](#vat-inbound-slot-translation)). A vat's code | ||
executes when the kernel initiates a **turn** by invoking one of these methods, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We're using "crank" for this, instead of "turn". A turn is one step of the engine's microtask queue, e.g. a single Promise's .then
callback. A crank is 1 or more turns initiated by a dispatch
invocation and finishing when the microtask queue is empty. Since vat code doesn't get access to timers or other forms of IO, they can only hold onto control until the end of the crank, at which point they cannot regain agency until the kernel sends them a new dispatch.
In fact, we're a bit casual about the difference between what vat code experiences and what the vat as a whole (including liveslots, and the worker process supervisor) experiences, and indeed what the kernel sees. Also our terminology has shifted over time.
If we're talking about the kernel, these days we use "crank" to talk about the processing of an event from the run-queue, which may or may not perform a delivery to a vat (e.g. the crank might merely queue a message on some unresolved kernel promise). If it does, we send a delivery message to the worker, which begins the "crank" that the vat knows about. Liveslots gets control first, deserializes the arguments, then invoke vat code, which can run for multiple turns until it loses agency. Then liveslots regains control, does some cleanup, then sends the results back to the waiting kernel.
For the purposes of this paragraph, I guess we should say the kernel initiates a "delivery". If "crank" comes up elsewhere, we could differentiate between a "kernel crank" (that may or may perform a delivery) and a "vat crank" which is the multi-turn execution triggered by a delivery).
@@ -146,7 +149,7 @@ enum CapSlot { | |||
Object(ObjectID), | |||
} | |||
struct CapData { | |||
body: Vec<u8>, | |||
body: String, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hm, I believe my original attempt at a Rust implementation used Vec<u8>
(the Rust name for bytes, as opposed to unicode), because I think that's what comes out of the serde
JSON+UTF8 serializer and it's what can be sent over a wire. But String
(i.e. the Rust name for unicode strings) is more meaningful to a JS audience, and is equally plausible, and is closer to what we actually do now in JS, depending upon where you look (there's one point where syscall.send
includes a string .body
, then there's a later point where the whole syscall gets serialized into bytes to send over the wire). So String
sounds fine.
(#2589 is a long-neglected aspiration to move away from JSON and use something denser, maybe CBOR, which would also accomodate bytes
arguments better, and if/when we figure that out, .body
will probably need to be bytes
instead of String
)
packages/SwingSet/docs/delivery.md
Outdated
* `Fulfilled`: includes an ObjectID (an Export) to which it was resolved | ||
* `Data`: includes resolution data (body+slots) | ||
* `Rejected`: includes the rejection data (body+slots, maybe an Error object) | ||
* `Fulfilled`: includes the ObjectID with which it was fulfilled |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We no longer distinguish between fulfilled
(fulfilled to a callable object) and data
(fulfilled to anything else). The current states (look for resolveKernelPromise
in src/kernel/state/kernelKeeper.js
) are unresolved
or fulfilled
or rejected
, and the last two mean there's a .data
property on the promise record that contains capdata, plus a .rejected
boolean. We wait until we're trying to send a message to the resolved promise before we look at .data
to distinguish between something that accepts messages and a data record that cannot.
packages/SwingSet/docs/delivery.md
Outdated
@@ -375,19 +383,18 @@ prefer to avoid deserializing the messages until their resolution is known, | |||
to avoid a wasteful reserialization cycle. | |||
|
|||
If the deciding Vat has *not* opted into pipelining, the messages are queued | |||
in the kernel's Promise Table entry instead. They remain there until the | |||
in the kernel's Promise table entry instead. They remain there until the |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I go back and forth about how to reinforce the idea that a "kernel promise" is not a javascript Promise
object (but does correspond to Promise
s in vats). Sometimes I use "kernel promise" vs "promise", sometimes I use "kernel promise" vs "Promise
" (i.e. use the font and capitalization to evoke the JS syntax for the non-kernel thing). I've used "Promise Table" (both caps) to sort of glue the two words together and avoid making it sound like the kernel has a table of Promise
s (it has a table of kernel promises, but no actual Promise
objects).
Eh, words, they're hard. No recommendations here.
packages/SwingSet/docs/delivery.md
Outdated
generally subscribe themselves, since they are the ones causing the Promise | ||
to become resolved, so they have no need to hear about it from the kernel. | ||
about the resolution of that Promise [TODO: document ordering | ||
relevance/constraints]. The Decider vat for a Promise will not generally |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We don't make any particular guarantees about that. Currently we probably accidentally deliver notifies in the order of subscription, but 1: vats would be hard pressed to tell, because we don't make any guarantees about the order of vat execution, also they only talk through queues, and those are the same queues that the subscribe/notify messages travel, so unless they both have access to a device that reveals ordering differences they have no reliable way to perform a test. And 2: the ordering is about to change a bunch with #3465 .
packages/SwingSet/docs/delivery.md
Outdated
@@ -826,16 +833,17 @@ A Vat might be informed that one Promise has been resolved to another Promise | |||
## Sample Message Delivery | |||
|
|||
This section follows a variety of messages are they are sent from Vat-1 to | |||
Vat-2. | |||
Vat-2. Syntax like `foo!bar(baz)` is used to indicate a message to Object or |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah, bang notation, even older than tildot/wavydot. This might be worth replacing with E()
, except I really like e.g. x~.foo()~.bar()
over E(E(x).foo()).bar()
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I figure this note allows us to keep it here as a not-actually-executable shorthand.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, good point, and I do that all the time in unit test comments. Let's update it to tildot then, so at least we only have one archaic dialect instead of two.
@@ -966,19 +974,19 @@ receiving pipelined messages. | |||
|
|||
The two `send` calls will look like: | |||
|
|||
* `syscall.send(target=o-1001, msg={method: "foo", .. result=p+104})` | |||
* `syscall.send(target=p+104, msg={method: "bar", .. result=p+105})` | |||
* `syscall.send(target=o-1001, msg={method: "foo", …, result=p+104})` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You're determined to drag me kicking and screaming into the modern unicode world, aren't you? :)
I don't have a button on my keyboard for that symbol. I'd kinda prefer to keep this document plain ASCII, but if you think there's an ambiguity caused by U+002E,U+002E (I know ..
looks like ...
which looks like a real JS operator, and ..
is an operator in other languages), then stick with U+2026 and I'll cut-and-paste this symbol if I ever need to edit this doc.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That was indeed the concern. Hopefully changes won't be needed too often. :P (or more properly 😜, although that one I had to copy from Emojipedia).
P.S. I don't know this part of Mac yet, but I think every Linux input method has a default <Multi_key> <period> <period>
.XCompose sequence for "…" (which is how I type it, where my Multi_key is configured to be the right-side Ctrl).
a kernel-facing C-List (however kernel "messages", really syscalls, are | ||
delivered immediately, whereas messages to remote machines are asynchronous). | ||
The Comms Vat does not need to manage a run-queue (`dispatch()` causes an | ||
immediate external message), nor does each unresolved promise have a queue of | ||
messages (these are pipelined immediately). | ||
|
||
The one wrinkle is that Vat-Vat connections are symmetric, which impacts the | ||
The one wrinkle is that Vat→Vat connections are symmetric, which impacts the |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ditto
|
||
The message names are also different. In the local-machine Vat-Kernel-Vat | ||
The message names are also different. In the local-machine Vat→Kernel→Vat |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ok here's a case where the unicode arrows are a tiny bit nicer than Vat->Kernel->Vat
, I'l begrudgingly admit :)
@warner The followup changes were sufficiently substantial that I'd like another round of review before merging. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good, a few small recommendations remaining.
packages/SwingSet/docs/delivery.md
Outdated
ordering properties better. | ||
|
||
In some places, `dispatch.deliver()` is named `message`: we're still in the | ||
process of refactoring and unifying the codebase. | ||
|
||
## Outbound (Vat->Kernel) translation | ||
## Vat-Outbound Slot Translation |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd recommend against this change. Each new person I've explained this to has gotten confused about the directions of "outbound" vs "inbound" for the first few days.. we have a coherent metaphor ("vats are egocentric"), but it isn't obvious, and I'd like to reinforce the scheme whereever we can, especially in a document like this one. So for section titles, having both the word "outbound" and the "vat->kernel" direction expressed at the same time feels important to me.
Feel free to rephrase it, and "Vat-Outbound" does make more sense than just "Outbound", but I think it's important to indicate the direction in the same place as introducing "outbound". Maybe "Vat Outbound Slot Translation (from Vat into Kernel)"? "Vat Outbound Slot Translation (Vat->Kernel)"?
Also maybe I'm being paranoid but I've seen enough <unrenderable glyph>
boxes in documents I read, that I don't trust that all readers will render a unicode arrow, and if that happens then we're losing critical information. If you do want to use a unicode arrow in the title, maybe add a few words to be beginning of the section, "As messages are sent "outbound" from the vat into the kernel, ..." for the hapless folks reading the .md in a terminal and cursing about newfangled character sets when the old one worked just fine.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I agree with keeping special characters out of section headings, even including ASCII punctuation (which is actually what motivated this change—I want predictable and stable auto-generated anchor ids). But I just pushed a fix for that by adding anchor elements directly.
packages/SwingSet/docs/delivery.md
Outdated
|
||
## Inbound (Kernel->Vat) translation | ||
## Vat-Inbound Slot Translation |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ditto
packages/SwingSet/docs/delivery.md
Outdated
translated just like `Message.args`. | ||
|
||
TODO: After a Vat receives notice of a Promise being resolved, we might | ||
choose to remove that promise from the vat's c-list, and forbid the Vat from | ||
choose to remove that promise from the vat's C-List, and forbid the Vat from |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's change this paragraph: we do indeed remove the promise from the c-list ("c-list promise entries are retired upon resolution")
We also use the word "retired" to describe removing c-list object entries when one side no longer needs to talk about the object, which is somewhat after the "drop" that one side does when it can no longer reach that object, because of WeakMaps. So let's be sure to use "retire" for promises here, and not "drop".
We don't exactly forbid the vat from mentioning the vpid
again (we don't keep a history of retired IDs), but if it did, the kernel would treat it as a brand new promise. Liveslots won't, because it uses a persistent incrementing counter to generate IDs, so it will never re-generate the old one.. oh, huh, ok that's a problem for vat-upgrade, I'll make a ticket #4730 about that.
Description
General improvements to documentation (fixing typos, improving internal consistency, clarifying concepts, etc.). It might make sense to review commit by commit and/or in rich-text mode.
Security Considerations
n/a
Documentation Considerations
See above.
Testing Considerations
n/a