Skip to content

Commit

Permalink
fix(liveslots): collection.clear(pattern) vs getSize
Browse files Browse the repository at this point in the history
The `.clear()` method, on (strong) collections, will delete all entries.
But it can also be called with `keyPatt` and/or `valuePatt` argument,
and only the matching entries will be deleted.

Previously, the `|entryCount` metadata field was unconditionally set
to zero for any call to `.clear()`, even those with patterns which
caused some entries to be retained. This caused a subsequent
`collection.getSize()` to return the wrong value, yielding zero even
if the collection still had entries.

This commit fixes the logic to decrement the `|entryCount` field by
the number of entries actually deleted. As an optimization, it is
still set directly to zero if `clear()` is not limited by patterns.

fixes #10007
  • Loading branch information
warner committed Aug 31, 2024
1 parent 813208a commit f8806f3
Show file tree
Hide file tree
Showing 2 changed files with 12 additions and 9 deletions.
15 changes: 10 additions & 5 deletions packages/swingset-liveslots/src/collectionManager.js
Original file line number Diff line number Diff line change
Expand Up @@ -616,16 +616,21 @@ export function makeCollectionManager(

function clearInternal(isDeleting, keyPatt, valuePatt) {
let doMoreGC = false;
if (isDeleting || (matchAny(keyPatt) && matchAny(valuePatt))) {
if (isDeleting) {
doMoreGC = clearInternalFull();
// |entryCount will be deleted along with metadata keys
} else if (matchAny(keyPatt) && matchAny(valuePatt)) {
doMoreGC = clearInternalFull();
if (!hasWeakKeys) {
syscall.vatstoreSet(prefix('|entryCount'), '0');
}
} else {
let numDeleted = 0;
for (const k of keys(keyPatt, valuePatt)) {
numDeleted += 1;
doMoreGC = deleteInternal(k) || doMoreGC;
}
}
if (!hasWeakKeys && !isDeleting) {
// TODO: broken, see #10007
syscall.vatstoreSet(prefix('|entryCount'), '0');
updateEntryCount(-numDeleted);
}
return doMoreGC;
}
Expand Down
6 changes: 2 additions & 4 deletions packages/swingset-liveslots/test/collections.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -503,8 +503,7 @@ test('map clear', t => {
t.is(testStore.getSize(), 0);
});

// see #10007
test.failing('map clear with pattern', t => {
test('map clear with pattern', t => {
const testStore = makeScalarBigMapStore('cmap', { keyShape: M.any() });
testStore.init('a', 'ax');
testStore.init('b', 'bx');
Expand All @@ -529,8 +528,7 @@ test('set clear', t => {
t.is(testStore.getSize(), 0);
});

// see #10007
test.failing('set clear with pattern', t => {
test('set clear with pattern', t => {
const testStore = makeScalarBigSetStore('cset', { keyShape: M.any() });
testStore.add('a');
testStore.add('b');
Expand Down

0 comments on commit f8806f3

Please sign in to comment.