ebkalderon / tower-lsp

Language Server Protocol implementation written in Rust
Apache License 2.0
962 stars 54 forks source link

Background task #365

Closed dalance closed 1 year ago

dalance commented 1 year ago

I want to implement background task like symbol table construction of the entire project directory. If I call the task in callback (ex. did_changed), the next callback can't be processed until finishing the task. Using spawn may resolve the problem, but communication between threads is not easy. So I prefer single thread. Is there a proper way?

I think the following interface may be useful, for example.

#[tower_lsp::async_trait]
impl LanguageServer for Backend {
    async fn background(&self) -> Result<()> { // `background` is called at the time of no request.
        // small task which doesn't degrade response
    }
}
ratmice commented 1 year ago

I'm not sure spawn/communication between threads is really that bad, if nothing else i'd posted an example of that in #340 as I hadn't had any luck in using the tokio's LocalSet, to get thread local futures working, one thing that really helps is that Client is clone. Perhaps there is a better way, but it does work at a last resort

ebkalderon commented 1 year ago

Perhaps the symbol table data structure could be wrapped in an Arc<tokio::sync::RwLock<_>>, and this could be used to synchronize the background task?

async fn background(&self) -> Result<()> {
    // Wait for background task to complete, if any. If none, this
    // should resolve immediately.
    let _ = self.symbol_table.read().await;

    // Spawn background task...
    let mut symbol_table = self.symbol_table.clone();
    tokio::spawn(async move {
        let mut table = symbol_table.write().await;
        // Update `table` here. Next call to `background()` will only be
        // allowed to execute until after this guard is dropped.
    });

    // ...
}

FYI @ratmice, I hope to relax the Send bound requirements on LanguageServer futures eventually as part of https://github.com/ebkalderon/tower-lsp/issues/284#issuecomment-1385611695, thereby supporting thread-local futures out of the box. The Client type will remain Send, of course, in case anyone wants to pass it into a Send task.

dalance commented 1 year ago

Thank you for your suggestion. I'll try to use background thread.

ebkalderon commented 1 year ago

Sure thing, @dalance! Feel free to post back with the results and if the behavior matches what you expect, and feel free to reopen the ticket if it doesn't resolve the issue. We can always iterate on ways to improve the experience going forward.