[PATCH] D43648: [clangd] Debounce streams of updates.

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov added inline comments.


================
Comment at: clangd/TUScheduler.cpp:140
+  // Time to wait after an update to see whether another update obsoletes it.
+  steady_clock::duration UpdateDebounce;
 };
----------------
Maybe make it `const` and put beside `RunSync`? Both are scheduling options, so it probably makes sense to group them together.


================
Comment at: clangd/TUScheduler.cpp:324
+        if (*Deadline)
+          RequestsCV.wait_until(Lock, **Deadline);
+        else
----------------
It looks like if we unwrap `Optional<Deadline>` to `Deadline`, we could replace this code with `wait` helper from `Threading.h`.
The tracing code (e.g. `if (!Requests.empty) { /*...*/}`) could be changed to log only when `*Deadline - steady_clock::now()` is positive.
Will probably make the code simpler. WDYT?


================
Comment at: clangd/TUScheduler.cpp:358
+  for (const auto &R : Requests)
+    if (R.UpdateType == None || R.UpdateType == WantDiagnostics::Yes)
+      return None;
----------------
NIT: I tend to find multi-level nested statements easier to read with braces, e.g. 
```
for (const auto &R : Requests) {
  if (Cond)
    return None
}
```
But this is up to you.


================
Comment at: clangd/TUScheduler.h:55
+              ASTParsedCallback ASTCallback,
+              std::chrono::steady_clock::duration UpdateDebounce =
+                  std::chrono::milliseconds(500));
----------------
As discussed offline, we want to expose the debounce parameter in `ClangdServer`, as there are existing clients of the code that already send updates with low frequency and it would be wasteful for them to do any extra waits.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D43648