feat: demand-paged vats in solo, chain #2848
Conversation
dckc
force-pushed
the
2273-snapstore
branch
6 times, most recently
from
c157c43 to
18e7e44
Compare
dckc
force-pushed
the
2273-snapstore
branch
4 times, most recently
from
f067780 to
6c03776
Compare
dckc
force-pushed
the
2273-snapstore
branch
6 times, most recently
from
f791bdf to
6be1e96
Compare
dckc
changed the title
|
@warner and co this is probably not ready to land, but I do want review. I updated the description. |
dckc
changed the title
Not only is this option not implemented now, but CM's analysis shows that adding it would likely be harmful.
- refactor: move snapshot decision up from vk.saveSnapshot() up
to vw.maybeSaveSnapshot
integration testing shows this is closer to ideal
oops. rebase problem.
rebase / merge problem?!
With snapshotInitial at 2, there is little reason to snapshot after
loading the supervisor bundles. The code doesn't carry its own weight.
Plus, it seems to introduce a strange bug with marshal or something...
```
test/test-home.js:37
36: const { board } = E.get(home);
37: await t.throwsAsync(
38: () => E(board).getValue('148'),
getting a value for a fake id throws
Returned promise rejected with unexpected exception:
Error {
message: 'Remotable (a string) is already frozen',
}
```
In an attempt to avoid reading the lastSnapshot DB key if the t.endPosition key was enough information to decide to take a snapshot, the vatWarehouse was peeking into the vatKeeper's business. Let's go with code clarity over (un-measured) performance.
|
I think I tracked down this CI failure: to the fact that two different tests are using the same "temporary" directory name ( I'm changing those Another small cleanup to add later: the tests do both |
The "temporary" snapstore directories used by two different tests began to overlap when the tests were moved into the same parent dir, and one test was deleting the directory while the other was still using it (as well as mingling files at runtime), causing an xsnap process to die with an IO error if the test were run in parallel. This changes the the two tests to use distinct directories. In the long run, we should either have them use `mktmp` to build a randomly-named known-unique directory, or establish a convention where tempdir names match the name of the test file and case using them, to avoid collisions as we add more tests.
|
Ooh. Nice catch. Will do move to real random tmp dirs. I started at some point but it got lost in the zillions of rebases this PR has been through. |
fixes #2273
Postponed
p.s. on Performance
unloaded chain startup time with this landed is 60x faster than yesterday's non-snapshot build: it went from 5m11s to 4.76s
Historical Note
This has been in the making since Aug 2019: #47. See MadMode: Toward secure distributed computing with Agoric at SFBW '19
See also Chat with Dean Tribble & Dan Connolly in Agoric Community Call #8 - YouTube.