From 6c85e21831f0c3f867686d4b5f5f66a25f1acdeb Mon Sep 17 00:00:00 2001 From: Brian Warner Date: Thu, 10 Jun 2021 18:27:43 -0700 Subject: [PATCH] fix(swingset): gc-actions: new algorithm, update test Implement the new algorithm to decide when a GC Action should be negated or bypassed. --- packages/SwingSet/src/kernel/gc-actions.js | 45 +++++++++++++--------- packages/SwingSet/test/test-gc-actions.js | 44 ++++++++++++++++++++- 2 files changed, 68 insertions(+), 21 deletions(-) diff --git a/packages/SwingSet/src/kernel/gc-actions.js b/packages/SwingSet/src/kernel/gc-actions.js index 9f6ca4dad0a..bf7e6b2636c 100644 --- a/packages/SwingSet/src/kernel/gc-actions.js +++ b/packages/SwingSet/src/kernel/gc-actions.js @@ -15,31 +15,39 @@ function parseAction(s) { export function processNextGCAction(kernelKeeper) { const allActionsSet = kernelKeeper.getGCActions(); - function getRefCount(kref) { - // When we check for a re-exported kref, if it's entirely missing, that - // qualifies (to us) as a zero refcount. - const owner = kernelKeeper.ownerOfKernelObject(kref); - if (owner) { - return kernelKeeper.getObjectRefCount(kref); + function filterAction(vatKeeper, action, type, kref) { + const hasCList = vatKeeper.hasCListEntry(kref); + const isReachable = hasCList ? vatKeeper.getReachableFlag(kref) : undefined; + const exists = kernelKeeper.kernelObjectExists(kref); + const { reachable, recognizable } = exists + ? kernelKeeper.getObjectRefCount(kref) + : {}; + + if (type === 'dropExport') { + if (!exists) return false; // already, shouldn't happen + if (reachable) return false; // negated + if (!hasCList) return false; // already, shouldn't happen + if (!isReachable) return false; // already, shouldn't happen + } + if (type === 'retireExport') { + if (!exists) return false; // already + if (reachable || recognizable) return false; // negated + if (!hasCList) return false; // already } - return { reachable: 0, recognizable: 0 }; + if (type === 'retireImport') { + if (!hasCList) return false; // already + } + return true; } - function filterActions(groupedActions) { + function filterActions(vatID, groupedActions) { + const vatKeeper = kernelKeeper.provideVatKeeper(vatID); const krefs = []; - const actions = []; for (const action of groupedActions) { const { type, kref } = parseAction(action); - const { reachable, recognizable } = getRefCount(kref); - // negate actions on re-exported krefs, and don't treat as work to do - if (reachable || (type === 'retireExport' && recognizable)) { - allActionsSet.delete(action); - } else { + if (filterAction(vatKeeper, action, type, kref)) { krefs.push(kref); - actions.push(action); } - } - for (const action of actions) { allActionsSet.delete(action); } return krefs; @@ -57,7 +65,6 @@ export function processNextGCAction(kernelKeeper) { } forVat.get(type).push(action); } - // console.log(`grouped:`, grouped); const vatIDs = Array.from(grouped.keys()); vatIDs.sort(); @@ -67,7 +74,7 @@ export function processNextGCAction(kernelKeeper) { for (const type of typePriority) { if (forVat.has(type)) { const actions = forVat.get(type); - const krefs = filterActions(actions); + const krefs = filterActions(vatID, actions); if (krefs.length) { // at last, we act krefs.sort(); diff --git a/packages/SwingSet/test/test-gc-actions.js b/packages/SwingSet/test/test-gc-actions.js index 88541e5a318..aaca62d5ae6 100644 --- a/packages/SwingSet/test/test-gc-actions.js +++ b/packages/SwingSet/test/test-gc-actions.js @@ -12,6 +12,8 @@ test('gc actions', t => { actions = a; newActions = Array.from(a); } + const clistState = { v1: { ko1: {}, ko2: {} }, v2: { ko2: {} } }; + const kernelKeeper = { getGCActions() { return new Set(actions); @@ -20,13 +22,23 @@ test('gc actions', t => { newActions = Array.from(a); newActions.sort(); }, - ownerOfKernelObject(kref) { - return rc[kref] ? 'vatX' : undefined; + kernelObjectExists(kref) { + return !!rc[kref]; }, getObjectRefCount(kref) { const [reachable, recognizable] = rc[kref]; return { reachable, recognizable }; }, + provideVatKeeper(vatID) { + return { + hasCListEntry(kref) { + return !!clistState[vatID][kref].exists; + }, + getReachableFlag(kref) { + return !!clistState[vatID][kref].isReachable; + }, + }; + }, }; function process() { return processNextGCAction(kernelKeeper); @@ -46,8 +58,10 @@ test('gc actions', t => { // fully dropped. the dropExport takes priority setActions(['v1 dropExport ko1', 'v1 retireExport ko1']); rc = { ko1: [0, 0] }; + clistState.v1.ko1 = { exists: true, isReachable: true }; msg = process(); t.deepEqual(msg, make('dropExports', 'v1', 'ko1')); + clistState.v1.ko1 = { exists: true, isReachable: false }; t.deepEqual(newActions, ['v1 retireExport ko1']); // then the retireExport setActions(['v1 retireExport ko1']); @@ -59,6 +73,7 @@ test('gc actions', t => { // fully dropped, then fully re-reachable before dropExports: both negated setActions(['v1 dropExport ko1', 'v1 retireExport ko1']); rc = { ko1: [1, 1] }; // re-exported, still reachable+recognizable + clistState.v1.ko1 = { exists: true, isReachable: true }; msg = process(); t.deepEqual(msg, undefined); t.deepEqual(newActions, []); @@ -66,11 +81,13 @@ test('gc actions', t => { // fully dropped, dropExport happens, then fully re-reachable: retire negated setActions(['v1 dropExport ko1', 'v1 retireExport ko1']); rc = { ko1: [0, 0] }; + clistState.v1.ko1 = { exists: true, isReachable: true }; msg = process(); t.deepEqual(msg, make('dropExports', 'v1', 'ko1')); t.deepEqual(newActions, ['v1 retireExport ko1']); setActions(['v1 retireExport ko1']); rc = { ko1: [1, 1] }; + clistState.v1.ko1 = { exists: true, isReachable: false }; msg = process(); t.deepEqual(msg, undefined); t.deepEqual(newActions, []); @@ -79,9 +96,11 @@ test('gc actions', t => { rc = { ko1: [0, 0] }; setActions(['v1 dropExport ko1', 'v1 retireExport ko1']); rc = { ko1: [1, 1] }; + clistState.v1.ko1 = { exists: true, isReachable: true }; rc = { ko1: [0, 1] }; msg = process(); t.deepEqual(msg, make('dropExports', 'v1', 'ko1')); + clistState.v1.ko1 = { exists: true, isReachable: false }; t.deepEqual(newActions, ['v1 retireExport ko1']); // the retire is left pending because we ignore lower-prority types setActions(['v1 retireExport ko1']); @@ -93,11 +112,13 @@ test('gc actions', t => { // fully dropped, dropExports happens, re-reachable, partial drop: retire // negated setActions(['v1 dropExport ko1', 'v1 retireExport ko1']); + clistState.v1.ko1 = { exists: true, isReachable: true }; rc = { ko1: [0, 0] }; msg = process(); t.deepEqual(msg, make('dropExports', 'v1', 'ko1')); t.deepEqual(newActions, ['v1 retireExport ko1']); setActions(['v1 retireExport ko1']); + clistState.v1.ko1 = { exists: true, isReachable: true }; rc = { ko1: [0, 1] }; msg = process(); t.deepEqual(msg, undefined); @@ -106,6 +127,7 @@ test('gc actions', t => { // partially dropped: recognizable but not reachable setActions(['v1 dropExport ko1']); rc = { ko1: [0, 1] }; // recognizable, not reachable + clistState.v1.ko1 = { exists: true, isReachable: true }; msg = process(); t.deepEqual(msg, make('dropExports', 'v1', 'ko1')); t.deepEqual(newActions, []); @@ -113,6 +135,7 @@ test('gc actions', t => { // partially dropped, re-reachable: negate dropExports setActions(['v1 dropExport ko1']); rc = { ko1: [1, 1] }; + clistState.v1.ko1 = { exists: true, isReachable: true }; msg = process(); t.deepEqual(msg, undefined); t.deepEqual(newActions, []); @@ -120,25 +143,40 @@ test('gc actions', t => { // priority order: retireImports is last setActions(['v1 dropExport ko1', 'v1 retireImport ko2']); rc = { ko1: [0, 0], ko2: [0, 0] }; + clistState.v1.ko1 = { exists: true, isReachable: true }; + clistState.v1.ko2 = { exists: true, isReachable: false }; msg = process(); t.deepEqual(msg, make('dropExports', 'v1', 'ko1')); t.deepEqual(newActions, ['v1 retireImport ko2']); setActions(['v1 retireExport ko1', 'v1 retireImport ko2']); rc = { ko1: [0, 0], ko2: [0, 0] }; + clistState.v1.ko1 = { exists: true, isReachable: true }; + clistState.v1.ko2 = { exists: true, isReachable: false }; msg = process(); t.deepEqual(msg, make('retireExports', 'v1', 'ko1')); t.deepEqual(newActions, ['v1 retireImport ko2']); setActions(['v1 retireImport ko2']); rc = { ko1: [0, 0], ko2: [0, 0] }; + clistState.v1.ko2 = { exists: true, isReachable: false }; msg = process(); t.deepEqual(msg, make('retireImports', 'v1', 'ko2')); t.deepEqual(newActions, []); + // retireImport but was already done + setActions(['v1 retireImport ko2']); + rc = { ko1: [0, 0], ko2: [0, 0] }; + clistState.v1.ko2 = { exists: false, isReachable: false }; + msg = process(); + t.deepEqual(msg, undefined); + t.deepEqual(newActions, []); + // multiple vats: process in sorted order setActions(['v1 dropExport ko1', 'v2 dropExport ko2']); rc = { ko1: [0, 0], ko2: [0, 0] }; + clistState.v1.ko1 = { exists: true, isReachable: true }; + clistState.v2.ko2 = { exists: true, isReachable: true }; msg = process(); t.deepEqual(msg, make('dropExports', 'v1', 'ko1')); t.deepEqual(newActions, ['v2 dropExport ko2']); @@ -146,6 +184,8 @@ test('gc actions', t => { // multiple vats: vatID is major sort order, type is minor setActions(['v1 retireExport ko1', 'v2 dropExport ko2']); rc = { ko1: [0, 0], ko2: [0, 0] }; + clistState.v1.ko1 = { exists: true, isReachable: false }; + clistState.v2.ko2 = { exists: true, isReachable: true }; msg = process(); t.deepEqual(msg, make('retireExports', 'v1', 'ko1')); t.deepEqual(newActions, ['v2 dropExport ko2']);