x/tools/gopls: don't pin all source text in memory #45528
Labels
FrozenDueToAge
gopls/performance
Issues related to gopls performance (CPU, memory, etc).
gopls
Issues related to the Go language server, gopls.
Tools
This label describes issues relating to any tools in the x/tools repository.
Milestone
In investigating recent high memory usage bugs, file content often appears near the top of the profile. One report had 500MB of live heap used by file content.
At first blush, this seems ridiculous: why should we keep (e.g.) the entire source text of the standard library in memory? We already keep the (exported) AST, so why should we need the raw text? At worst we should read it back in when we need it.
The most basic problem is that we can't guarantee that the file we read is in the state we want it to be. If a file changes on disk twice in quick succession, we may get the second contents when we meant to read the first. Depending on what we're doing, that could result in a user facing error or a corruption of the metadata cache.
A less fundamental but still serious problem is that we assume that the file contents are available whenever we need them, for free. Naive changes to load the file on demand will probably cause performance problems. Minimizing the length of time file data is held in memory, while also minimizing the number of times it is read from disk, will require refactoring. I think that refactoring will be fairly invasive.
Off the top of my head, here are the places we might need .go file contents. (I'm deliberately ignoring go.mod files; we work with them very differently, so they're a separate topic. But also they're tiny and not worth worrying about.)
I'm probably missing other places, of course.
Once we have the necessary foundation laid, I can think of some ideas for how to reduce churn: an actual (LRU?) cache for file content, compressing less frequently used source text, etc. But first we need to get file content lifetime under control, the same way I had to get snapshot lifetime under control to deal with growth in the memoize.Cache.
The text was updated successfully, but these errors were encountered: