From 7fcae41b1f5304454ea0128352f13dd7225c3a68 Mon Sep 17 00:00:00 2001 From: Brian Warner Date: Fri, 18 Jun 2021 23:51:55 -0700 Subject: [PATCH] feat(swingset): comms: add importer tracking To support `dispatch.retireImport` (the "downstream" retirement that happens when a Remotable is dropped, and downstream importers need to be informed of the loss), the comms vat needs to know a list of the kernel and/or remotes which have imported an object that the comms vat has exported. The kernel currently implements this in a simple and non-performant way, by reading every vat's c-list for a matching entry. This makes lookups (which happen only when an upstream export is deleted) `O(numVats)`, but has no space penalty. #3223 is about making this faster. For now, the comms vat uses a differently-non-ideal approach: for each lref, it maintains a key with a JSON-serialized list of remotes. Adding, removing, and querying the importers are all is `O(numImporters)`, with a space penalty of one key per object, and `O(numImporters)` space for the values. A faster approach is implemented, but commented out, which uses distinct keys for each (remoteID, lref) pair. This is `O(1)` for all operations, with a space penalty of one key per import (and a constant-size value). However, this approach would require a non-existent `syscall.vatstoreGetKeys()` API, to iterate over vatstore keys. This wouldn't be hard to add, but we should consider our storage model more carefully before deciding to commit to that feature. --- packages/SwingSet/src/vats/comms/remote.js | 10 ++++ packages/SwingSet/src/vats/comms/state.js | 64 ++++++++++++++++++++++ packages/SwingSet/test/test-comms.js | 5 ++ 3 files changed, 79 insertions(+) diff --git a/packages/SwingSet/src/vats/comms/remote.js b/packages/SwingSet/src/vats/comms/remote.js index 3664b53e7a34..5bd8dd7e99c4 100644 --- a/packages/SwingSet/src/vats/comms/remote.js +++ b/packages/SwingSet/src/vats/comms/remote.js @@ -97,6 +97,11 @@ export function makeRemote(state, store, remoteID) { store.set(toKey, flipRemoteSlot(rref)); const mode = isImport ? 'clist-import' : 'clist-export'; state.incrementRefCount(lref, `{rref}|${remoteID}|clist`, mode); + if (type === 'object') { + if (isImport) { + state.addImporter(lref, remoteID); + } + } } function deleteRemoteMapping(lref) { @@ -112,6 +117,11 @@ export function makeRemote(state, store, remoteID) { store.delete(`${remoteID}.c.${rrefInbound}`); store.delete(`${remoteID}.c.${lref}`); state.decrementRefCount(lref, `{rref}|${remoteID}|clist`, mode); + if (type === 'object') { + if (isImport) { + state.removeImporter(lref, remoteID); + } + } } function nextSendSeqNum() { diff --git a/packages/SwingSet/src/vats/comms/state.js b/packages/SwingSet/src/vats/comms/state.js index e360f5fab7c5..2269a2fa2171 100644 --- a/packages/SwingSet/src/vats/comms/state.js +++ b/packages/SwingSet/src/vats/comms/state.js @@ -88,6 +88,9 @@ export function makeState(syscall, identifierBase = 0) { // c.$kfref = $lref // inbound kernel-facing c-list (o+NN/o-NN/p+NN/p-NN -> loNN/lpNN) // c.$lref = $kfref // outbound kernel-facing c-list (loNN/lpNN -> o+NN/o-NN/p+NN/p-NN) // cr.$lref = 1 | // isReachable flag + // //imps.$lref.$remoteID = 1 // one key per importer of $lref (FUTURE) + // imps.$lref = JSON([remoteIDs]) // importers of $lref + // imps.$lref.$remoteID = 1 // one key per importer of $lref // meta.$kfref = true // flag that $kfref (o+NN/o-NN) is a directly addressable control object // // lo.nextID = $NN // local object identifier allocation counter (loNN) @@ -164,6 +167,53 @@ export function makeState(syscall, identifierBase = 0) { deleteLocalPromiseState(lpid); } + /* we need syscall.vatstoreGetKeys to do it this way + function addImporter(lref, remoteID) { + assert(!lref.includes('.'), lref); + const key = `imps.${lref}.${remoteID}`; + store.set(key, '1'); + } + function removeImporter(lref, remoteID) { + assert(!lref.includes('.'), lref); + const key = `imps.${lref}.${remoteID}`; + store.delete(key); + } + function getImporters(lref) { + const remoteIDs = []; + const prefix = `imps.${lref}`; + const startKey = `${prefix}.`; + const endKey = `${prefix}/`; // '.' and '/' are adjacent + for (const k of store.getKeys(startKey, endKey)) { + const remoteID = k.slice(0, prefix.length); + if (remoteID !== 'kernel') { + insistRemoteID(remoteID); + } + remoteIDs.push(remoteID); + } + return harden(remoteIDs); + } + */ + + function addImporter(lref, remoteID) { + const key = `imps.${lref}`; + const value = JSON.parse(store.get(key) || '[]'); + value.push(remoteID); + value.sort(); + store.set(key, JSON.stringify(value)); + } + function removeImporter(lref, remoteID) { + assert(!lref.includes('.'), lref); + const key = `imps.${lref}`; + let value = JSON.parse(store.get(key) || '[]'); + value = value.filter(r => r !== remoteID); + store.set(key, JSON.stringify(value)); + } + function getImporters(lref) { + const key = `imps.${lref}`; + const remoteIDs = JSON.parse(store.get(key) || '[]'); + return harden(remoteIDs); + } + /* A mode of 'clist-import' means we increment recognizable, but not * reachable, because the translation function will call setReachable in a * moment, and the count should only be changed if it wasn't already @@ -354,6 +404,11 @@ export function makeState(syscall, identifierBase = 0) { store.set(`c.${lref}`, kfref); const mode = isImport ? 'clist-import' : 'clist-export'; incrementRefCount(lref, `{kfref}|k|clist`, mode); + if (type === 'object') { + if (isImport) { + addImporter(lref, 'kernel'); + } + } } // GC or delete-remote should just call deleteKernelMapping without any @@ -370,6 +425,11 @@ export function makeState(syscall, identifierBase = 0) { store.delete(`c.${kfref}`); store.delete(`c.${lref}`); decrementRefCount(lref, `{kfref}|k|clist`, mode); + if (type === 'object') { + if (isImport) { + removeImporter(lref, 'kernel'); + } + } } function hasMetaObject(kfref) { @@ -620,6 +680,10 @@ export function makeState(syscall, identifierBase = 0) { getPromiseData, allocatePromise, + addImporter, + removeImporter, + getImporters, + changeReachable, lrefMightBeFree, incrementRefCount, diff --git a/packages/SwingSet/test/test-comms.js b/packages/SwingSet/test/test-comms.js index 06482c8dae88..99afda4f971a 100644 --- a/packages/SwingSet/test/test-comms.js +++ b/packages/SwingSet/test/test-comms.js @@ -28,6 +28,11 @@ test('provideRemoteForLocal', t => { t.is(provideRemoteForLocal(remoteID, lo4), 'ro-20'); t.is(provideRemoteForLocal(remoteID, lo4), 'ro-20'); t.is(provideRemoteForLocal(remoteID, lo5), 'ro-21'); + + t.deepEqual(s.getImporters(lo4), [remoteID]); + const { remoteID: remoteID2 } = s.addRemote('remote2', 'o-2'); + provideRemoteForLocal(remoteID2, lo4); + t.deepEqual(s.getImporters(lo4), [remoteID, remoteID2]); }); function mockSyscall() {