Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(swingset): gc-actions: new algorithm, update test #3296

Merged
merged 2 commits into from
Jun 12, 2021
Merged

fix(swingset): gc-actions: new algorithm, update test #3296

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
6c85e21
fix(swingset): gc-actions: new algorithm, update test
warner Jun 11, 2021
fe5a721
chore(swingset): add docs to processNextGCAction and filterAction
warner Jun 12, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
45 changes: 26 additions & 19 deletions packages/SwingSet/src/kernel/gc-actions.js
View file
Open in desktop
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) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This probably could use a comment explaining why the actions are being filtered, since the verb "filter" is pretty generic, i.e., what the respective meanings of the pre-filtration and post-filtration collections are supposed to be. I think what's happening is it's throwing away the actions that it's not actually going to perform this time through, but I'm not 100% sure my understanding is correct.

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
Open in desktop
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