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
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
