ilya-biryukov added inline comments.
================ Comment at: clangd/TUScheduler.h:56 + std::chrono::steady_clock::duration UpdateDebounce = + std::chrono::milliseconds(500)); ~TUScheduler(); ---------------- Maybe we could remove this default argument now that `ClangdServer` also has the default debounce? I guess the key thing that bothers me is having multiple places that have the magical default constant of `500ms`, it would be nice to have it in one place. ================ Comment at: clangd/Threading.h:76 +private: + enum Type { Zero, Infinite, Finite } Type; + Deadline(enum Type Type) : Type(Type) {} ---------------- Maybe prefer grouping all the fields together? It seems we're giving that up to avoid writing one extra instance of `enum Type`. ``` enum Type { Zero, Infinite, Finite }; Deadline(enum Type Type) : Type(Type) {} enum Type Type; std::chrono::steady_clock::time_point Time; ``` ================ Comment at: clangd/Threading.h:83 Deadline timeoutSeconds(llvm::Optional<double> Seconds); +/// Wait once for on CV for the specified duration. +void wait(std::unique_lock<std::mutex> &Lock, std::condition_variable &CV, ---------------- s/Wait for on/Wait on/? ================ Comment at: unittests/clangd/TUSchedulerTests.cpp:127 + /*ASTParsedCallback=*/nullptr, + /*UpdateDebounce=*/std::chrono::milliseconds(50)); + auto Path = testPath("foo.cpp"); ---------------- I wonder if the default debounce of `500ms` will make other tests (especially those that use `ClangdServer`) too slow? Maybe we should consider settings a smaller default (maybe even `Deadline::zero()`?) and having `500ms` set only by `ClangdLSPServer`? 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