kadircet added inline comments.
================
Comment at: clang-tools-extra/clangd/index/BackgroundIndexLoader.cpp:93
+
+void BackgroundIndexLoader::load(PathRef MainFile,
+ BackgroundIndexStorage *Storage) {
----------------
sammccall wrote:
> This handles only one main file, I can't see where you skip loading shards
> for headers that were loaded for a previous file
The real loading happens down the line at `const CachedShard &CS =
loadShard(ShardIdentifier, Storage);`, which returns shard from cache if it was
already loaded.
================
Comment at: clang-tools-extra/clangd/index/BackgroundIndexLoader.cpp:126
+ assert(TUsIt != FileToTUs.end() && "No TU registered for the shard");
+ Result.back().DependentTUs = std::move(TUsIt->second);
+ }
----------------
sammccall wrote:
> I don't think this works, the storage for these StringRefs is the
> LoadedShards you just moved from.
> (In practice, maybe you get away with it if the strings are longer than the
> SSO?)
>
>
Yes you are right, I don't see a good way out of it with current data model. I
suppose we can keep an "intern table :)" for the strings, but then LoadedShards
would be meaningless without that intern table so we would end up returning
another struct from `loadIndexShards` that will wrap
`std::vector<LoadedShards>` and the intern table(but we tried to mitigate from
that struct in previous iteration).
So instead I am changing the model to return only the first translation unit
and actually this makes a lot of sense when we think about shards for standard
library, in a code base like chromium vector could easily have ~40k dependent
TUs and even if we just returned a `vector<StringRef>` it would still be around
640kB, and we have a lot of standard library headers. this can quite easily add
hundreds of MB to clangd's memory footprint while loading shards.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D64712/new/
https://reviews.llvm.org/D64712
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
