Continued implementation of resolved promise pruning #930

FUDCo · 2020-04-15T06:40:12Z

These changes implement the c-list cleanup and liveslots table cleanup portions of getting rid of resolved promises.

warner

Please move the kernel-side retirement one step earlier as noted. I'll look at the tests and see if there's an easy way to add an assertion for this (it'll mean getting control in the middle of a dispatch and checking the kernel tables at that point).

packages/SwingSet/src/kernel/state/vatKeeper.js

packages/SwingSet/src/kernel/vatManager.js

packages/SwingSet/test/test-vpid-liveslots.js

We need an additional assertion: the upcoming promise-retirement change should remove the kernel c-list entries *before* the vat is notified about incoming resolutions, not afterwards. This adds a test which looks at the c-lists during that notification, so we have a place to update when the change lands. refs #930

warner · 2020-04-15T21:16:56Z

packages/SwingSet/test/test-vpid-liveslots.js

@@ -300,52 +303,24 @@ async function doVatResolveCase23(t, which, mode) {
  // now it sends three() with the promise. For now, we expect the same vpid


I think this comment is stale: once promises are retired, we expect a new vpid in the post-resolution messages

We need an additional assertion: the upcoming promise-retirement change should remove the kernel c-list entries *before* the vat is notified about incoming resolutions, not afterwards. This adds a test which looks at the c-lists during that notification, so we have a place to update when the change lands. refs #930

rewrite liveslots use of HandledPromise, remove deliver() stall refs #894 refs #930 (should unblock, although rewrites will be needed) refs #767 refs #766 (should unblock) closes #1053 (includes rebased form of the PR) closes #769 (incidentally)

warner · 2020-05-07T18:56:41Z

with #1068 landed, this should be unblocked.. you'll need to rewrite things, but the core HandledPromise stale-vpid problem should be fixed

warner

everything looks great, but I'd feel better if we have those extra tests on the kernel side (test-vpid-kernel for the promise-data/promise-reject cases) as we land this

packages/SwingSet/src/kernel/liveSlots.js

packages/SwingSet/src/kernel/state/vatKeeper.js

packages/SwingSet/test/test-vpid-kernel.js

packages/SwingSet/test/test-vpid-liveslots.js

warner · 2020-05-13T17:42:37Z

packages/swingset-runner/demo/resolveCircular/vat-bob.js

+      [p2, r2] = makePR();
+      return p2;
+    },
+    usePromises(pa, pb) {


these demos are great.. do we have a test of cyclic resolution in swingset proper?

I think we still need a unit test for this in swingset

FUDCo · 2020-05-16T07:41:09Z

@warner Most of the touchups from your review were straightforward, but the revisions and additions to test-vpid-kernel.js could probably benefit from a closer look.

packages/SwingSet/test/test-vpid-kernel.js

warner · 2020-05-18T20:04:44Z

packages/swingset-runner/demo/resolveCircular/vat-bob.js

+      [p2, r2] = makePR();
+      return p2;
+    },
+    usePromises(pa, pb) {


I think we still need a unit test for this in swingset

packages/SwingSet/test/test-vpid-liveslots.js

warner

New tests are great. I still want a resolve-to-cycle test in swingset proper. Minor changes otherwise:

test-vpid-kernel should check both sides of the expectedRetirement case
call tests in a loop

warner

get rid of that one test.only and we're good to go!

…lutions

… promises

FUDCo added enhancement New feature or request SwingSet package: SwingSet performance Performance related issues labels Apr 15, 2020

FUDCo requested a review from warner April 15, 2020 06:40

warner self-assigned this Apr 15, 2020

warner requested changes Apr 15, 2020

View reviewed changes

packages/SwingSet/src/kernel/state/vatKeeper.js Show resolved Hide resolved

packages/SwingSet/src/kernel/vatManager.js Outdated Show resolved Hide resolved

packages/SwingSet/test/test-vpid-liveslots.js Outdated Show resolved Hide resolved

warner mentioned this pull request Apr 15, 2020

chore: improve swingset vpid-retirement test #940

Merged

warner reviewed Apr 15, 2020

View reviewed changes

warner mentioned this pull request May 7, 2020

rewrite liveslots use of HandledPromise, remove deliver() stall #1068

Merged

warner assigned FUDCo and unassigned warner May 7, 2020

FUDCo force-pushed the 675-prune-resolved-promises branch from 3b73783 to 3a5a6db Compare May 12, 2020 03:05

FUDCo requested a review from warner May 12, 2020 03:07

warner requested changes May 13, 2020

View reviewed changes

FUDCo force-pushed the 675-prune-resolved-promises branch from 2ed0876 to cad4119 Compare May 16, 2020 07:39

FUDCo requested a review from warner May 16, 2020 07:39

warner reviewed May 18, 2020

View reviewed changes

warner requested changes May 18, 2020

View reviewed changes

warner approved these changes May 18, 2020

View reviewed changes

FUDCo force-pushed the 675-prune-resolved-promises branch from e1e6c9a to c8d870b Compare May 18, 2020 22:29

FUDCo added 2 commits May 18, 2020 15:29

fix: Recipient-side resolved promise retirement

dc0aec9

fix: Resolver-side resolved promise retirement

7cb2984

FUDCo added 11 commits May 18, 2020 15:29

test: update swingset tests in light of promise pruning

5507e33

fix: delete c-list entries *before* notifying vats about promise reso…

7fb8a1f

…lutions

fix: Recipient-side resolved promise retirement

65010cf

fix: Resolver-side resolved promise retirement

401e86a

chore: more tests

a376876

fix: don't retire promises that resolve to data structures containing…

00098da

… promises

test: update vpid tests to account for promise retirement

f6fb087

fix: eslint is tricksy, it is

1245ebb

chore: tidy up various bits of grunge

7b36d63

test: cleanup and augmentation with more test cases

8ab23b3

test: further test tweaks and additions

e98d720

FUDCo force-pushed the 675-prune-resolved-promises branch from c8d870b to e98d720 Compare May 18, 2020 22:30

FUDCo merged commit d4f5f0f into master May 18, 2020

FUDCo deleted the 675-prune-resolved-promises branch May 18, 2020 23:10

FUDCo mentioned this pull request May 21, 2020

Garbage collect resolved promises #1124

Closed

3 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Continued implementation of resolved promise pruning #930

Continued implementation of resolved promise pruning #930

FUDCo commented Apr 15, 2020

warner left a comment

warner Apr 15, 2020

warner commented May 7, 2020

warner left a comment

warner May 13, 2020

warner May 18, 2020

FUDCo commented May 16, 2020

warner May 18, 2020

warner left a comment

warner left a comment

		@@ -300,52 +303,24 @@ async function doVatResolveCase23(t, which, mode) {
		// now it sends three() with the promise. For now, we expect the same vpid

Continued implementation of resolved promise pruning #930

Continued implementation of resolved promise pruning #930

Conversation

FUDCo commented Apr 15, 2020

warner left a comment

Choose a reason for hiding this comment

warner Apr 15, 2020

Choose a reason for hiding this comment

warner commented May 7, 2020

warner left a comment

Choose a reason for hiding this comment

warner May 13, 2020

Choose a reason for hiding this comment

warner May 18, 2020

Choose a reason for hiding this comment

FUDCo commented May 16, 2020

warner May 18, 2020

Choose a reason for hiding this comment

warner left a comment

Choose a reason for hiding this comment

warner left a comment

Choose a reason for hiding this comment