Skip to content

Commit

Permalink
Merge pull request #3296 from Agoric/3109-new-gc-actions
Browse files Browse the repository at this point in the history 
fix(swingset): gc-actions: new algorithm, update test
  • Loading branch information
@warner
warner authored Jun 12, 2021
2 parents 2a2081d + fe5a721 commit 4e02c6a
Show file tree
Hide file tree
Showing 2 changed files with 108 additions and 21 deletions.
85 changes: 66 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,79 @@ 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);
// GC actions are each one of 'dropExport', 'retireExport', or
// 'retireImport', aimed at a specific vat and affecting a specific kref.
// They are added to the durable "GC Actions" set (stored in kernelDB) when
// `processRefcounts` notices a refcount sitting at zero, which means some
// vat needs to be told that an object can be freed. Before each crank, the
// kernel calls processNextGCAction to see if there are any GC actions that
// should be taken. All such GC actions are executed before any regular vat
// delivery gets to run.

// However, things might have changed between the time the action was
// pushed into the durable set and the time the kernel is ready to execute
// it. For example, the kref might have been re-exported: we were all set
// to tell the exporting vat that their object isn't recognizable any more
// (with a `dispatch.retireExport`), but then they sent a brand new copy to
// some importer. We must negate the `retireExport` action, because it's no
// longer the right thing to do. Alternatively, the exporting vat might
// have deleted the object itself (`syscall.retireExport`) before the
// kernel got a chance to deliver the `dispatch.retireExport`, which means
// we must bypass the action as redundant (since it's an error to delete
// the same c-list entry twice).

// This `filterAction` function looks at each queued GC Action and decides
// whether the current state of the c-lsits and reference counts warrants
// permits the action to run, or if it should be negated/bypassed.

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) {
// We process actions in groups (sorted first by vat, then by type), to
// make it deterministic, and to ensure that `dropExport` happens before
// `retireExport`. This examines one group at a time, filtering everything
// in that group, and returning the survivors of the first group that
// wasn't filtered out entirely. Our available dispatch functions take
// multiple krefs (`dispatch.dropExports`, rather than
// `dispatch.dropExport`), so the set of surviving krefs can all be
// delivered to a vat in a single crank.

// Some day we may consolidate the three GC delivery methods into a single
// one, in which case we'll batch together an entire vat's worth of
// actions, instead of the narrower (vat+type) group. The filtering rules
// may need to change to support that, to ensure that `dropExport` and
// `retireExport` can both be delivered.

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 +105,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 +114,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 4e02c6a

Please sign in to comment.