Skip to content
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

evicting local vats could be expensive #3413

Open
dckc opened this issue Jun 25, 2021 · 3 comments
Open

evicting local vats could be expensive #3413

dckc opened this issue Jun 25, 2021 · 3 comments
Assignees
Labels
performance Performance related issues SwingSet package: SwingSet

Comments

@dckc
Copy link
Member

dckc commented Jun 25, 2021

Describe the bug

The LRU policy from #2848 applies to all vats, including local vats. Paging a local vat out and in is likely to be expensive and should be avoided.

As @warner notes, our defaults are probably generous enough to avoid evicting any vat, so it's unlikely to be observable any time soon.

To Reproduce

Steps to reproduce the behavior:

  1. reduce maxVatsOnline to something like 7
  2. run agoric start
  3. run ps ax | grep xsnap
  4. see that there are fewer than 7 XS vats running, because some got evicted to make way for local vats

Expected behavior

local vats don't get considered for swapping out (eviction).

@dckc dckc added bug Something isn't working SwingSet package: SwingSet performance Performance related issues xsnap the XS execution tool labels Jun 25, 2021
@dckc dckc self-assigned this Jun 25, 2021
@dckc dckc removed the bug Something isn't working label Sep 8, 2021
@dckc
Copy link
Member Author

dckc commented Sep 8, 2021

This is a performance issue but not correctness, IIUC.

@warner
Copy link
Member

warner commented Feb 9, 2022

Another reason to deprioritize this is that we only have one local vat (comms), and it doesn't use a transcript anyways, so loading it back in is pretty cheap.

OTOH I don't think we've ever experienced it being evicted, so I'm not even sure we can have confidence that it would get paged back in correctly. But @dckc 's STR should exercise that pretty easily. So one task here is to just reproduce that and make sure comms still works afterwards. If that's true, we can probably put this off until we enable the creation of more vats (MN-2?)

@dckc
Copy link
Member Author

dckc commented Feb 9, 2022

we only have one local vat (comms), and it doesn't use a transcript

Nice! That addresses this issue, to my satisfaction.

I don't have any reason to believe that comms doesn't survive being swapped out, but if it does not, that seems like a separate issue (though I suppose morphing this one into it works for me).

@aj-agoric aj-agoric removed the xsnap the XS execution tool label Sep 24, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
performance Performance related issues SwingSet package: SwingSet
Projects
None yet
Development

No branches or pull requests

4 participants