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

Implement durable kind API #4824

Merged
merged 1 commit into from
Mar 18, 2022
Merged

Implement durable kind API #4824

merged 1 commit into from
Mar 18, 2022

Conversation

FUDCo
Copy link
Contributor

@FUDCo FUDCo commented Mar 11, 2022

This implements the durable kind API (though not the rest of the baggage machinery) per #4495

In particular, we provide the new VatData function makeKindHandle to produce durable (and storable) handles that may be used to associate durable kinds with their implementations. At least, that is what it is called until we change the name again.

Closes #4495

@FUDCo FUDCo added enhancement New feature or request SwingSet package: SwingSet labels Mar 11, 2022
@FUDCo FUDCo requested a review from warner March 11, 2022 03:12
@FUDCo FUDCo self-assigned this Mar 11, 2022
@FUDCo FUDCo requested a review from erights March 11, 2022 03:12
Copy link
Member

@erights erights left a comment

Choose a reason for hiding this comment

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

New functions must be arrow functions.

packages/SwingSet/docs/virtual-objects.md Outdated Show resolved Hide resolved
packages/SwingSet/docs/virtual-objects.md Outdated Show resolved Hide resolved
packages/SwingSet/docs/virtual-objects.md Outdated Show resolved Hide resolved
packages/SwingSet/docs/virtual-objects.md Show resolved Hide resolved
@FUDCo FUDCo requested a review from erights March 11, 2022 09:12
Copy link
Member

@erights erights left a comment

Choose a reason for hiding this comment

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

All new functions must be arrow functions.

Aside from that, looks good to me!

@FUDCo FUDCo requested a review from erights March 11, 2022 21:51
Copy link
Member

@erights erights left a comment

Choose a reason for hiding this comment

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

LGTM, thanks!

Copy link
Member

@warner warner left a comment

Choose a reason for hiding this comment

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

This can probably land with just the recommended harden calls.

Please think about the kindIDID-on-DB issue, it may be that we can't address it properly until #4730 has a solution, in which case we could fix both at the same time. I guess at least add a note to the let kindIDID line to remind us that in-RAM counters are going to be trouble.

Consider the "use 'defineDurableKindto define theKindHandle` kind" thing, but I wouldn't consider it mandatory.

return defineKindInternal(tag, init, actualize, finish, true);
const makeKindHandle = tag => {
if (!kindIDID) {
kindIDID = `${allocateExportID()}`;
Copy link
Member

Choose a reason for hiding this comment

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

I'm a bit bothered by the idea that this will differ from one vat to another, depending upon when that vat happens to make it's first handle.. I think that's going to make debugging harder. It overlaps with a similar concern about the IDs used for the standard virtual collections.

Is there any good way to make these values be the same for all vats?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't think there's a good way, though it's certainly quite doable. The reason I did it the way it works now is that many vats don't need these things at all and so they only get created/allocated the first time somebody asks for one. We could pre-allocate these for every vat, which would yield uniformity, but then we'd be carrying the overhead (mostly in the form of junk database entries, which I've found to be its own source of confusion when debugging) in all the places it's not used. In practice I haven't found the variance of the IDs from one vat to another to be a debugging issue, since if you are looking at things at that low a level you see the record of the ID allocations along with their use. And of course you typically debug in the context of a single vat and in that context the IDs stay the same from one debug run to another.

packages/SwingSet/src/liveslots/virtualObjectManager.js Outdated Show resolved Hide resolved
packages/SwingSet/src/liveslots/virtualObjectManager.js Outdated Show resolved Hide resolved
@FUDCo FUDCo force-pushed the 3834-multifaceted-virtual-objects branch 4 times, most recently from cd6ca3a to e090593 Compare March 16, 2022 07:26
@FUDCo FUDCo force-pushed the 4495-durable-kind branch from 3892136 to f14dee7 Compare March 16, 2022 17:55
@FUDCo FUDCo force-pushed the 3834-multifaceted-virtual-objects branch 2 times, most recently from a644780 to 0696866 Compare March 18, 2022 06:29
Base automatically changed from 3834-multifaceted-virtual-objects to master March 18, 2022 06:45
@FUDCo FUDCo force-pushed the 4495-durable-kind branch from 6c02b89 to 56bad98 Compare March 18, 2022 07:10
@FUDCo FUDCo added the automerge:rebase Automatically rebase updates, then merge label Mar 18, 2022
@mergify mergify bot merged commit 5252f78 into master Mar 18, 2022
@mergify mergify bot deleted the 4495-durable-kind branch March 18, 2022 07:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
automerge:rebase Automatically rebase updates, then merge enhancement New feature or request SwingSet package: SwingSet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

decide+implement API for durable virtual objects/collections: makeDurableKind?
3 participants