[PATCH] D87450: [clangd] Implement hot index reloading for clangd-index-server

Mon Sep 14 01:54:13 PDT 2020

kadircet added inline comments.

================
Comment at: clang-tools-extra/clangd/index/remote/server/Server.cpp:226
+  llvm::sys::TimePoint<> LastModificationTime =
+      std::chrono::system_clock::now();
+  for (;; std::this_thread::sleep_for(std::chrono::seconds(90))) {
----------------
kbobyrev wrote:
> kadircet wrote:
> > i think we should rather store the file_status from the original file in here. as we want to reload whenever files size/modification_time has changed.
> > it doesn't have to be moving forward let alone share the same timezone restrictions and such provided by system_clock::now.
> > 
> > Also it would be nice to use a VFS in here, that way we can move this logic into a helper in the future if other components need similar logic.
> Can you elaborate more on VFS usage? I'm not sure how this would improve existing and potential future code over plain `llvm::sys::*` calls.
it is mostly for unit testing the helper when there's a wider usage. also some users of clangd($Employer's internal editor) doesn't really have a real fs, they surface it through a VFS.

it also makes the code a little bit more easier to read, but definitely doesn't require immediate action, so up to you.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D87450