Skip to content

Commit

Permalink
fix(swingset): don't assume we can cache GC actions in processRefcounts
Browse files Browse the repository at this point in the history
Previously, processRefcounts() populated a local RAM cache upon entry
with getGCActions(), added things to it during operation, then saved
it with setGCActions() at the end. The intention was to avoid lots of
redundant kvStore gets/sets as we loop over a (potentially large)
number of changes. But, this approach would lose actions if we called
other functions in the middle which did their own additions (eg with
addGCActions).

Since we only add actions, never remove them, it's just as fast (and
infinitely less buggy) to accumulate a RAM cache of *new* actions, and
add them all at the end, with addGCActions().

We don't make calls in the middle yet, but upcoming changes might
start doing that, so best to fix this bug now.

closes #5054
  • Loading branch information
warner committed Aug 11, 2024
1 parent ff5bf48 commit 92b728a
Showing 1 changed file with 2 additions and 2 deletions.
4 changes: 2 additions & 2 deletions packages/SwingSet/src/kernel/state/kernelKeeper.js
Original file line number Diff line number Diff line change
Expand Up @@ -1493,7 +1493,7 @@ export default function makeKernelKeeper(

function processRefcounts() {
if (enableKernelGC) {
const actions = getGCActions(); // local cache
const actions = new Set();
// TODO (else buggy): change the iteration to handle krefs that get
// added multiple times (while we're iterating), because we might do
// different work the second time around. Something like an ordered
Expand Down Expand Up @@ -1541,7 +1541,7 @@ export default function makeKernelKeeper(
}
}
}
setGCActions(actions);
addGCActions([...actions]);
}
maybeFreeKrefs.clear();
}
Expand Down

0 comments on commit 92b728a

Please sign in to comment.