LS: Implement multithreaded diagnostics refreshing

[mkaput]

Stack:

• LS: Implement multithreaded diagnostics refreshing #6658 ⬅
• LS: Refactor DiagnosticsController #6657
• LS: Add batching files for diagnostics and plug it into refreshing logic #6615

⚠️ Part of a stack created by spr. Do not merge manually using the UI - doing so may have unexpected results.

[reviewable-StarkWare]

This change is 

[Arcticae]

Reviewed 2 of 6 files at r1, all commit messages.
Reviewable status: 2 of 6 files reviewed, 6 unresolved discussions (waiting on @Draggu, @mkaput, @orizi, and @piotmag769)

---

crates/cairo-lang-language-server/src/lang/diagnostics/mod.rs line 80 at r1 (raw file):

        };

        let parallelism = this.workers.parallelism().get();

Shouldn't this value be injected?

---

crates/cairo-lang-language-server/src/lang/diagnostics/mod.rs line 110 at r1 (raw file):

    fn diagnostics_controller_tick(&self, mut state_snapshots: Vec<StateSnapshot>) {
        assert_eq!(state_snapshots.len(), self.workers.parallelism().get() + 1);
        let state = state_snapshots.pop().expect("we just asserted that it exists");

Not a useful message, if we get it (i know you get the location with the trace but still it's just weird)

---

crates/cairo-lang-language-server/src/lang/diagnostics/mod.rs line 129 at r1 (raw file):

        self.spawn_worker(move |project_diagnostics, notifier| {
            clear_old_diagnostics(&state.db, files_to_preserve, project_diagnostics, notifier);

Just to make sure, there's a guarantee that the ones that were refreshed in this batch will publish too, when the diags are empty? (otherwise there's inconsistent state on the side of the client)

---

crates/cairo-lang-language-server/src/lang/diagnostics/project_diagnostics.rs line 12 at r1 (raw file):

/// Takes the result of locking and on error logs it and returns the default result value.
macro_rules! try_lock {

It implies that the macro itself does the locking which is a tad confusing

---

crates/cairo-lang-language-server/src/lang/diagnostics/refresh.rs line 75 at r1 (raw file):

    for mut file_diagnostics in removed {
        // It might be that we are removing a file that actually had some diagnostics.
        // For example, this might happen if a `mod` item is removed for a file with a syntax error.

I don't really understand this one sadly :|

---

crates/cairo-lang-language-server/src/lang/diagnostics/file_diagnostics.rs line 31 at r1 (raw file):

/// ## Virtual files
///
/// When collecting diagnostics using [`FileDiagnostics::collect`], all virtual files related

Shouldn't it be mentioned that it's also only visiting main module files?

[mkaput]

Reviewable status: 1 of 6 files reviewed, 6 unresolved discussions (waiting on @Arcticae, @Draggu, @orizi, and @piotmag769)

---

crates/cairo-lang-language-server/src/lang/diagnostics/mod.rs line 80 at r1 (raw file):

Previously, Arcticae (Tomasz Rejowski) wrote…

Shouldn't this value be injected?

what do you mean by injected?

---

crates/cairo-lang-language-server/src/lang/diagnostics/mod.rs line 110 at r1 (raw file):

Previously, Arcticae (Tomasz Rejowski) wrote…

Not a useful message, if we get it (i know you get the location with the trace but still it's just weird)

removed this message while refactoring. done

[Draggu]

Reviewable status: 1 of 6 files reviewed, 8 unresolved discussions (waiting on @Arcticae, @mkaput, @orizi, and @piotmag769)

---

crates/cairo-lang-language-server/src/server/schedule/thread/pool.rs line 96 at r2 (raw file):

        }

        Pool { _handles: handles, job_sender, parallelism: NonZero::new(threads).unwrap() }

Can this be constructed from _handles.len()?

---

crates/cairo-lang-language-server/src/lang/diagnostics/file_diagnostics.rs line 27 at r2 (raw file):

///
/// Diagnostics in this structure are stored as Arcs that directly come from Salsa caches.
/// This means that equality comparisons of `FileDiagnostics` are efficient.

This is just not true. https://github.com/rust-lang/rust/blob/a4cedecc9ec76b46dcbb954750068c832cf2dd43/library/alloc/src/sync.rs#L3267-L3268. Arc is calling T implementation of PartialEq so it does not matter if we use Arc here or not.

Code quote:

/// Diagnostics in this structure are stored as Arcs that directly come from Salsa caches.
/// This means that equality comparisons of `FileDiagnostics` are efficient.