[PATCH] D81719: [clangd] Drop usage of PreambleStatCache in scanPreamble

[Kadir Cetinkaya via Phabricator via cfe-commits]

kadircet added a comment.

Looked at this more in details, today we call `buildCompilerInvocation` in 3 different places:

- Once in TUScheduler whenever we receive an update, later on it propagates into `buildPreamble` and `buildAST`.
- Once during signatureHelp/codeCompletion. In theory that one can benefit from the stats we've performed during update above.
- Two times during preamblePatching, as we create two different compiler instances. It seems like we don't care about those stats in here at all. As it is used to figure out resource/stdlib directories and other built-in include paths, and preamble patching doesn't care about those.
- Once for read operations, if AST was eviced from cache, again this can benefit from the IO done while building invocation during last update request.

As for the amount of IO done during `buildCompilerInvocation`, I've run a simple test and here is a summary:

  % time     seconds  usecs/call     calls    errors syscall
  ------ ----------- ----------- --------- --------- ----------------
   80.71    0.002330           5       394       374 openat
   10.43    0.000301           4        71        39 stat
    1.49    0.000043           3        12           write
    1.42    0.000041           5         8           lstat
    1.28    0.000037           6         6           getdents64
    1.07    0.000031           1        21           fstat
    1.00    0.000029           1        20           close
    0.87    0.000025           4         6           readlink
    0.55    0.000016           2         6           pread64
    0.38    0.000011          11         1           getcwd
    0.31    0.000009           4         2           ioctl
    0.28    0.000008           8         1         1 lseek
    0.21    0.000006           0        50           mmap
    0.00    0.000000           0        10           read
    0.00    0.000000           0        35        34 access
    0.00    0.000000           0         1           execve
  ------ ----------- ----------- --------- --------- ----------------
  100.00    0.002887                   644       448 total

Looks like only caching stats, especially without caching failures, is not  going to a big win.
Most of the io is done during directory listing(`openat`), even though caching "success" in case of directory listing might be hard, we can actually cache only failures to mark missing files.

So ISTM:

- buildCompilerInvocation usage inside scanPreamble doesn't need any access to any files, so I suggest we just pass empty FS.
- we need a different cache for `buildCompilerInvocation`, one that caches `dir_begin()` failures.
- IO done by `buildCompilerInvocation` depends on compiler args like `sysroot`, `stdlib` (and likely many more), hence I believe they should be cached inside TUScheduler and invalidated whenever compile commands change (maybe even more intrusive, a new one at each update since underlying FS can change too).

I am planning to just drop FS usage inside scanPreamble for good, and let the new caching mechanism emerge when we need it more :D WDYT?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D81719