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

fix: Clear vat table entries for promises when they are resolved #737

Closed
wants to merge 1 commit into from

Conversation

FUDCo
Copy link
Contributor

@FUDCo FUDCo commented Mar 19, 2020

This takes care of the first part of #675, namely cleaning up the liveslots side of things.

@FUDCo FUDCo added SwingSet package: SwingSet performance Performance related issues labels Mar 19, 2020
@FUDCo FUDCo requested review from warner and michaelfig March 19, 2020 18:13
@warner
Copy link
Member

warner commented Mar 19, 2020

I don't believe we can land the two sides of the vat/kernel boundary separately: if the vat is going to forget about the p+4 identifier, the kernel must uphold its promise to never use that identifier again either.

So let's combine this with removing the kernel-side c-list entries, and unit tests to check that both have been removed. One of the tests should submit a second message with the same kernel promise identifier, and confirm that the vat receives a fresh identifier.

@FUDCo
Copy link
Contributor Author

FUDCo commented Mar 19, 2020

My memory from our meeting last week was that we concluded that the liveslots changes could stand alone, but on further reflection I think you're right: if the kernel mentions one of these promises to the vat, it seems like Bad Things would happen.

I was going for incrementalism here, but, well, never mind.

@FUDCo FUDCo closed this Mar 19, 2020
@FUDCo FUDCo deleted the 675-prune-resolved-promises-part1 branch March 19, 2020 21:38
@FUDCo FUDCo mentioned this pull request May 21, 2020
3 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
performance Performance related issues SwingSet package: SwingSet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants