[PATCH] D92201: [clangd] Make sure project-aware index is up-to-date for estimateMemoryUsage()

Sun Nov 29 02:45:03 PST 2020

sammccall added a comment.

In D92201#2419933 <https://reviews.llvm.org/D92201#2419933>, @kbobyrev wrote:

> Hm, yes, this makes sense. I just found it awkward that memory usage estimate will spike right after the first request but that's actually the reality (loading the index is deferred until the first actual index request).

Exactly. The good news is: I don't think we have any particular use case for probing the memory usage right after startup.

> My confusion here is that I don't really understand whether deferring the index loading is a good thing because we're practically moving the delay from the warm-up to the working state which is suboptimal.

It's not great that we offer partial functionality without signaling this somehow, and we could consider making this more transparent.
However this happens in lots of places in clangd, especially w.r.t index (completion before preamble, background index async load, actual background indexing itself, the cached-build-artifacts we have internally at google) and in every case feedback has been positive when we introduced it. Blocking the user for any one feature is rarely a good trade, if we can recover even half-gracefully instead.

> I was trying to cause the index to load in the initialization stage for D92198 <https://reviews.llvm.org/D92198> but couldn't figure out where exactly the config is loaded and what could be the best place for triggering index loading.

Fair point, will have a look at that. Forcing an index load might be a nice side-effect of this patch, but I think we can find a better way to do it.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92201