This PR aims to remove the need for artificial CURSOR identifier which was appended after current cursor position:
Lis@@
Would result in code:
Lis@@CURSOR
This lead to super inefficient IDE experience in terms of number of compilations done after each change, but let's be a bit more specific. Let's imagine that we have 2 scenarios in which IDE events arrive to metals:
Scenario B: Semantic Highlight (No CURSOR compilation) -> Completion (CURSOR compilation) -> Inlay Hints (No CURSOR compilation)
On top of that, we've implemented a compilation caching, where code snippet and compiler configuration is a key. Now you should notice the issue, that adding a CURSOR into a code has different compilation result with cache invalidations.
In theory, we could handle CURSOR compilation as normal ones, but in reality it is a completely different run with different result (for example in diagnostics, as each one will contain CURSOR in the message). This is a no-go, especially if we would want to have diagnostics coming from presentation compiler in the future.
Because of that, each keypress results in at least 2 compilation and in the worst case scenario in 3. This also make metals way more battery heavy.
This PR is an attempt to drop CURSOR insertion for most cases.
A bit of history, how we ended up with CURSOR in a first place.
Most of the reasons are caused by parser and its recovery.
For example, finding a proper scope in this snippet:
def outer: Int =
def inner: Int =
val innerVal = 1
@@ // completion triggered here
We have to find the correct scope in which we are (inner vs outer). We can achieve this in multiple ways, for example, count indents. This solution may not be so straightforward, as there can be different indentations etc. Inserting a synthetic CURSOR into this place:
def outer: Int =
def inner: Int =
val innerVal = 1
@@CURSOR // completion triggered here
Will actually parse into an identifier with scope provided to us by Scala compiler. This is way easier and will always be correct.
Second example are keywords, let's say we have the following snippet:
var value = 0
val newValue = 1
value = new@@
This code will expect a type, as the parser found a new keyword. Adding a CURSOR here resolves the problem, as now we're dealing with newCURSOR, not new keyword (identifier vs keyword).
This PR is basically a change, which disables adding a CURSOR in all cases but 2 mentioned above. Those cases are very, very, very rare and is something that we can deal with. With this change, each compilation will now be cached and reused as intended resulting in way longer battery life, performance, response times and will enable us to access diagnostics for free without risking recompilation.
TODO:
[x] - remove caching for snippets with CURSOR,
[x] - add tests to verify it.
I'd also love to have this backported to LTS, as it is a significant performance tweak and will allow me to add diagnostics on the fly for the Scastie.
This PR aims to remove the need for artificial
CURSORidentifier which was appended after current cursor position:Would result in code:
This lead to super inefficient IDE experience in terms of number of compilations done after each change, but let's be a bit more specific. Let's imagine that we have 2 scenarios in which IDE events arrive to metals:
On top of that, we've implemented a compilation caching, where code snippet and compiler configuration is a key. Now you should notice the issue, that adding a CURSOR into a code has different compilation result with cache invalidations.
In theory, we could handle CURSOR compilation as normal ones, but in reality it is a completely different run with different result (for example in diagnostics, as each one will contain CURSOR in the message). This is a no-go, especially if we would want to have diagnostics coming from presentation compiler in the future.
Because of that, each keypress results in at least 2 compilation and in the worst case scenario in 3. This also make metals way more battery heavy.
This PR is an attempt to drop CURSOR insertion for most cases.
A bit of history, how we ended up with CURSOR in a first place. Most of the reasons are caused by parser and its recovery. For example, finding a proper scope in this snippet:
We have to find the correct scope in which we are (inner vs outer). We can achieve this in multiple ways, for example, count indents. This solution may not be so straightforward, as there can be different indentations etc. Inserting a synthetic
CURSORinto this place:Will actually parse into an identifier with scope provided to us by Scala compiler. This is way easier and will always be correct.
Second example are keywords, let's say we have the following snippet:
This code will expect a type, as the parser found a new keyword. Adding a
CURSORhere resolves the problem, as now we're dealing withnewCURSOR, notnewkeyword (identifier vs keyword).This PR is basically a change, which disables adding a CURSOR in all cases but 2 mentioned above. Those cases are very, very, very rare and is something that we can deal with. With this change, each compilation will now be cached and reused as intended resulting in way longer battery life, performance, response times and will enable us to access diagnostics for free without risking recompilation.
TODO:
I'd also love to have this backported to LTS, as it is a significant performance tweak and will allow me to add diagnostics on the fly for the Scastie.
[test_windows_full]