This revision was automatically updated to reflect the committed changes.
Closed by commit rL346221: [clangd] auto-index stores symbols per-file instead
of per-TU. (authored by ioeric, committed by ).
Herald added a subscriber: llvm-commits.
Repository:
rL LLVM
https://reviews.llvm.org/D53433
ioeric updated this revision to Diff 172732.
ioeric added a comment.
- Rebase
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D53433
Files:
clangd/index/Background.cpp
clangd/index/Background.h
clangd/index/FileIndex.cpp
clangd/index/FileIndex.h
clangd/index/SymbolColle
ioeric added inline comments.
Comment at: clangd/index/Background.cpp:235
+IndexedFileDigests[Path] = FilesToUpdate.lookup(Path);
+IndexedSymbols.update(Path,
+ make_unique(std::move(Syms).build()),
kadircet wrote:
> This call is
kadircet added inline comments.
Comment at: clangd/index/Background.cpp:235
+IndexedFileDigests[Path] = FilesToUpdate.lookup(Path);
+IndexedSymbols.update(Path,
+ make_unique(std::move(Syms).build()),
This call is already thread-s
sammccall added inline comments.
Comment at: clangd/index/Background.cpp:194
- // FIXME: partition the symbols by file rather than TU, to avoid duplication.
- IndexedSymbols.update(AbsolutePath,
-llvm::make_unique(std::move(Symbols)),
ka
kadircet added inline comments.
Comment at: clangd/index/Background.cpp:194
- // FIXME: partition the symbols by file rather than TU, to avoid duplication.
- IndexedSymbols.update(AbsolutePath,
-llvm::make_unique(std::move(Symbols)),
sam
sammccall accepted this revision.
sammccall added inline comments.
Comment at: clangd/index/Background.cpp:194
- // FIXME: partition the symbols by file rather than TU, to avoid duplication.
- IndexedSymbols.update(AbsolutePath,
-llvm::make_unique(std::m
ioeric added a comment.
Made some more changes to make the code work in multiple threads (add mutex for
digests, take snapshot of digests for each run etc). PTAL
Comment at: clangd/index/Background.cpp:311
SPAN_ATTACH(Tracer, "refs", int(Refs.numRefs()));
- // FIXME: parti
ioeric updated this revision to Diff 171901.
ioeric marked 3 inline comments as done.
ioeric added a comment.
- Merged with multi-threading changes. Added mutex for file digests.
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D53433
Files:
clangd/index/Background.cpp
clangd/
kadircet added inline comments.
Comment at: clangd/index/SymbolCollector.cpp:619
+ shouldIndexFile(SM, SM.getFileID(Loc), Opts, &FilesToIndexCache);
+ if (auto DefLoc = getTokenLocation(Loc,
ND.getASTContext().getSourceManager(),
Opts, AST
sammccall accepted this revision.
sammccall added inline comments.
Comment at: clangd/index/Background.cpp:311
SPAN_ATTACH(Tracer, "refs", int(Refs.numRefs()));
- // FIXME: partition the symbols by file rather than TU, to avoid duplication.
- IndexedSymbols.update(AbsolutePa
ioeric added inline comments.
Comment at: clangd/index/Background.cpp:311
SPAN_ATTACH(Tracer, "refs", int(Refs.numRefs()));
- // FIXME: partition the symbols by file rather than TU, to avoid duplication.
- IndexedSymbols.update(AbsolutePath,
-llvm::ma
ioeric updated this revision to Diff 171684.
ioeric marked 11 inline comments as done.
ioeric added a comment.
- address review comments.
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D53433
Files:
clangd/index/Background.cpp
clangd/index/Background.h
clangd/index/FileInd
sammccall accepted this revision.
sammccall added a comment.
This revision is now accepted and ready to land.
Great stuff! Just nits (though a few of them; this is a tricky patch!).
Comment at: clangd/index/Background.cpp:122
+inline BackgroundIndex::FileDigest digest(StringR
ioeric updated this revision to Diff 171467.
ioeric added a comment.
- minor cleanup and a friendly ping.
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D53433
Files:
clangd/index/Background.cpp
clangd/index/Background.h
clangd/index/FileIndex.cpp
clangd/index/FileIndex.
ioeric added inline comments.
Comment at: clangd/index/IndexAction.h:33
+std::function RefsCallback,
+std::function FileDigestsCallback);
sammccall wrote:
> thinking about what we eventually want here:
> - the index action needs to tell the auto-indexe
ioeric updated this revision to Diff 170664.
ioeric marked 5 inline comments as done.
ioeric added a comment.
- address review comments
- merged with origin/master
- Cleanup
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D53433
Files:
clangd/index/Background.cpp
clangd/index
sammccall added inline comments.
Comment at: clangd/index/Background.h:68
+ FileSymbols IndexedSymbols;
+ FileDigests IndexedFileDigests; // Keyed by file URIs.
Keying this with file URIs seems suspicious to me - why this change?
It seems to be motivated by
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
19 matches
Mail list logo
