Skip to content
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

refactor VatManager, prepare for VatWorker #1339

Merged
merged 4 commits into from
Jul 30, 2020
Merged

Conversation

warner
Copy link
Member

@warner warner commented Jul 28, 2020

This starts the process of #1321, refactoring VatManager and the code by which the kernel interacts with vats (deliveries and syscalls).

Redefine VatManager according to the new architecture in docs/vat-worker.md .
The translation duties in the old vatManager.js are split out into
"translators", which are built separately for each vat, but which don't care
how the VatWorker is implemented.

Devices are now defined as files with a named buildRootDeviceNode function,
just like how vats export a buildRootObject function. However device files
no longer support setup at all.

This should pave the way for new kinds of VatWorkers, such as XS in a child
process.

I removed some leftover code in kernel.js that guarded against cross-Realm
function calls (stringifying arguments, copy arrays into new arrays) because
new-SES means we have only one shared Realm everywhere.

The code in kernel.js that builds the metering tools was factored out to a
separate file.

vatManager.js was broken up into a number of separate files, with
src/kernel/vatManager/vatManager.js being the entry point. The
inbound/outbound translation code was moved out to vatTranslator.js,
because it does not depend upon the particular kind of VatWorker we're using.

The portions of kernel.js which handled syscalls on behalf of vats was
moved out to kernelSyscall.js, and made to work in terms of KernelSyscall
objects (which come out of the translator, which is fed with a VatSyscall
object, which comes from the VatManager). Statistics incrementing is the
responsibility of kernelSyscall.js.

The panic() function was made to take the Error object it will be throwing,
to preserve the stack trace when an exception kills the kernel (as opposed to
an assertion failure).

@warner warner added the SwingSet package: SwingSet label Jul 28, 2020
@warner
Copy link
Member Author

warner commented Jul 28, 2020

I found that so many functions are being called with lots of arguments, so for some of them I replaced the positional list with a single arg named tools, and then do a destructuring assignment to get the individual names. (I'm a python programmer, I like named arguments). If that feels weird or non-idiomatic, let me know, I'm happy to change it back.

Also, the diff will appear slightly smaller if you tell github to hide whitespace changes: many of the test vats/devices were changed to use buildRootObject/buildRootDeviceNode instead of setup(), and that makes it lose a level of indentation.

@warner warner marked this pull request as ready for review July 28, 2020 05:53
@warner warner requested a review from FUDCo July 28, 2020 05:53
@warner warner changed the title refactor VatMananger, prepare for VatWorker refactor VatManager, prepare for VatWorker Jul 29, 2020
Redefine VatManager according to the new architecture in docs/vat-worker.md .
The translation duties in the old vatManager.js are split out into
"translators", which are built separately for each vat, but which don't care
how the VatWorker is implemented.

Devices are now defined as files with a named `buildRootDeviceNode` function,
just like how vats export a `buildRootObject` function. However device files
no longer support `setup` at all.

This should pave the way for new kinds of VatWorkers, such as XS in a child
process.

I removed some leftover code in `kernel.js` that guarded against cross-Realm
function calls (stringifying arguments, copy arrays into new arrays) because
new-SES means we have only one shared Realm everywhere.

The code in kernel.js that builds the metering tools was factored out to a
separate file.

`vatManager.js` was broken up into a number of separate files, with
`src/kernel/vatManager/vatManager.js` being the entry point. The
inbound/outbound translation code was moved out to `vatTranslator.js`,
because it does not depend upon the particular kind of VatWorker we're using.

The portions of `kernel.js` which handled syscalls on behalf of vats was
moved out to `kernelSyscall.js`, and made to work in terms of `KernelSyscall`
objects (which come out of the translator, which is fed with a `VatSyscall`
object, which comes from the VatManager). Statistics incrementing is the
responsibility of `kernelSyscall.js`.

The `panic()` function was made to take the Error object it will be throwing,
to preserve the stack trace when an exception kills the kernel (as opposed to
an assertion failure).

refs #1321
Also remove unneeded exports.
Copy link
Contributor

@FUDCo FUDCo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looking good. Let's pull the trigger!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
SwingSet package: SwingSet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants