Skip to content

Commit

Permalink
fix(swingset): gc-actions: new algorithm, update test
Browse files Browse the repository at this point in the history 
Implement the new algorithm to decide when a GC Action should be negated or
bypassed.
  • Loading branch information
@warner
warner committed Jun 11, 2021
1 parent 2ad50b8 commit 64de132
Show file tree
Hide file tree
Showing 2 changed files with 68 additions and 21 deletions.
45 changes: 26 additions & 19 deletions packages/SwingSet/src/kernel/gc-actions.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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();
Expand All @@ -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();
Expand Down
44 changes: 42 additions & 2 deletions packages/SwingSet/test/test-gc-actions.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand All @@ -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);
Expand All @@ -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']);
Expand All @@ -59,18 +73,21 @@ 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, []);

// 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, []);
Expand All @@ -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']);
Expand All @@ -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);
Expand All @@ -106,46 +127,65 @@ 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, []);

// 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, []);

// 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']);

// 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']);
Expand Down

0 comments on commit 64de132

Please sign in to comment.