[PATCH] D73873: [clangd] Mechanism to make update debounce responsive to rebuild speed.

[Haojian Wu via Phabricator via cfe-commits]

hokein accepted this revision.
hokein added a comment.
This revision is now accepted and ready to land.

looks good, a few nits.



================
Comment at: clang-tools-extra/clangd/TUScheduler.cpp:249
+  llvm::SmallVector<DebouncePolicy::clock::duration, 8>
+      RebuildTimes; /* GUARDED_BY(Mutex) */
   /// File that ASTWorker is responsible for.
----------------
nit: maybe move it after the `Mutex` declaration.


================
Comment at: clang-tools-extra/clangd/TUScheduler.cpp:771
   // e.g. the first keystroke is live until obsoleted by the second.
   // We debounce "maybe-unused" writes, sleeping 500ms in case they become dead.
   // But don't delay reads (including updates where diagnostics are needed).
----------------
nit: update the stale comment.


================
Comment at: clang-tools-extra/clangd/TUScheduler.h:66
+/// This is so we rebuild once the user stops typing, not when they start.
+/// The debounce time should be responsive to user preferences and rebuild time.
+/// In the future, we could also consider different types of edits.
----------------
nit: `should be` sounds like this is something we haven't done yet? maybe add `FIXME`?


================
Comment at: clang-tools-extra/clangd/TUScheduler.h:72
+  /// The minimum time that we always debounce for.
+  /// (Debounce may still be disabled/interrupted if we must build this version)
+  clock::duration Min = /*zero*/ {};
----------------
nit: this comment in `()` seems to be more related to the `DebouncePolicy`, maybe move it around the class.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D73873/new/

https://reviews.llvm.org/D73873