From b9d3bf4804a7c55ec74e4b33ab8003a869e76091 Mon Sep 17 00:00:00 2001 From: Mathieu Hofman Date: Tue, 22 Feb 2022 19:52:24 +0000 Subject: [PATCH] feat(SwingSet): Split syscall processing into its own crank --- packages/SwingSet/src/kernel/kernel.js | 81 ++++++++++++++----- .../SwingSet/test/devices/test-devices.js | 30 ++++--- packages/SwingSet/test/gc/test-gc-vat.js | 2 + .../test/run-policy/test-run-policy.js | 14 ++-- packages/SwingSet/test/test-controller.js | 22 +++-- packages/SwingSet/test/test-gc-kernel.js | 42 ++++++---- packages/SwingSet/test/test-kernel.js | 20 +++++ 7 files changed, 157 insertions(+), 54 deletions(-) diff --git a/packages/SwingSet/src/kernel/kernel.js b/packages/SwingSet/src/kernel/kernel.js index 52fe26535986..2dd2974ee4b5 100644 --- a/packages/SwingSet/src/kernel/kernel.js +++ b/packages/SwingSet/src/kernel/kernel.js @@ -731,15 +731,8 @@ export default function buildKernel( const gcMessages = ['dropExports', 'retireExports', 'retireImports']; - function processSyscallQueue() { - while (!kernelKeeper.isSyscallQueueEmpty()) { - const syscallMessage = kernelKeeper.getNextSyscallQueueMsg(); - kernelKeeper.addToRunQueue(syscallMessage); - } - } - let processQueueRunning; - async function processQueueMessage(message) { + async function processDeliveryMessage(message) { kdebug(`processQ ${JSON.stringify(message)}`); kdebug(legibilizeMessage(message)); if (processQueueRunning) { @@ -803,7 +796,7 @@ export default function buildKernel( // delivered again on the next crank. If we don't want that, then // we need to remove it again. // eslint-disable-next-line no-use-before-define - getNextMessage(); + getNextDeliveryMessage(); } } // state changes reflecting the termination must also survive, so @@ -833,6 +826,38 @@ export default function buildKernel( return harden(policyInput); } + async function processSyscallMessage(message) { + kdebug(`processSyscallQ ${JSON.stringify(message)}`); + kdebug(legibilizeMessage(message)); + if (processQueueRunning) { + console.error(`We're currently already running at`, processQueueRunning); + assert.fail(X`Kernel reentrancy is forbidden`); + } + kernelSlog.write({ type: 'crank-start', message }); + /** @type { PolicyInput } */ + const policyInput = ['none']; + try { + processQueueRunning = Error('here'); + + kernelKeeper.addToRunQueue(message); + + kernelKeeper.processRefcounts(); + kernelKeeper.saveStats(); + const crankNum = kernelKeeper.getCrankNumber(); + kernelKeeper.incrementCrankNumber(); + const { crankhash, activityhash } = kernelKeeper.commitCrank(); + kernelSlog.write({ + type: 'crank-finish', + crankNum, + crankhash, + activityhash, + }); + } finally { + processQueueRunning = undefined; + } + return harden(policyInput); + } + const gcTools = harden({ WeakRef, FinalizationRegistry, @@ -1118,7 +1143,7 @@ export default function buildKernel( kernelKeeper.loadStats(); } - function getNextMessage() { + function getNextDeliveryMessage() { const gcMessage = processNextGCAction(kernelKeeper); if (gcMessage) { return gcMessage; @@ -1134,6 +1159,28 @@ export default function buildKernel( return undefined; } + function getNextSyscallMessage() { + if (!kernelKeeper.isSyscallQueueEmpty()) { + return kernelKeeper.getNextSyscallQueueMsg(); + } + return undefined; + } + + function maybeProcessNextMessage() { + /** @type {Promise | undefined} */ + let resultPromise; + let message = getNextSyscallMessage(); + if (message) { + resultPromise = processSyscallMessage(message); + } else { + message = getNextDeliveryMessage(); + if (message) { + resultPromise = processDeliveryMessage(message); + } + } + return { resultPromise }; + } + async function step() { if (kernelPanic) { throw kernelPanic; @@ -1141,11 +1188,10 @@ export default function buildKernel( if (!started) { throw new Error('must do kernel.start() before step()'); } - processSyscallQueue(); + const { resultPromise } = maybeProcessNextMessage(); // process a single message - const message = getNextMessage(); - if (message) { - await processQueueMessage(message); + if (resultPromise) { + await resultPromise; if (kernelPanic) { throw kernelPanic; } @@ -1174,15 +1220,14 @@ export default function buildKernel( } let count = 0; for (;;) { - processSyscallQueue(); - const message = getNextMessage(); - if (!message) { + const { resultPromise } = maybeProcessNextMessage(); + if (!resultPromise) { break; } count += 1; /** @type { PolicyInput } */ // eslint-disable-next-line no-await-in-loop - const policyInput = await processQueueMessage(message); + const policyInput = await resultPromise; if (kernelPanic) { throw kernelPanic; } diff --git a/packages/SwingSet/test/devices/test-devices.js b/packages/SwingSet/test/devices/test-devices.js index 929607883f86..680c64a4a0a4 100644 --- a/packages/SwingSet/test/devices/test-devices.js +++ b/packages/SwingSet/test/devices/test-devices.js @@ -122,7 +122,8 @@ test.serial('d1', async t => { await c.run(); c.queueToVatRoot('bootstrap', 'step1', capargs([])); - await c.step(); + await c.step(); // syscall + await c.step(); // deliver t.deepEqual(c.dump().log, [ 'callNow', 'invoke 1 2', @@ -155,10 +156,13 @@ async function test2(t, mode) { const c = await makeSwingsetController(hostStorage, {}); for (let i = 0; i < 5; i += 1) { // eslint-disable-next-line no-await-in-loop - await c.step(); // vat start + await c.step(); // vat start syscall + // eslint-disable-next-line no-await-in-loop + await c.step(); // vat start deliver } c.pinVatRoot('bootstrap'); - await c.step(); + await c.step(); // syscall + await c.step(); // deliver if (mode === '1') { t.deepEqual(c.dump().log, ['calling d2.method1', 'method1 hello', 'done']); } else if (mode === '2') { @@ -187,7 +191,8 @@ async function test2(t, mode) { ]); } else if (mode === '5') { t.deepEqual(c.dump().log, ['calling v2.method5', 'called']); - await c.step(); + await c.step(); // syscall + await c.step(); // deliver t.deepEqual(c.dump().log, [ 'calling v2.method5', 'called', @@ -195,7 +200,8 @@ async function test2(t, mode) { 'method5 hello', 'left5 did d2.method5, got ok', ]); - await c.step(); + await c.step(); // syscall + await c.step(); // deliver t.deepEqual(c.dump().log, [ 'calling v2.method5', 'called', @@ -346,11 +352,14 @@ test.serial('liveslots throws when D() gets promise', async t => { for (let i = 0; i < 4; i += 1) { // eslint-disable-next-line no-await-in-loop - await c.step(); // vat start + await c.step(); // vat start syscall + // eslint-disable-next-line no-await-in-loop + await c.step(); // vat start deliver // eslint-disable-next-line no-await-in-loop await c.step(); // bring out your dead } - await c.step(); + await c.step(); // syscall + await c.step(); // deliver // When liveslots catches an attempt to send a promise into D(), it throws // a regular error, which the vat can catch. t.deepEqual(c.dump().log, ['sending Promise', 'good: callNow failed']); @@ -388,11 +397,14 @@ test.serial('syscall.callNow(promise) is vat-fatal', async t => { const c = await makeSwingsetController(hostStorage, {}); for (let i = 0; i < 3; i += 1) { // eslint-disable-next-line no-await-in-loop - await c.step(); // vat start + await c.step(); // vat start syscall + // eslint-disable-next-line no-await-in-loop + await c.step(); // vat start deliver // eslint-disable-next-line no-await-in-loop await c.step(); // bring out your dead } - await c.step(); // bootstrap, which will fail + await c.step(); // syscall + await c.step(); // deliver bootstrap, which will fail // if the kernel paniced, that c.step() will reject, and the await will throw t.deepEqual(c.dump().log, ['sending Promise', 'good: callNow failed']); // now check that the vat was terminated: this should throw an exception diff --git a/packages/SwingSet/test/gc/test-gc-vat.js b/packages/SwingSet/test/gc/test-gc-vat.js index 1c82625186e2..dea20db82c20 100644 --- a/packages/SwingSet/test/gc/test-gc-vat.js +++ b/packages/SwingSet/test/gc/test-gc-vat.js @@ -52,9 +52,11 @@ async function dropPresence(t, dropExport) { c.queueToVatRoot('bootstrap', 'one', capargs([])); if (dropExport) { c.queueToVatRoot('bootstrap', 'drop', capargs([])); + await c.step(); // syscall await c.step(); // message await c.step(); // reap } + await c.step(); // syscall await c.step(); // message await c.step(); // reap diff --git a/packages/SwingSet/test/run-policy/test-run-policy.js b/packages/SwingSet/test/run-policy/test-run-policy.js index 4b023e0f2af6..7da3b19899e7 100644 --- a/packages/SwingSet/test/run-policy/test-run-policy.js +++ b/packages/SwingSet/test/run-policy/test-run-policy.js @@ -67,17 +67,17 @@ async function testCranks(t, mode) { let more; if (mode === 'messages' || mode === 'resolutions') { - more = await c.run(crankCounter(7, 0, true)); + more = await c.run(crankCounter(15, 0, true)); t.truthy(more, 'vat was supposed to run forever'); - t.is(elapsedCranks(), 7); + t.is(elapsedCranks(), 15); - more = await c.run(crankCounter(1, 0, true)); + more = await c.run(crankCounter(2, 0, true)); t.truthy(more, 'vat was supposed to run forever'); - t.is(elapsedCranks(), 1); + t.is(elapsedCranks(), 2); - more = await c.run(crankCounter(8, 0, true)); + more = await c.run(crankCounter(16, 0, true)); t.truthy(more, 'vat was supposed to run forever'); - t.is(elapsedCranks(), 8); + t.is(elapsedCranks(), 16); } else if (mode === 'computrons') { // the doMessage cycle has four steps: // 1: normal delivery (122k-134k computrons) @@ -91,7 +91,7 @@ async function testCranks(t, mode) { // setting a threshold of 4M, we should finish c.run() just after that // extra-compute step. await c.run(computronCounter(4_000_000n)); - t.is(elapsedCranks(), 17); + t.is(elapsedCranks(), 35); const ckey = `${rightID}.vs.vvs.seqnum`; const seqnum = parseInt(hostStorage.kvStore.get(ckey), 10); t.is(seqnum, 5); diff --git a/packages/SwingSet/test/test-controller.js b/packages/SwingSet/test/test-controller.js index 0c3c195de7cd..91feaffdb6dc 100644 --- a/packages/SwingSet/test/test-controller.js +++ b/packages/SwingSet/test/test-controller.js @@ -119,7 +119,9 @@ test('bootstrap', async t => { const c = await buildVatController(config); for (let i = 0; i < 5; i += 1) { // eslint-disable-next-line no-await-in-loop - await c.step(); // vat starts + await c.step(); // vat start syscall + // eslint-disable-next-line no-await-in-loop + await c.step(); // vat start deliver } t.deepEqual(c.dump().log, ['bootstrap called']); }); @@ -133,7 +135,9 @@ test('XS bootstrap', async t => { const c = await buildVatController(config, [], { hostStorage }); for (let i = 0; i < 5; i += 1) { // eslint-disable-next-line no-await-in-loop - await c.step(); // vat starts + await c.step(); // vat start syscall + // eslint-disable-next-line no-await-in-loop + await c.step(); // vat start deliver } t.deepEqual(c.dump().log, ['bootstrap called']); t.is( @@ -170,7 +174,9 @@ test('static vats are unmetered on XS', async t => { ); for (let i = 0; i < 5; i += 1) { // eslint-disable-next-line no-await-in-loop - await c.step(); // vat starts + await c.step(); // vat start syscall + // eslint-disable-next-line no-await-in-loop + await c.step(); // vat start deliver } t.deepEqual(c.dump().log, ['bootstrap called']); t.deepEqual(limited, [false, false, false, false]); @@ -265,9 +271,9 @@ test.serial('bootstrap export', async t => { }, ]); - // this test was designed before GC, and wants to single-step the kernel, - // but doesn't care about the GC action steps, so we use this helper - // function + // this test was designed before GC and syscall queues, and wants to + // single-step the kernel, but doesn't care about the GC action steps, + // or the temporary syscall queue, so we use this helper function async function stepGC() { while (c.dump().gcActions.length) { // eslint-disable-next-line no-await-in-loop @@ -277,6 +283,10 @@ test.serial('bootstrap export', async t => { // eslint-disable-next-line no-await-in-loop await c.step(); } + while (c.dump().syscallQueue.length) { + // eslint-disable-next-line no-await-in-loop + await c.step(); + } await c.step(); // the non- GC action } diff --git a/packages/SwingSet/test/test-gc-kernel.js b/packages/SwingSet/test/test-gc-kernel.js index 9e1b06b1e244..fddf444bc1cf 100644 --- a/packages/SwingSet/test/test-gc-kernel.js +++ b/packages/SwingSet/test/test-gc-kernel.js @@ -324,7 +324,8 @@ async function prep(t, options = {}) { if (sendToSelf) { kernel.queueToKref(alice, 'one-alice', capdataOneSlot(alice), 'none'); - await kernel.step(); + await kernel.step(); // syscall + await kernel.step(); // deliver } // look up amy's kref @@ -424,7 +425,8 @@ async function testDropAndRetire(t, mode) { const { amy, bob, vatA, vrefs } = p; // tell bob to both drop and retire amy p.kernel.queueToKref(bob, 'drop and retire', capargs([]), 'none'); - await p.kernel.step(); + await p.kernel.step(); // syscall + await p.kernel.step(); // deliver t.true(p.aliceClistPresent()); t.false(p.bobClistPresent()); t.false(p.amyRetired()); @@ -476,7 +478,8 @@ async function testDrop(t, mode) { // tell bob to drop amy, but not retire p.kernel.queueToKref(bob, 'drop', capargs([]), 'none'); - await p.kernel.step(); + await p.kernel.step(); // syscall + await p.kernel.step(); // deliver t.true(p.aliceClistPresent()); t.true(p.bobClistPresent()); t.false(p.amyRetired()); @@ -505,7 +508,8 @@ async function testDrop(t, mode) { if (mode === '24A') { // 24A: alice emits retire first p.kernel.queueToKref(alice, 'retire', capargs([]), 'none'); - await p.kernel.step(); + await p.kernel.step(); // syscall + await p.kernel.step(); // deliver // that deletes the kobj and queues a retire to the importers (bob) t.false(p.aliceClistPresent()); t.true(p.bobClistPresent()); @@ -515,7 +519,8 @@ async function testDrop(t, mode) { // 24B: bob emits retire first // console.log(`++ telling bob to retire first`); p.kernel.queueToKref(bob, 'retire', capargs([]), 'none'); - await p.kernel.step(); + await p.kernel.step(); // syscall + await p.kernel.step(); // deliver // that was the last importer: queue a retire to the exporter t.true(p.aliceClistPresent()); t.false(p.bobClistPresent()); @@ -621,7 +626,8 @@ test('two importers both drop+retire', async t => { const { amy, bob, carol, vatA, vrefs } = p; // tell bob to drop+retire amy p.kernel.queueToKref(bob, 'drop and retire', capargs([]), 'none'); - await p.kernel.step(); + await p.kernel.step(); // syscall + await p.kernel.step(); // deliver t.deepEqual( p.logB.shift(), makeMessage(vrefs.bobForBob, 'drop and retire', capargs([])), @@ -654,7 +660,8 @@ test('two importers both drop but not retire', async t => { const { amy, bob, carol, vatA, vatB, vatC, vrefs } = p; // tell bob to drop amy, but not retire p.kernel.queueToKref(bob, 'drop', capargs([]), 'none'); - await p.kernel.step(); + await p.kernel.step(); // syscall + await p.kernel.step(); // deliver t.deepEqual( p.logB.shift(), makeMessage(vrefs.bobForBob, 'drop', capargs([])), @@ -667,7 +674,8 @@ test('two importers both drop but not retire', async t => { p.gcActionsAre([]); // carol drops amy too p.kernel.queueToKref(carol, 'drop', capargs([]), 'none'); - await p.kernel.step(); + await p.kernel.step(); // syscall + await p.kernel.step(); // deliver t.deepEqual( p.logC.shift(), makeMessage(vrefs.carolForCarol, 'drop', capargs([])), @@ -714,7 +722,8 @@ test('two importers: drop+retire, cross-import, drop+retire', async t => { // tell carol to drop+retire amy p.kernel.queueToKref(carol, 'drop and retire', capargs([]), 'none'); - await p.kernel.step(); + await p.kernel.step(); // syscall + await p.kernel.step(); // deliver t.deepEqual( p.logC.shift(), makeMessage(vrefs.carolForCarol, 'drop and retire', capargs([])), @@ -747,7 +756,8 @@ test('two importers: drop+retire, cross-import, drop+retire', async t => { // bob drops+retires p.kernel.queueToKref(bob, 'drop and retire', capargs([]), 'none'); - await p.kernel.step(); + await p.kernel.step(); // syscall + await p.kernel.step(); // deliver t.deepEqual( p.logB.shift(), makeMessage(vrefs.bobForBob, 'drop and retire', capargs([])), @@ -791,7 +801,8 @@ test('promise resolution holds the only reference', async t => { // step far enough to retire the GC actions // there should be a notify(carol) on the queue, and no GC actions - await p.kernel.step(); + await p.kernel.step(); // syscall + await p.kernel.step(); // deliver // amy should now be held alive by only the resolved promise data t.true(p.aliceClistPresent()); @@ -809,7 +820,8 @@ test('promise resolution holds the only reference', async t => { // when the notify(carol) runs, carol should acquire a reference, and the // resolved promise goes away, so the refcount should be back to 1 - await p.kernel.step(); + await p.kernel.step(); // syscall + await p.kernel.step(); // deliver t.true(p.aliceClistPresent()); t.false(p.bobClistPresent()); t.true(p.carolClistPresent()); @@ -872,7 +884,8 @@ test('promise queue holds the only reference, resolved', async t => { // tell carol to resolve the promise (to herself), transferring // 'queued-message()' from the promise queue to the regular run-queue p.kernel.queueToKref(carol, 'resolve-result', capargs([]), 'none'); - await p.kernel.step(); + await p.kernel.step(); // syscall + await p.kernel.step(); // deliver t.deepEqual( p.logC.shift(), makeMessage(vrefs.carolForCarol, 'resolve-result', capargs([])), @@ -984,7 +997,8 @@ test('message to self', async t => { p.gcActionsAre([]); // deliver the message-to-self, which should drop the refcount to 0 - await p.kernel.step(); + await p.kernel.step(); // syscall + await p.kernel.step(); // deliver t.true(p.aliceClistPresent()); t.false(p.amyRetired()); t.deepEqual(dumpObjects(p.kernel)[amy], [vatA, 0, 0]); diff --git a/packages/SwingSet/test/test-kernel.js b/packages/SwingSet/test/test-kernel.js index b340d8844a12..da0da75369e0 100644 --- a/packages/SwingSet/test/test-kernel.js +++ b/packages/SwingSet/test/test-kernel.js @@ -303,6 +303,9 @@ test('outbound call', async t => { }, ]); + // Move the send to the run-queue + await kernel.step(); + // Deliver the send await kernel.step(); // that queues pid=o2!bar(o2, o7, p7) @@ -357,6 +360,7 @@ test('outbound call', async t => { kt.push(['kp42', vat1, 'p+5']); checkKT(t, kernel, kt); + await kernel.step(); await kernel.step(); t.deepEqual(log, [ // todo: check result @@ -514,6 +518,9 @@ test('three-party', async t => { const o4 = kernel.addExport(vatA, 'o+4'); kernel.queueToKref(o4, 'foo', capdata('args')); + // Move the send to the run-queue + await kernel.step(); + // Deliver the send await kernel.step(); t.deepEqual(log.shift(), ['vatA', 'o+4', 'foo', capdata('args')]); @@ -565,6 +572,7 @@ test('three-party', async t => { kt.push(['kp42', vatA, 'p+5']); checkKT(t, kernel, kt); + await kernel.step(); await kernel.step(); t.deepEqual(log, [['vatB', 'o+5', 'intro', capdata('bargs', ['o-50'])]]); kt.push([carol, vatB, 'o-50']); @@ -779,6 +787,9 @@ test('promise resolveToData', async t => { }, ]); + // Move the notify to the run-queue + await kernel.step(); + // Deliver the notify await kernel.step(); // the kernelPromiseID gets mapped back to the vat PromiseID t.deepEqual(log.shift(), [ @@ -853,6 +864,9 @@ test('promise resolveToPresence', async t => { }, ]); + // Move the notify to the run-queue + await kernel.step(); + // Deliver the notify await kernel.step(); t.deepEqual(log.shift(), [ 'notify', @@ -925,6 +939,9 @@ test('promise reject', async t => { }, ]); + // Move the notify to the run-queue + await kernel.step(); + // Deliver the notify await kernel.step(); // the kernelPromiseID gets mapped back to the vat PromiseID t.deepEqual(log.shift(), [ @@ -960,6 +977,9 @@ test('transcript', async t => { const bobForAlice = kernel.addImport(vatA, bob); kernel.queueToKref(alice, 'store', capdata('args string', [alice, bob])); + // Move the send to the run-queue + await kernel.step(); + // Deliver the send await kernel.step(); // the transcript records vat-specific import/export slots