[PATCH] D125488: [Preprocessor] Make the special lexing for dependency scanning a first-class feature of the `Preprocessor` and `Lexer`

Duncan P. N. Exon Smith via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Thu May 12 16:19:17 PDT 2022


dexonsmith added a comment.

In D125488#3510265 <https://reviews.llvm.org/D125488#3510265>, @akyrtzi wrote:

> In D125488#3510214 <https://reviews.llvm.org/D125488#3510214>, @dexonsmith wrote:
>
>> Is there code in DepFS that can/should be deleted as part of this patch, or in a follow-up, or is it still around as an option?

[To be clear, my question was because I don't see this patch deleting the code path that minimizes / saves-minimized sources. Can/should we delete the "minimize sources" code path?]

> After these changes, with DepFS we are using its multi-threading sharding technique to cache file `stat`s and the source file directive scanning results. We could use multi-threading sharding only to cache source file directive scanning results, and get rid of `DepFS` altogether, but then I don't see a good way to cache the file `stat`s as well, unless we want to try to re-use the `FileManager` across depscan instances and make its `stat` caching as efficient as DepFS (maybe by generalizing the multi-threading sharding technique and using it in `FileManager`).
>
> There's also `FileSystemStatCache` which seems like a leftover right now, but we could enhance it and have it shared by individual `FileManager` instances (instead of sharing the same `FileManager` in depscan instances).
>
> What do you think?

FWIW, I don't think we should be sharing FileManager at all, since it has state that makes sharing it unsound.

I like the direction of trying to remove FSStatCache. I think stat/content caching belongs at the VFS level so DepFS seems like a better starting point (maybe generalized a bit). Note that DepFS isn't just amortizing `stat` cost, it's also avoiding reopening / `mmap`ing / `mempcy`ing files.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125488



More information about the cfe-commits mailing list