Stream: t-compiler/rust-analyzer

Topic: Getting unexpected publish diagnostics notifications


Martin Asquino (Nov 27 2020 at 21:09, on Zulip):

Hello again! I noticed something in my workflow that I'm not sure if it's a client issue or server issue, but for one it seems like it doesn't happen with for example the Go language server.

The issue I'm seeing is something like this: if I enter insert mode in vim and say I add 10 lines all of which have a diagnostic to be reported, then I leave insert mode, everything is fine at this point. When I save that file though, and the client sends the did_save notification, the server eventually ends up sending 10 publish diagnostics notifications to the client, and it seems like they are incrementally reporting the diagnostics. So for example, the first notification has one the first diagnostic, the second notification has the first two diagnostics, the third notification has the first three, etc. Is this a known issue? Or is this some sort of incremental approach that is expected?

Jonas Schievink [he/him] (Nov 27 2020 at 21:14, on Zulip):

Yeah, this does look like the expected behavior from looking at the code

Jonas Schievink [he/him] (Nov 27 2020 at 21:14, on Zulip):

Does it cause problems? Aside from being inefficient, of course

Martin Asquino (Nov 27 2020 at 21:21, on Zulip):

Not really causing issues no, I was just confused about it. I don't suppose you know why that is the case, but now I'm curious and I have to ask :grinning:

Jonas Schievink [he/him] (Nov 27 2020 at 21:24, on Zulip):

Each rustc diagnostics is received as a flycheck::Message::AddDiagnostic here. A bit later we check for file whose diagnostics changed, and send all diagnostics of that file to the client.

Since both happens once per loop turn we'll resend all diagnostics of a file for every new rustc diagnostic.

Jonas Schievink [he/him] (Nov 27 2020 at 21:27, on Zulip):

The fix is to do the same thing we already do for VFS and PrimeCaches: Coalesce as many messages as possible from the flycheck receiver into a single loop turn.

Martin Asquino (Nov 27 2020 at 21:29, on Zulip):

oh ok so this is something that would be ideally fixed then, it's not a design decision that's set in stone

Jonas Schievink [he/him] (Nov 27 2020 at 21:53, on Zulip):

@Martin Asquino https://github.com/rust-analyzer/rust-analyzer/pull/6656 should fix the issue, could you try that?

Martin Asquino (Nov 27 2020 at 21:55, on Zulip):

hm I pretty much made the same modification in my local copy of ra and it didn't seem to fix, but I'll try yours just in case

Martin Asquino (Nov 27 2020 at 21:56, on Zulip):

but I reckon that's the best we can do in any case

Jonas Schievink [he/him] (Nov 27 2020 at 21:57, on Zulip):

If diagnostics are coming in too slowly from rustc, it might not have an effect

Jonas Schievink [he/him] (Nov 27 2020 at 21:58, on Zulip):

For example, the PrimeCaches coalescing code still results in a few dozen messages, even though they arrive so quickly that I never see the progress indicator show up

Jonas Schievink [he/him] (Nov 27 2020 at 21:58, on Zulip):

Maybe what we need is to actually delay for a couple milliseconds before sending these sorts of incremental updates to the client

matklad (Nov 28 2020 at 14:56, on Zulip):

Maybe what we need is to actually delay for a couple milliseconds before sending these sorts of incremental updates to the client

Yeah, this seems like a good idea.

matklad (Nov 28 2020 at 14:56, on Zulip):

The problem here though, is that I don't know how ti implement this logic in a simple and composable way.

matklad (Nov 28 2020 at 14:59, on Zulip):

The ideal debouncing behavior is this:

matklad (Nov 28 2020 at 14:59, on Zulip):

And the last bit requries support for timers

matklad (Nov 28 2020 at 15:03, on Zulip):

it almost feels like we should make the main_loop async :fear:

Last update: Jul 29 2021 at 08:45UTC