kbobyrev added inline comments.

================
Comment at: clang-tools-extra/clangd/index/remote/server/Server.cpp:308
+                          .count());
+    // FIXME(kirillbobyrev): This is not really "freshness", this is just the
+    // time since last index reload.
----------------
kadircet wrote:
> what's the issue to be fixed here exactly? do we want to display the index 
> build time instead of age? (if so what's the blocker)
The point here is that is the index load freshness but it's not the index load, 
not the index age itself. The index age is e.g. the time of the last commit 
being indexed. Hence, we don't actually have this information _right now_ - it 
should be provided separately (probably in a way of adjacent `index.metadata` 
file or something similar). E.g. when we download from GitHub release, we do 
know the time an index was uploaded, that's a better estimate of the index 
"age" probably. But what the comment in Proto file says is actually more like 
the commit time, e.g. when did the last change that made it into the index 
happen.


================
Comment at: clang-tools-extra/clangd/index/remote/server/Server.cpp:457
 
-  std::thread HotReloadThread([&Index, &Status, &FS]() {
+  Monitor Monitor(std::chrono::system_clock::now());
+
----------------
kadircet wrote:
> unless we call updateIndex here too, monitoring service won't know about the 
> one being served until the first reload right?
> 
> (another option would be to just not load the index at startup immediately, 
> and handle the initial load in HotReloadThread, but that can be a separate 
> change.)
I think using `Status.getLastModificationTime()` should be correct, right? This 
is the index we actually loaded (checked above).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D98246

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to