Build on change characteristics - when builds are not fast

[alexheretic]

RLS build logic reacts to didChange notifications by building immediately.

This isn't ideal because changes are soon followed by friends & the R in RLS stands for "Really slow compiling".

Let me demonstrate:

Here Rls handles a long compile time by doubling it and showing a meaningless error at half time. (The error you'd get from the first character typed).

Options

• Change to simply debounce on didChange, in a non-eager way, using the wait_to_build duration.
• Interact with rustc via a seperate process so it can be killed & replaced when a new request appears.

Hopefully that makes sense, any other ideas to improve this?

[nrc]

Change to simply debounce on didChange, in a non-eager way, using the wait_to_build duration.

We used to do this, there is a bit of a trade-off here - an initial wait means longer until the RLS has any info (consider a block delete), and longer if there is just a single change. OTOH, the current behaviour is not ideal as you observe. It is pretty easy to wait on the initial change, we should probably just give it a go and see how it feels.

Interact with rustc via a seperate process so it can be killed & replaced when a new request appears.

This is tricky since we need to pass changed (and not saved) files in memory which requires the same process (and we return save-analysis info in memory too). We could consider using some kind of RPC but I suspect that will be complex and might have significant performance overhead (the output especially can be significant and serialisation/deserialisation has been a significant part of profiles before we moved to a direct approach). In a future where we do this incrementally, it is probably worth investigating - being able to cancel build jobs would be really good.

[alexheretic]

We used to do this, there is a bit of a trade-off here - an initial wait means longer until the RLS has any info (consider a block delete), and longer if there is just a single change. OTOH, the current behaviour is not ideal as you observe. It is pretty easy to wait on the initial change, we should probably just give it a go and see how it feels.

Yep the drawback of non-eager debounce is having to wait. This seems to be a problem mostly when the builds are fast and waiting ~500ms becomes a large part of the build time. We could be cleverer here, recording the last build duration and acting differently if it was slow. So if a build takes N * wait_to_build we switch to non-eager behaviour for future builds.

However, I can't help but think that cancelling builds is a capability that is very important in making Rls as responsive as it should be. That we can't actually puts us at a fairly large disadvantage compared to a naive linter calling/killing cargo check. For some users the fact that Rls can work on in-memory changes (which is great) may not be enough of a consolation.

I am aware that cancelling a thread is not something std currently supports. But I think we need a plan for cancelling builds somehow.

• Step 1 (easy): Introduce cancelled build concept - avoid publishing diagnostics from a cancelled build
• Step 2 (hard): Stop rustc building as soon as a build is cancelled.

Actually step 1 would at least avoid the invalidated error above 'expected item, found `/`'.

[nrc]

I pushed a commit which waits for the timeout before doing the initial build and also makes the timeout a bit longer. The added wait time didn't cause much difference to response time but didn't make a lot of difference to CPU utilisation either.

An alternate behaviour which would do less work (and cut out the intermediate results) would be to not build any normal priority builds until the user stops typing (i.e., until we get to a timeout without any new events. That would make it even slower to get help from the compiler, but should be much better for CPU utilisation - which people care about a lot more than I expected - and mean fewer false positive error messages.

[alexheretic]

I'd expect it to behave like regular debounce, start a build Xms after the last build triggering event.

That way we can have a small wait duration (500-1000ms) as it just gets extended as more characters are typed.

[alexheretic]

I'm not sure about the 1500 wait to build default. I think 500 was better, we should just avoid publishing outdated diagnostics.

[Xanewok]

Cancellable build should be implemented with #1307 and @alexheretic implemented a dynamically inferred wait_for_build so this shouldn't be a problem now (feel free to reopen if that doesn't cover everything here).