Optimizing `did_change` Callbacks for Responsive Compilation

[JoshuaBatty]

Problem

Each LSP call to the did_change function triggers compilation via calling forc_pkg::check(). Currently this function takes ~ 1 second to return. If the user is typing then we could reasonably expect the did_change function to get called every 100ms or less.

Ideally, if forc_pkg::check() has not yet completed when a new did_change callback is triggered, we would have a way to instantly stop executing compilation and take advantage of RAII to clean up memory resources. Unfortunately, we do not currently have any way to signal to the compiler that it should stop the compilation process early in order to achieve this.

1. Spawn unique process

In most operating systems, it's possible to instantly terminate a process. We could use the std::process::Command API in Rust's standard library to start a new process, and the std::process::Child::kill method to terminate it. We would then need to use an Inter-process communication (IPC) mechanism to communicate data back to the server process.

Advantages

• Compilation could be terminated immediately in the middle of execution and all resources would be cleaned up.

Potential Issues

• IPC is more complex than inter-thread communication.
• Synchronization and concurrency issues can be more difficult to solve.
• Performance can also be an issue due to the overhead of switching between processes and copying data between them.
• Starting a new process has a higher overhead than starting a new thread.

2. Pass an atomic bool through the compiler indicating it should bail

We can consider passing an atomic boolean flag through the compiler, indicating that it should terminate the compilation process early, providing a means to instantly stop execution and clean up resources.

1. When a did_change callback is triggered, the atomic boolean flag is set to false, indicating that the compilation process should continue normally.
2. The compilation process periodically checks the atomic boolean flag during its execution. If the flag is found to be true, it means an external request to terminate the compilation process has been made.
3. Upon detecting the true value of the atomic boolean flag, the compilation process can immediately stop execution, clean up any resources, and terminate gracefully.
4. The flag can be updated externally by the language server to indicate the desire to stop the ongoing compilation process.

Advantages

• By using an atomic boolean flag, we can provide a means to control the compilation process externally and stop it early if necessary, leveraging the benefits of RAII for resource cleanup.
• Provides the most responsiveness to user typing and uses the least amount of resources.

Potential Issues

• Unsure how to introduce this check in the compiler without degrading the readability of its codebase. Could lead to a lot of “noise”.

3. Each did_change callback spawns a unique thread for compilation

A new thread is created for every call back and compilation is able to be started immediately. This approach improves perceived responsiveness at the cost of wasting system resources.

1. Key stroke event triggers the did_change function which begins compilation via calling forc_pkg::check()
2. This is kicked off on a new thread, when the computation is finished then the results are returned via an mpsc channel and the thread is closed.
3. If a new did_change callback is triggered while the thread is still executing, then a new thread is spawned and starts executing the a new call to forc_pkg::check(). An atomic int is incremented, this variable keeps track of how many threads are running and which thread is the most recent.
4. Step 3 continues as long as new key pressed events arrive while the most recent thread is still executing.
5. Once the final thread has finished it’s computation, the results are sent back via the mpsc channel. All threads are closed and the atomic int is reset to 0.

Advantages

• Since each did_change callback triggers the compilation on a separate thread, the responsiveness can be improved. The main thread handling user input remains available to respond to further events.

Potential Issues

• Each thread consumes system resources, such as stack memory and CPU time. If the number of threads becomes excessive or threads are long-running, it can lead to high resource consumption and potentially impact the overall system performance.
• If the frequency of did_change callbacks is very high, continuously spawning and terminating threads can become expensive and impact performance.
• If there are many threads running in parallel, all threads apart from the most recent one are redundant and their results will be discarded.

4. Use a single thread for compilation

This approach is similar to the previous, yet it optimises away unnecessary computation by keeping the maximum available threads to 1. Our current use of tokio semaphores is giving us similar functionality to this approach.

On each did_change we still write any changes reported in the DidChangeTextDocumentParams type our files in temporary memory. We then kick off compilation. Subsequent calls to did_change while compilation is running sets a should_recompile flag to true. After compilation has finished we check if should_recompile is true, if it is we then recompile, otherwise we set the flag to false.

Advantages

• We only ever have a single thread for compilation. This is the most efficient is terms of system resources.
• Only the first and then most recent change is compiled during rapid typing.

Potential Issues

• Latency is introduced as compilation for the latest change can only begin after the current compilation has finished.

Summary

I think option 4 makes the most sense to implement right away as it matches the same behaviour as what we currently have. It would be great if we could find away for option 2 to work somehow. Would love to hear anyones thoughts. @tritao @IGI-111 @sdankel

[anton-trunov]

Linking here a more general related issue on incremental compilation: https://github.com/FuelLabs/sway/issues/3381

[eureka-cpu]

4 and 2 seem to be the most logical choices imo, though 4 doesn't step on the compiler's toes

[sdankel]

I agree that 4 makes sense for now. In the future we could improve user-perceived responsiveness by doing something in-between options 3 and 4: we could have up to a X number of threads (e.g. 5) processing forc_pkg::check().

I agree that we should stay away from option 1 because it would make concurrency issues much more difficult to solve.

For 2, I think it would only be worth doing if there were another use case besides LSP that would benefit from it, for example a watch mode for the sway compiler.

[JoshuaBatty]

Thanks for the feedback everyone. I think i'll move ahead with option 4 for now. Open to further feedback from anyone else also.