-
Notifications
You must be signed in to change notification settings - Fork 337
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add a way to filter in/out modules to prevent metals from triggering their compilation #6782
Comments
Thanks for reporting! I think we could actually change the defaults, currently Metals runs: when anything changes. Instead of doing that, we could only compile for the currently used target and do the full compilation when |
Actually, looking into that we do only compile one build target per file. There is no way to choose (scalameta/metals-feature-requests#13), but it should not cause any issues aside from the full compilation at the start. Are you getting more targets compiled? Do you have any example projects that this is happening at and what is your workflow that triggers this behaviour? |
Hey Tomasz, sure, the main repos I can share are : Edit : oops, I hit "send" by accident. Anyhow, I was saying : I think I'm getting more target compiles, in the sense that I'm seeing compilation information in the bottom right of my screen for targets I'm not interested in. Now that I'm thinking about it, in smithy4s in particular, there is bootstrapped code-generation involved (ie, the code generator is compiled in the project, but also shelled-out to in the build logic of other modules). It might eagerly trigger some I'll try and pay more attention and report |
PS : both projects I shared use https://github.com/sbt/sbt-projectmatrix |
I tried to reproduce your issue, but I think I might need a bit more clarification. When does more than one target get compiled? Usually we just invoke compile on save for a single target. We don't invoke it for every possible target the file belongs to. Could you explain a bit more what happens currently, maybe on an example? (open this file, modify this, I see a lot of targets compiling etc.) |
Hey @tgodzik, sorry for not having replied earlier. I've tried reproducing, and it might be that the compilation isn't invoked on all possible targets, but rather the targets on which they are invoked seems to be inconsistant: ie sometimes a 2.12-specific target will be invoked, sometimes 2.13, sometimes js, etc. I've yet to find a properly reproducible scenario, I'm afraid. It seems to happen after a regeneration of the bloop config, but not 100% sure.
is there anything that dictates what specific target is supposed to be invoked ? Is it deterministic ? |
The code that determines the order of build targets seems to be here: metals/metals/src/main/scala/scala/meta/internal/metals/BuildTargets.scala Lines 66 to 89 in ee7372d
So it seems JVM is preferred over JS and Scala 2 over Scala 3. There can be some undeterminism between Scala 2.12 and Scala 2.13. This would surely happen after restarting Metals or re-importing the build. |
We could for sure change it to make it deterministic and always pick the higher version. Should be a simple change. I will take a look! |
Any chance this could be configured ? I think this would basically address my request. In particular, though I cross-compile to 3, I'd rather 2.13 was default on my projects for the time being (to get linting options that don't have an equivalent, like warn-unused etc), but also, I'd like for JVM to be selected over JS every time, and not sure it's the case for everybody |
Sure, there is actually a feature request for that: scalameta/metals-feature-requests#13 But in your case it seems we would actually use |
Oh right ... regardless, I do see JS targets being compiled for apparently no reason. I thought it may have been a non-determinism thing, because I rarely ever touch JS-specific code :/ |
That's what worries me, I don't currently see a situation where we would compile unrelated build targets especially JS ones. We do sometimes invoke Next time this happens could you try and see what were the previous steps that let up to those compilations being done? Did you look for references, did you invoke a code action etc. I will try to look further into this in the meantime. |
yup, I'll definitely try to capture a reproducible sequence of steps next time I see it happening. When using bloop, is the computation of the cascade handled by metals or bloop ? Could you point me to the code that decides to trigger it ? Understanding how the code works could help me figure out a reproduction. |
It's handled by Metals in
In most casess Bloop will just return a NOP results. |
@tgodzik following from scalameta/metals-feature-requests#332 (comment) ...
I don't think there's necessarily anything wrong. It's just slow, because there are many cross-compiled projects sharing the same sources. Combine this with the sbt bsp, which blocks while it is compiling, and it gets to be rather annoying :) In fact, I'd love to be able to simply import a build into metals without waiting forever. I just tried it now, and for Cats Effect it takes about 10 minutes from triggering the build import to being able to use the sbt console again. Now that I've imported the project, playing around with it actually seems okay :) from the logs it looks like when I work on shared sources, it is only recompiling them for JVM. But if a switch to a source that's only for JS or Native, there is quite a bit of latency while it catches up (?). I'm also getting some messages like this (and others have reported it as well).
Here are some examples of large, cross-compiled projects I frequently work on: |
While writing the above post, we got a bug report in Cats Effect. It's an easy one to fix, I just need to switch branches—but now I'm stuck waiting several minutes for metals, with a blocked sbt console, before I can do anything. In the same time, I can open a new cloud workspace and fix the bug with sbt and no metals 😅 |
This would be great to fix before making sbt the default, but still compilation would be taking a lot of resources here 🤔
We always do cascade compile at the start, we could exclude Native and Js targets either in an option or just make them compile on demand when the only target a file belongs to is Native/Js. What do you think? The only risk here is that when finding references/rename we could lose some that only belong to other targets than JVM. But overall if we had an option to run concurrent tasks, we could queue them and make it all less problematic on the CPU/RAM while also making it possible to use the console.
That's probably expected, since only then we would recompile the sources for them.
I think that's sbt, right? I've seen this issue when crosscompiling a lot, not sure how to fix it 🤔 I just checked and for cats repo it seems to work much faster with Bloop, which makes me think that sbt is surely not ready yet for being the default. @ckipp01 I would love to hear your input, any idea why sbt behaves weirdly here? |
Ok, so Native seems to add a lot of compilation time here I think, so the best bet is to remove it from initial compilation. Also did you try increasing Metals max heap? Might also help in case Metals is having issues in a large codebase. I changed it to 3 GB and that seems more than enough (it was using up to 2.2 GB and down to 1.3 GB normally.) |
Also, I wondering if it is normal that we link everything on compilation:
shouldn't that be done on publish? |
Thanks so much for looking at this!
I mean, that's basically the whole point of this issue 😛 to let the user configure which targets to include/exclude. I don't think we should give JVM special status here, necessarily. But I am very much in favor of exposing some option, that lets the user exclude targets.
True, but I think this is an edge case. The majority of the code in the repos I linked is shared, actually, so doing the rename on one target is sufficient. Indeed, doing most of your work on one target is sufficient. In cases where code is not shared and is platform-specific, it might have to be a pretty major rename (essentially affecting the entire codebase) if it needs to be changed for multiple platforms.
Right, except on the day when I want to fix a bug in Native-specific code :) then I would prefer it compiles that one initially, and I don't care about the other targets at all actually.
Yeah, the GC warning is from sbt. Good point about Metals heap, I will try adjusting that.
Sad 😕 seems to be trade-offs either way. Ok, thanks, good to know.
It's not even needed for |
Well, it is not that simple, actually. I personally find that it is usually way more easy to work with both apps simultaneously: VSCode/Metals and SBT. It just provides more flexibility. I.e. I usually keep a project loaded in both these tools. That means, in particular, that if Metals uses Bloop, then the project has to be compiled twice for each change – one time with Bloop from VSCode and also with SBT itself. Which is substantially less performant comparing to a case when everything is compiled with SBT/BSP only (even though it is slower than Bloop by itself). |
Is your feature request related to a problem? Please describe.
I maintain a number of cross-compiled libraries and metals seem to struggle with them (ie, I need to restart it fairly often, etc), but also, I don't necessarily want to cross-compile against all targets whilst I'm developing, because it uses a lot of resources (even with incremental compilation), and sometimes drags the attention away from what I'm actually trying to do.
Describe the solution you'd like
I'd like to be able to tell metals to filter-out (or filter-in) some modules, based on a sequence of glob-patterns.
I don't want to solve it at the build level, because I don't want to risk checking-in this kind of configuration, nor do I want to trigger a bloop config dump every time I change it. I also like being able to use bloop from the command-line to compile/test things, without necessarily for it to be reflected in the editor.
When editing a file, before triggering compilation via BSP, metals would look at its configuration and inspect the filter to decide whether it should omit some targets.
Describe alternatives you've considered
I've considered playing with the build, but as stated previously, this poses a risk of inadvertently checking-in configuration that is very much workflow specific. It's also not very portable, in the fact that I maintain both mill and sbt projects. Finally, using bloop from CLI is nice, and discarding modules at the build level would disallow their compilation by bloop in terminal.
Additional contex
My workflow when working on these heavily cross-compiled projects is usually :
I'd like to put
*_212*
,*_3*
and*_JS*
in a disallow-list in some configuration somewhere (following the pattern used in my projects) when working on step 1, then replace*_212*
by*_213*
when working on step 2, without having to recompute the bloop configuration, and for metals to immediately understand that it clean diagnostics and recompile against the newly allowed targets.Search terms
filter, config, cross
The text was updated successfully, but these errors were encountered: