-
Notifications
You must be signed in to change notification settings - Fork 214
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.
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
feat(swingset-liveslots): endow passStyleOf to liveslots guest compartment #9874
Conversation
Update the swingset test to look for it there, and compare against a bundle-generated version (they should be different). This currently fails because importBundle() does not respect symbol-named properties of `vatGlobals`. Restored the SwingSet/package.json dependency on `@endo/pass-style` so the vat-under-test can get a bundle-generated version, for comparison. Added a liveslots-local test of the same, which passes because it's only looking at the `vatGlobals` given to `buildVatNamespace()`, and doesn't use `importBundle()`.
Ready for review! |
The only CI failure is
which I do not understand. Reviewers or anyone else, your advice would be appreciated. Thanks. |
…2408) Closes: #2407 Refs: Agoric/agoric-sdk#9874 Agoric/agoric-sdk#9431 Agoric/agoric-sdk#9781 #2377 ## Description While working on Agoric/agoric-sdk#9874 , I found that it still failed to endow `passStyleOf` via a symbol-named property as recently agree. Turns out there was still a lurking `Object.keys` on the compartment endowment pathway that needs to be changed to enumerate all enumerable own properties, whether string-named or symbol-named. ### Security Considerations None ### Scaling Considerations For this PR, none. But it enables liveslots to endow its `passStyleOf` to the compartments it makes, which has the scaling benefits explained at Agoric/agoric-sdk#9781 ### Documentation Considerations For the normal developer, none. Once endowed, `passStyleOf` will have performance that is less surprising. ### Testing Considerations Agoric/agoric-sdk#9874 both patches in the equivalent of this PR, uses it to endow `passStyleOf`, and test that it has done so, across both node and XS. However, after this fix, Agoric/agoric-sdk#9874 works on the Node host. But it still fails on the XS host for undiagnosed reasons, so there may be another bug like #2407 still lurking somewhere. Until diagnosed, it isn't clear if that remaining problem is an endo problem or an agoric-sdk problem. ### Compatibility and Upgrade Considerations Except for code meant only to test this, `passStyleOf` should not have any observable difference. We have not attempted to make an other use of symbol-named global properties, so there should be no compat or upgrade issues.
Confirming that passStyleOf was able to reuse an endowment was easy, but I just now finally assessed this code in a representative liveslots environment with a real- vs. virtual-cache liveslots, albeit with some hacking (updating Results: As hoped for (and expected), heap snapshots demonstrated that the virtual cache retains presences but the real cache does not. Further, GC duration always climbs in both V8 and XS for a yet-to-be-determined reason, but is faster with the real cache than with the virtual one (and on XS, GC with a real cache is almost twice as fast). And I also found another WeakMap subject to liveslots interference in
Experimental setup# Run on V8 and XS, with argument N=1000, a constant 10 observations per sample,
# 40 seconds of sampling (replaced with 0 for interactive debugging),
# and JSON output to see the increasing durations.
./esbench.mjs -h 'V8,*XS' -a N:1000 -r 10 -b 20 -t json \
-i '
import { getGCTriggerType, forceCollectionP } from "./gc-tools.js";
Object.assign(globalThis, { getGCTriggerType, forceCollectionP });
forceCollectionP.then(() => print(`GC trigger by ${getGCTriggerType()}`));
' \
-i '
// Trick packages/vat-data/src/vat-data-bindings.js into accepting
// a VatData with nothing actually in it (yet).
globalThis.VatData = new Proxy(
{},
{
get(target, k) {
return (target[k] ||= (...args) => realVatData[k](...args));
},
},
);
' \
-i '
// Define and start the vat.
import {
Far,
passStyleOf as goodPassStyleOf,
// Patched in for this testing:
makePassStyleOf,
} from "@endo/pass-style";
import { kslot, kser, kunser } from "@agoric/kmarshal";
import { makeDurableZone } from "@agoric/zone/durable.js";
import { makeLiveSlots } from "@agoric/swingset-liveslots/src/liveslots.js";
import { makeDummyMeterControl } from "@agoric/swingset-liveslots/test/dummyMeterControl.js";
import {
makeStartVat,
makeMessage,
makeBringOutYourDead,
} from "@agoric/swingset-liveslots/test/util.js";
const { stringify } = JSON;
const { Fail } = assert;
// cf. swingset-liveslots/test/liveslots-helpers.js `buildSyscall`
const { syscall, settlements } = (() => {
const settlements = new Map();
const vatstore = new Map();
let sortedKeys, priorKeyReturned, priorKeyIndex;
const ensureSorted = () => {
if (sortedKeys) return;
sortedKeys = [...vatstore.keys()];
sortedKeys.sort((a, b) => a.localeCompare(b));
};
const clearGetNextKeyCache = () => {
priorKeyReturned = undefined;
priorKeyIndex = -1;
};
const clearSorted = () => {
sortedKeys = undefined;
clearGetNextKeyCache();
};
clearSorted();
const syscall = {
send() {},
subscribe() {},
resolve(resolutions) {
for (const [vpid, isRejection, capdata] of resolutions) {
settlements.set(vpid, { isRejection, capdata });
}
},
dropImports() {},
retireImports() {},
retireExports() {},
exit() {},
vatstoreGet: key => vatstore.get(key),
vatstoreGetNextKey(priorKey) {
assert.typeof(priorKey, "string");
ensureSorted();
const start = priorKeyReturned === priorKey ? priorKeyIndex : 0;
let result;
for (let i = start; i < sortedKeys.length; i += 1) {
const key = sortedKeys[i];
if (key > priorKey) {
priorKeyReturned = key;
priorKeyIndex = i;
result = key;
break;
}
}
if (!result) clearGetNextKeyCache();
return result;
},
vatstoreSet(key, value) {
if (!vatstore.has(key)) clearSorted();
vatstore.set(key, value);
},
vatstoreDelete(key) {
if (vatstore.has(key)) clearSorted();
vatstore.delete(key);
},
};
return { syscall, settlements };
})();
const forceGC = () => forceCollectionP.then(fn => fn());
const gcTools = {
WeakRef,
FinalizationRegistry,
waitUntilQuiescent: forceGC,
gcAndFinalize: forceGC,
meterControl: makeDummyMeterControl(),
};
const vatPowers = {};
const buildVatNamespace = (_vatGlobals, { WeakMap: VirtualWeakMap }) => ({
buildRootObject: ({ VatData }, vatParameters, baggage) => {
globalThis.VatData = globalThis.realVatData = VatData;
const badPassStyleOf = makePassStyleOf(VirtualWeakMap);
const zone = makeDurableZone(baggage);
const makeRepresentative = zone.exoClass(
"Dummy",
undefined,
() => ({}),
{},
);
const { psoCacheType } = vatParameters;
const passStyleOfChoices = new Map();
passStyleOfChoices.set("real", goodPassStyleOf);
passStyleOfChoices.set("virtual", badPassStyleOf);
const passStyleOf = passStyleOfChoices.get(psoCacheType)
|| Fail`unknown passStyleOf cache type ${psoCacheType} in ${stringify(vatParameters)}`;
print(`passStyleOf uses a ${psoCacheType} WeakMap cache`);
const work = async n => {
let result = [];
for (let i = 0; i < n; i++) result.push(makeRepresentative());
harden(result);
passStyleOf(result);
return result;
};
return Far("root", { work });
},
});
const liveslots = makeLiveSlots(
syscall,
"vat-alice",
vatPowers,
{ virtualObjectCacheSize: 0 },
gcTools,
console,
buildVatNamespace,
);
const { dispatch } = liveslots;
// "real" or "virtual" here:
const psoCacheType = "real";
const vatStartedP = dispatch(makeStartVat(kser({ psoCacheType })));
let lastID = 1000;
Object.assign(globalThis, {
dispatch,
vatRoot: "o+0",
kslot,
kser,
kunser,
makeStartVat,
makeMessage,
makeBringOutYourDead,
settlements,
vatStartedP,
lastID,
});
' \
-s 'const forceGC = await forceCollectionP; await vatStartedP;' \
work-and-gc:'{
// The code to measure: work+BOYD+forceGC
// Reserve a new result VPID.
const vpid = `p-${++lastID}`;
// Send the request for N new representatives.
await dispatch(makeMessage(vatRoot, "work", [N], vpid));
const { isRejection, capdata } = settlements.get(vpid);
if (isRejection) throw Error(`unexpected rejection for ${vpid}: ${capdata}`);
await dispatch(makeBringOutYourDead());
await forceGC();
}'
|
…tment (#9874) A variant of #9431 . @warner , feel free to just adopt these changes into #9431 rather than reviewing this alternate. closes: #9781 refs: #9431 endojs/endo#2377 endojs/endo#2408 ## Description The code running under liveslots, i.e., user-level vat code such as contracts, must not be able to sense gc. Thus, liveslots endows them with virtual-storage-aware WeakMap and WeakSet, which treats the virtual object as the weakly held key, whereas the builtin WeakMap and WeakSet would treat the momentary representative as the weakly held key. To achieve this, the virtual-storage-aware WeakMap and WeakSet must impose a comparative storage leak. However, some WeakMaps and WeakSets are used purely as an encapsulated unobservable memo, in the sense that the clients of encapsulating abstraction cannot sense whether the memo hit or missed (modulo timing of course, which we can also deny). `passStyleOf` is such an abstraction. Measurements show that the storage leak it causes is significant. The judgement of `passStyleOf` is only to report the pass-style of its arguments, and all virtual objects that have representative have a pass-style of `'remotable'` every time any of its representatives are tested. To avoid this storage leak, endojs/endo#2377 (merged, released, and synced with agoric-sdk) and endojs/endo#2408 (still in review) together enable liveslots to endow the compartment it unbundles with its own efficient `passStyleOf`, built from the primitive WeakMap which it encapsulates. This PR does two things: - makes the change to liveslots to do this endowing, according to the conventions supported by endojs/endo#2377 and endojs/endo#2408 - because endojs/endo#2408 is not yet synced with agoric-sdk, this PR adds an "equivalent" patch, so that we can depend on it before the next endo sync. ### Security Considerations This design *assumes* that the endowed `passStyleOf` makes the memo hits vs misses unobservable, so its dependence on these does not enable the code using it to observe gc. If there is some way to trick it into exposing the difference between a hit and miss, that would be a security concern. ### Scaling Considerations The point. With this PR, the storage leak caused by the `passStyleOf` memo should go away. For some vats, this should be a big improvement. ### Documentation Considerations For the normal developer, none. ### Testing Considerations Adapts the tests originally written by @warner in #9431 , which seem to demonstrate that this works both for node-based and for XS-based vats. ### Upgrade Considerations I don't believe there are any. When linked with an endo preceding even endojs/endo#2377 , the only consequence should be that the storage leak remains unfixed. Likewise, if an endo with endojs/endo#2377 and even endojs/endo#2408 is linked with an agoric-sdk prior to the PR, the only consequence should be that the storage leak remains unfixed.
A variant of #9431 . @warner , feel free to just adopt these changes into #9431 rather than reviewing this alternate.
closes: #9781
refs: #9431 endojs/endo#2377 endojs/endo#2408
Description
The code running under liveslots, i.e., user-level vat code such as contracts, must not be able to sense gc. Thus, liveslots endows them with virtual-storage-aware WeakMap and WeakSet, which treats the virtual object as the weakly held key, whereas the builtin WeakMap and WeakSet would treat the momentary representative as the weakly held key. To achieve this, the virtual-storage-aware WeakMap and WeakSet must impose a comparative storage leak.
However, some WeakMaps and WeakSets are used purely as an encapsulated unobservable memo, in the sense that the clients of encapsulating abstraction cannot sense whether the memo hit or missed (modulo timing of course, which we can also deny).
passStyleOf
is such an abstraction. Measurements show that the storage leak it causes is significant. The judgement ofpassStyleOf
is only to report the pass-style of its arguments, and all virtual objects that have representative have a pass-style of'remotable'
every time any of its representatives are tested.To avoid this storage leak, endojs/endo#2377 (merged, released, and synced with agoric-sdk) and endojs/endo#2408 (still in review) together enable liveslots to endow the compartment it unbundles with its own efficient
passStyleOf
, built from the primitive WeakMap which it encapsulates.This PR does two things:
Security Considerations
This design assumes that the endowed
passStyleOf
makes the memo hits vs misses unobservable, so its dependence on these does not enable the code using it to observe gc. If there is some way to trick it into exposing the difference between a hit and miss, that would be a security concern.Scaling Considerations
The point.
With this PR, the storage leak caused by the
passStyleOf
memo should go away. For some vats, this should be a big improvement.Documentation Considerations
For the normal developer, none.
Testing Considerations
Adapts the tests originally written by @warner in #9431 , which seem to demonstrate that this works both for node-based and for XS-based vats.
Upgrade Considerations
I don't believe there are any. When linked with an endo preceding even endojs/endo#2377 , the only consequence should be that the storage leak remains unfixed. Likewise, if an endo with endojs/endo#2377 and even endojs/endo#2408 is linked with an agoric-sdk prior to the PR, the only consequence should be that the storage leak remains unfixed.