Merge #4133

4133: main: eagerly prime goto-definition caches r=matklad a=BurntSushi

This commit eagerly primes the caches used by goto-definition by
submitting a "phantom" goto-definition request. This is perhaps a bit
circuitous, but it does actually get the job done. The result of this
change is that once RA is finished its initial loading of a project,
goto-definition requests are instant. There don't appear to be any more
surprise latency spikes.

This _partially_ addresses #1650 in that it front-loads the latency of the
first goto-definition request, which in turn makes it more predictable and
less surprising. In particular, this addresses the use case where one opens
the text editor, starts reading code for a while, and only later issues the
first goto-definition request. Before this PR, that first goto-definition request
is guaranteed to have high latency in any reasonably sized project. But
after this PR, there's a good chance that it will now be instant.

What this _doesn't_ address is that initial loading time. In fact, it makes it
longer by adding a phantom goto-definition request to the initial startup
sequence. However, I observed that while this did make initial loading
slower, it was overall a somewhat small (but not insignificant) fraction
of initial loading time.

-----

At least, the above is what I _want_ to do. The actual change in this PR is just a proof-of-concept. I came up with after an evening of printf-debugging. Once I found the spot where this cache priming should go, I was unsure of how to generate a phantom input. So I just took an input I knew worked from my printf-debugging and hacked it in. Obviously, what I'd like to do is make this more general such that it will always work.

I don't know whether this is the "right" approach or not. My guess is that there is perhaps a cleaner solution that more directly primes whatever cache is being lazily populated rather than fudging the issue with a phantom goto-definition request.

I created this as a draft PR because I'd really like help making this general. I think whether y'all want to accept this patch is perhaps a separate question. IMO, it seems like a good idea, but to be honest, I'm happy to maintain this patch on my own since it's so trivial. But I would like to generalize it so that it will work in any project.

My thinking is that all I really need to do is find a file and a token somewhere in the loaded project, and then use that as input. But I don't quite know how to connect all the data structures to do that. Any help would be appreciated!

cc @matklad since I've been a worm in your ear about this problem. :-)

Co-authored-by: Andrew Gallant <jamslam@gmail.com>

2 files changed, 10 insertions, 11 deletions

diff --git a/crates/ra_ide/src/prime_caches.rs b/crates/ra_ide/src/prime_caches.rs
index 628c989bfc3..90bf7d25f20 100644
--- a/crates/ra_ide/src/prime_caches.rs
+++ b/crates/ra_ide/src/prime_caches.rs
@@ -3,13 +3,10 @@
 //! request takes longer to compute. This modules implemented prepopulating of
 //! various caches, it's not really advanced at the moment.
 
-use hir::Semantics;
-
 use crate::{FileId, RootDatabase};
 
 pub(crate) fn prime_caches(db: &RootDatabase, files: Vec<FileId>) {
-    let sema = Semantics::new(db);
     for file in files {
-        let _ = sema.to_module_def(file);
+        let _ = crate::syntax_highlighting::highlight(db, file, None);
     }
 }
diff --git a/crates/rust-analyzer/src/main_loop.rs b/crates/rust-analyzer/src/main_loop.rs
index d993c414689..f3aef3f0f29 100644
--- a/crates/rust-analyzer/src/main_loop.rs
+++ b/crates/rust-analyzer/src/main_loop.rs
@@ -222,6 +222,7 @@ pub fn main_loop(ws_roots: Vec<PathBuf>, config: Config, connection: Connection)
     libdata_receiver.into_iter().for_each(drop);
     log::info!("...tasks have finished");
     log::info!("joining threadpool...");
+    pool.join();
     drop(pool);
     log::info!("...threadpool has finished");
 
@@ -417,28 +418,29 @@ fn loop_turn(
         && loop_state.pending_libraries.is_empty()
         && loop_state.in_flight_libraries == 0
     {
+        state_changed = true;
         loop_state.workspace_loaded = true;
         if let Some(flycheck) = &world_state.flycheck {
             flycheck.update();
         }
-        pool.execute({
-            let subs = loop_state.subscriptions.subscriptions();
-            let snap = world_state.snapshot();
-            move || snap.analysis().prime_caches(subs).unwrap_or_else(|_: Canceled| ())
-        });
     }
 
     if show_progress {
         send_startup_progress(&connection.sender, loop_state);
     }
 
-    if state_changed {
+    if state_changed && loop_state.workspace_loaded {
         update_file_notifications_on_threadpool(
             pool,
             world_state.snapshot(),
             task_sender.clone(),
             loop_state.subscriptions.subscriptions(),
-        )
+        );
+        pool.execute({
+            let subs = loop_state.subscriptions.subscriptions();
+            let snap = world_state.snapshot();
+            move || snap.analysis().prime_caches(subs).unwrap_or_else(|_: Canceled| ())
+        });
     }
 
     let loop_duration = loop_start.elapsed();