warner
commented
1 year ago I looked into a few of these, here are some preliminary notes.
The first one ("XS: fuzzilli 38", vat v24, tnum=1547) is a makeWantMintedInvitation to o+19/1, in which the "expected" behavior has syscalls which allocate a new InvitationHandle (instance of durable Kind 12), but the "got" is a state fetch for o+19/1 (the target object). This indicates that liveslots had no Representative or state for the target of the message (I guess the GC variance caused the Representative to get collected early), and when makeWantMintedInvitation needs the state, it had to be paged in.
@FUDCo and I considered this during design/implementation, but it looks like we missed something. Our intention was that the Representative can be built without doing a vatstore read (the Kind ID is parsed out of the vref), so simply delivering a message to a vobj shouldn't trigger a syscall (or divergence if the Rep is/is-not currently present in the slotToVal table's WeakRef). The state object is built at the same time, but we don't need the actual data until (and unless) the method being invoked tries to do a property lookup, so simple behavior-less marker objects wouldn't need a syscall either. Only when the setter/getter fires, that's when we fetch from the vatstore. And we made it to perform that fetch unconditionally (even if the data is available in our LRU cache), specifically to make the syscalls a deterministic function of userspace behavior (knowing that the reads were redundant: the cache doesn't help).
Hm, I wonder if the write-back cache is involved: if userspace does:
- 1: read
- 2: write
- .. time passes, GC might happen
- 3: read
then the 3-read must use the dirty data from the cache, for correctness, but we thought we were doing an unconditional read from the vatstore, for determinism. I don't remember considering the conflict between those two, which means I'm not confident that it does the right thing.
(@FUDCo and I have talked about removing that LRU cache entirely, or making it write-through, in #6693, partly because of how it confuses issues like this)
The last one ("XS profile ID by function definition", vat v6, tnum=12258) is a BringOutYourDead, which (in the transcripts provided) starts with a pair of vatstoreSet calls that flush the dirty contents of the virtual-object LRU cache.
Basically the vatstoreSet of a vom.o+NN/NN key is a LRU cache write, the vatstoreSet of a vom.dking.NN key is incrementing the nextInstanceID counter because a new virtual/durable object has just been allocated, and the vatstoreGet of a vom.rc.o+NN/NN key is checking the virtual reference counts for an object that has presumably just lost its RAM pillar and GC is deciding whether to delete it entirely.
I concur that we have more sensitivity to GC order than we first thought (i.e. #6360 and #6760 do not form an exhaustive list).
mhofman
Describe the bug
While validating an upgrade to Moddable SDK 3.6.0 from the fork we've been running, I attempted to validate that an upgraded XS version could still execute transcripts of mainnet vats.
Liveslots is supposed to hide any gc effect from the user code, and hide organic gc from the kernel (as captured by transcripts), only revealing the result of a full gc in a deterministic manner when explicitly requested by bringOutYourDead. As such if the JavaScript behavior of the user program is the same, the transcript should be the same.
While we're aware of and are able to work around a gc sensitivity in virtual collections, something is causing other execution differences.
I've narrowed down the triggers of those differences to 5 commits in the Moddable SDK, which have all been confirmed to have an impact on allocations.
To Reproduce
release-pismowith the Moddable SDK updated to 3.6.0 and select reverts on top to make all transcripts run as they initially did.moddablesubmodule to trigger the corresponding error. Please note thatyarn buildinpackages/xsnapautomatically restores the moddable submodule commit, unless it's added to the index. Inpackages/xsnap/moddable:git reset --hard mhofman/sync-sdk-keep-behaviorgit revert $commitsha(cd .. && git add moddable && yarn build)/path/to/agoric-sdk/packages/SwingSet/misc-tools/replay-transcript.js --no-forced-reload-from-snapshot transcript-v$i.sstObserved behaviors
(partial results)