ioeric added inline comments.
================
Comment at: clangd/index/FileIndex.h:49
+/// rename the existing FileSymbols to something else e.g. TUSymbols?
+class SymbolsGroupedByFiles {
+public:
----------------
sammccall wrote:
> `FileSymbols` isn't actually that opinionated about the data it manages:
> - the keys ("filenames") just identify shards so we can tell whether a new
> shard is an addition or replaces an existing one.
> - the values (tu symbols/refs) are merged using pretty generic logic, we
> don't look at the details.
>
> I think we should be able to use `FileSymbols` mostly unmodified, and store
> digests in parallel, probably in `BackgroundIndexer`. Is there a strong
> reason to store "main file" digests separately to header digests?
>
> There are a couple of weak points:
> - The merging makes subtle assumptions: for symbols it picks one copy
> arbitrarily, assuming there are many copies (merging would be expensive) and
> they're mostly interchangeable duplicates. We could add a constructor or
> `buildIndex()` param to FileSymbols to control this, similar to the IndexType
> param. (For refs it assumes no duplicates and simply concatenates, which is
> also fine for our purposes).
> - `FileSymbols` locks internally to be threadsafe while keeping critical
> sections small. To keep digests in sync with FileSymbols contents we probably
> have to lock externally with a second mutex. This is a little ugly but not a
> real problem I think.
Good idea! Thanks!
> FileSymbols locks internally to be threadsafe while keeping critical sections
> small. To keep digests in sync with FileSymbols contents we probably have to
> lock externally with a second mutex. This is a little ugly but not a real
> problem I think.
This doesn't seem to be a problem as `Background::index()` currently assumes
single-thread? I can either make it thread-safe now or add a `FIXME`. WDYT?
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D53433
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
