metals-feature-requests
metals-feature-requests copied to clipboard
Terminate Bloop after all workspaces are closed
Describe the bug The bloop server is not terminated after visual studio code is closed.
To Reproduce Steps to reproduce the behavior:
- open a Scala sbt project with metals plugin in visual studio code
- close the editor
- type
ps aux | grep javaand you shall see bloop
Expected behavior after the editor is closed all the related processes launched by the plugin should also be terminated.
Screenshots
The following line is shown after typing the command above:
/usr/bin/java -Xss4m -XX:MaxInlineLevel=20 -XX:+UseParallelGC -classpath (omitted ..) bloop.Server
Installation:
- Operating system: macOS/Linux
- Editor: Visual Studio Code
- Metals version: v0.9.0
Additional context If that was the original intent to keep the server alive, at least there should be an option to to disable it.
Search terms Bloop Metals Visual Studio Code Plugin
Hi, thanks for reporting! This is currently expected. A single Bloop instance will work with all of the metals instances and CLI. Because of that we can't turn off server while it can be running for another workspace. Also, the current philosophy of Bloop is to always keep a server running so that the compiler is JITed and can work faster.
We were previously discussing some timeout after which Bloop would turn off, but that would be a long timeout by default. Alternatively, we could introduce another setting that would turn off Bloop if there are no other VS Code instances running (this would require some kind of a file lock), but that wouldn't be done by default since some users might still use the CLI to run compilation.
Bloop tends to take up all the memory on the machine if you leave it open for too long, so I think something like the setting you mentioned (that would turn off Bloop if there are no other VS Code instances running) would be nice to have. I find myself shutting it down periodically for that reason at least daily.
@smarter pointed out that it might be super confusing for users (especially the ones starting with Scala) if we keep Bloop open between sessions. Additionally, the warning about an older Bloop version might pop up more in that case.
I wonder if we should have Bloop default and advanced modes. Advanced would work the same way as currently, but the default would:
- when starting create a file lock in ~/.metals and then create an additional PID file representing the server
- when closing the server would also lock the file, check if no other PIDs are registered and if none kill the Bloop server
Drawback is that users that want to have the current behavior would need to specify it and it might not be obvious what is happening.
btw. @kubukoz I don't know if I ever mentioned it, but I suggest adding -Xmx settings to ~/.bloop/.jvmopts - 2G should be enough for Bloop overall, otherwise it will just keep being lazy about GC and continue to raise the memory usage.
For ~/.bloop/.jvmopts it probably works but on bloop website they recommend use ~/.bloop/bloop.json
https://scalacenter.github.io/bloop/docs/server-reference#start-the-build-server
cat ~/.bloop/bloop.json
{
"javaOptions": [
"-Xmx1G",
"-Xms256M"
]
}
And then you can see ps aux:
java -Xss4m -XX:MaxInlineLevel=20 -XX:+UseParallelGC -Xmx1G -Xms256M .... bloop.Server 8212
When @tgodzik told me about it it was like a lifesaver, it was this or restarting bloop every day ...
About this Terminate bloop wouldn't it be nice if it was actually part of a bloop? A setting that when set would tell bloop to shutdown itself if no clients are connected over some period of time?
About this
Terminate bloopwouldn't it be nice if it was actually part of a bloop? A setting that when set would tell bloop to shutdown itself if no clients are connected over some period of time?
We've discussed it with @jvican and it would need to be much longer amount of time (something like 24 h) for it to be a default behaviour, which will not be super useful here. I think it's safer do do in Metals as it's often the case that Metals starts Bloop and users even don't know about it.
I wonder if we should have Bloop default and advanced modes. Advanced would work the same way as currently, but the default would:
* when starting create a file lock in ~/.metals and then create an additional PID file representing the server * when closing the server would also lock the file, check if no other PIDs are registered and if none kill the Bloop server
Instead of having a setting, would it be possible to instead sort of follow the rule where if Metals started the server, then on exit it would shutdown the server. However, if the server was already running, then it won't shut it down on exit. That way it avoids a setting, still allows for more advanced usage if people want to keep it running, and should solve the problem that was mentioned?
Would it make sense to have a "keepalive" functionality in Bloop that would make it shut down by itself if no clients connected for N minutes?
Ideally this would be an optional flag, that we can set when launching Bloop from Metals. Users who launch Bloop manually won't be affected.
Would it make sense to have a "keepalive" functionality in Bloop that would make it shut down by itself if no clients connected for N minutes?
Ideally this would be an optional flag, that we can set when launching Bloop from Metals. Users who launch Bloop manually won't be affected.
That might work, though we would first gather some feedback from @jvican I will add an issue in Bloop to discuss.
What's the point of adding that feature to bloop if Metals will be using sbt as the default build server for the vast majority of users soon?
I'm not a fan of this feature, bloop is a persistent server and it's not meant to be shut down after X time. I also don't like the fact that Metals would be changing the default keep alive value, as that will produce very inconsistent results with the default, recommended approach. I can consider merging it if it's well implemented and the default value is the same in both Metals and bloop (e.g. Metals doesn't have an option to tweak the keep alive).
@jvican no it won't default to sbt, see https://github.com/scalameta/metals/issues/2049 TL;DR By default we will use bloop as usual until we do more testing and add smooth integration.
I don't really think we would be switching to sbt any time soon as default. It is nice to have the option to use different build tools, but I am pretty confident we would rather promote the approach Bloop is presenting. Although, for sure if someone wants to use sbt we would like it to be easy to do so.
My point is, bloop is a transitional technology so I prefer to refrain from changing the behavior it currently has unless it's a force majeure.
In my opinion, sbt server is a much better place to make these changes, it's the future after all! Any performance changes in bloop as those mentioned by @kubukoz will be tracked down and fixed, I believe the reason for this cpu consumption is the zipkin threads.
@tgodzik wrote my comment while you were writing yours. Let's sync up offline to make sure we're on the same page!
In my opinion, sbt server is a much better place to make these changes, it's the future after all!
@jvican Sure, although I would ask you to refrain from being sarcastic as it is not constructive to the discussion and can mislead the readers. It is not accepted it from any other people, so especially not from contributors.
If your comment wasn't intended to be sarcastic then I am sorry I misread it.
I was not sarcastic, I'm serious.
My point is, bloop is a transitional technology
Could you expand on what you mean by this? Is this a general statement or referring to the use of Bloop in Metals?
In my opinion, sbt server is a much better place to make these changes, it's the future after all!
I was not sarcastic, I'm serious.
Could you also explain what you mean by this?
Could you expand on what you mean by this? Is this a general statement or referring to the use of Bloop in Metals?
This is specific to the use of Bloop that Metals has.
Could you also explain what you mean by this?
My quote was meant to be interpreted literally: "sbt server is a much better place to make these changes, it's the future after all!".
There has been a bunch of work behind sbt to make its build server work well, IntelliJ has put a lot of effort in integrating with it and the server has improved build speed and ergonomics. I want to encourage Metals to use sbt instead of bloop because I don't want to invest time to replicate or improve the work that has been done on the sbt side (even if sbt-bloop works pretty great now).
Bloop has been a great replacement as a high-quality build server for sbt, but now that there's one native build server I don't see the point of continuing using it, and hence my strong recommendation to invest in the sbt future. Bloop will still be used for gradle or maven, but given that only a minority uses those build tools, it's less important that we adapt Bloop only for Metals.
So we've talked with @jvican and the arguments against auto shutdown are:
- adding an additional setting that would dictate when the server should shut down will create quite a different experiences for users and it is not something that should be done inside Bloop. It's core idea is based on the fact that it runs continually like for example gradle deamon, which should not be problematic for most users.
- there is no sensible way of knowing if there is an another client that might want to use Bloop and any timeouts will be arbitrary (except something really large, which doesn't help out here)
- Bloop is no longer just used for Metals and we should keep Metals specific features inside Metals
Some action points we can follow up:
- Add an option for people to auto shutdown Bloop from Metals, which might be useful if someone is only using one workspace, but could provide a bad experience in some cases. Good thing is that even if user opened other workspaces, they will autoconnect and restart Bloop themselves.
- Add a proper explanation how does Bloop with Metals work (documentation). We could even post an information message that would auto hide or if that would create a clutter, at least add the information to the output.
- Add a command to just stop Bloop from Metals, not only restart.
- Improve the Bloop restart message, so that it doesn't show up only if needed. Especially, it will only create confusions in case that we just merged Bloop version with some bugfixes.
I want to encourage Metals to use sbt instead of bloop because I don't want to invest time to replicate or improve the work that has been done on the sbt side (even if sbt-bloop works pretty great now).
I think it's going to take a long time until the the sbt BSP server is good enough. For example right now if I run console in sbt, metals won't be able to display any compilation error message until I exit the console because the server won't respond to them.
- Add a command to just stop Bloop from Metals, not only restart.
This may already be sufficient IMO, probably in combination with documentation.
If metals does explicitly start the Bloop server it would make sense for it to kill the process at shutdown. If it was already running on beforehand the opposite would be expected by the casual user.
I can imagine the described use cases for people running multiple Metals instances and wanting to share the Bloop process. Since starting Bloop upfront should be doable for such more advanced users killing Bloop by default sounds like the most logical behavior me. Adding an option to leave it hanging for those users would be nice to have I think.
I agree with some of the other people comments on this thread. If metals does explicitly start the Bloop server it would make sense for it to kill the process at shutdown. If this was a application and it didn't clean-up resources it explicitly started, it would be called a memory leak.
I just started developing with scala and vscode and was shocked to see bloop.server running days after I shut down vscode.
Can we simply kill it after a period of inactivity, provided no clients are connected? (that is, if Bloop tracks its clients at all)
That's possible for sure, but I just haven't had the time to take a look. We could keep a variable in bloop and change it every time a request is sent. A scheduled thread would check if the last modified is within some time and if it's not then shutdown the process
I'm curious what people's workarounds to this behavior is. I've noticed that "killall bloop.server" doesn't work due to the huge classpath. So far I do a 'ps -Af | grep bloop', find the pid, copy it , and paste it into a kill command. Is there a quicker way that is reliable and won't accidentally kill another process?
Since this issue has been known for four years and hasn't been addressed, I'm assuming the community doesn't care too much about it. Do most people just leave bloop.server running all the time?
I leave mine on, but I also almost never don't need one. When I want to close it, I use jps -l and kill by pid.
I think most people do not notice, so this is certainly a valid feature request, just nobody had the time to work on it.