Add option for overriding GOMEMLIMIT
#1808
Conversation
|
@maschwenk I think I want to try to get this in, just so we can have people try this and see how it behaves. The current code looks fine, though it requires a merge from main. |
|
@jakebailey on it! |
maschwenk
force-pushed
the
mfs-poc-go-memlimit
branch
from
22c740c to
697ec86
Compare
maschwenk
force-pushed
the
mfs-poc-go-memlimit
branch
from
18be026 to
e0f1bb7
Compare
_extension/src/client.ts
Outdated
| if (goMemLimit) { | ||
| // Keep this regex aligned with the pattern in package.json. | ||
| if (!/^[0-9]+(([KMGT]i)?B)?$/.test(goMemLimit)) { | ||
| this.outputChannel.error(`Invalid goMemLimit: ${goMemLimit}. Must be a valid memory limit (e.g., '2048MiB', '4GiB'). Not overriding GOMEMLIMIT.`); |
There was a problem hiding this comment.
This doesn't unset goMemLimit
There was a problem hiding this comment.
pulled out into a little helper to avoid having to do a bunch of weird nested conditional stuff and then don't need to "unset" anything, just set it once it's validated.
also figure at some point there might other env mutations that need to go on for other Go env vars, and might be easier to reason about if all that env configuration is done in one method
maschwenk
changed the title
maschwenk
changed the title
GOMEMLIMIT
jakebailey
left a comment
jakebailey
left a comment
There was a problem hiding this comment.
This seems to work, though it does not integrate with the restart command because it's resolved before the client is created and then .restart() provide no opportunity to update it.
It may be that we should instead move this code into a user preference in the server itself and then configure it there on the fly.
Unfortunately, in my own testing, I felt like the option probably hurt more than it helped. If I set it to something like 2GiB, then opening the main TS codebase would use 1.4G, when without the option it would use 600MB. Not sure what to make of that.
2 participants
But less silly