Skip to content

Conversation

@toliaqat
Copy link
Contributor

Reducing maxVatsOnline to only 5 online vats to reduce the load on the system that is running this container.

Testing:
Tested locally to see if this SwingSet change works without a3p. Now, with PR, trying to build an image in the CI to test.

@mhofman
Copy link
Member

mhofman commented May 24, 2024

I suspect 5 is a much too aggressive number, and would result in a lot of paging in and out of vats, which is expensive. What is the problem being solved? What does "load" mean in this case? Unused but loaded vats would at most consume memory, is that the concern?

warehousePolicy,
}) {
- const { maxVatsOnline = 50, restartWorkerOnSnapshot = true } =
+ const { maxVatsOnline = 5, restartWorkerOnSnapshot = true } =
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't know whether this PR is a good idea, but if it goes forward I would suggest setting the option instead of changing the default.

I.e. adding a warehousePolicy to the runtimeOptions param here: https://github.com/Agoric/agoric-sdk/blob/50f94936e038d6b8872dfde77b5f54811f339ccd/packages/cosmic-swingset/src/launch-chain.js#L221-L233

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You are right. I wanted to experiment without making a change in agoric-sdk to verify that it works, and then make the change in agoric-sdk so that we don't need a patch here.

@toliaqat
Copy link
Contributor Author

I suspect 5 is a much too aggressive number, and would result in a lot of paging in and out of vats, which is expensive. What is the problem being solved? What does "load" mean in this case? Unused but loaded vats would at most consume memory, is that the concern?

@mhofman I have created this issue #159
I will do some experiments with different numbers to see what is the best number here before removing WIP from the PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants