Excessive memory usage when using `session_restore_mode = { lines = 30000 }`

[someplaceguy]

What happened

When I use shpool with an empty config file, it works fine. However, if I add the following line to the config file:

session_restore_mode = { lines = 30000 }

... then memory usage skyrockets as lines are printed out within a session, i.e. shpool quickly starts to multiple GBs of memory and becomes really slow.

What I expected to happen

I expected memory usage to remain at a reasonable level.

To Reproduce

1. Add session_restore_mode = { lines = 30000 } to the shpool config file
2. Restart the shpool user service
3. Run shpool attach -f test
4. Run yes inside the shpool session
5. Observe how within seconds, the memory usage of the shpool daemon increases to more than 10 GBs

Version info

shpool 0.6.2

[ethanpailes]

Yeah this is a known issue resulting from a workaround we have for a bug in how the vt100 crate represents terminal state. https://github.com/shell-pool/shpool/issues/46 has some more details, but essentially the problem is that the vt100 crate stores a grid of cells rather than an array of lines that it projects into a grid space. This means that when you re-size a vt100 terminal to have a smaller width, any data on the right side of the screen is perminantly lost. To deal with this bug, we just make each line long enough that it can hold a ton of data and never resize the terminal down, but unfortunately this explodes the memory usage. Fixing this is one of the main goals of the replacement crate for vt100 that @Aetf is working on.

I've been planning to do some memory profiling with valgrind for a while now to see if there are any other big sources of memory usage, but I'm pretty sure this is the main one.

In the short term, we should probably do a few things:

1. Tune the default line width down. I set it quite high because I was thinking about the usecase where someone dumps a giant json blob all on one line and it takes up the whole screen, but this is an edge case and probably not worth it given the memory overhead.
2. Add a configuration option to allow users to explicitly set the vt100 line width so they can tune their memory overhead further.
3. Add a section to the troubleshooting wiki page explaining this stuff

Unfortunately, the real fix, which is replacing our vt100 fork with something better, is a longer term effort. Until we do that shpool doesn't really live up to its goal of being "lightweight".

[ethanpailes]

I just posted https://github.com/shell-pool/shpool/pull/73 to address (1) and (2). I'll hold off on (3) until that change gets released since it would be confusing to have the wiki mention a config option that only exists at HEAD.

[ethanpailes]

Oh, I should also probably mention that completely disabling session restore mode with session_restore_mode = "simple" should remove the memory impact since we won't create the in-memory terminal in that case.