[PATCH] D53433: [clangd] auto-index stores symbols per-file instead of per-TU.

2018-10-29 Thread Eric Liu via Phabricator via cfe-commits
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.

[PATCH] D53433: [clangd] auto-index stores symbols per-file instead of per-TU.

2018-10-30 Thread Sam McCall via Phabricator via cfe-commits
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

[PATCH] D53433: [clangd] auto-index stores symbols per-file instead of per-TU.

2018-10-30 Thread Eric Liu via Phabricator via cfe-commits
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

[PATCH] D53433: [clangd] auto-index stores symbols per-file instead of per-TU.

2018-10-30 Thread Eric Liu via Phabricator via cfe-commits
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

[PATCH] D53433: [clangd] auto-index stores symbols per-file instead of per-TU.

2018-10-30 Thread Sam McCall via Phabricator via cfe-commits
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

[PATCH] D53433: [clangd] auto-index stores symbols per-file instead of per-TU.

2018-10-31 Thread Kadir Cetinkaya via Phabricator via cfe-commits
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

[PATCH] D53433: [clangd] auto-index stores symbols per-file instead of per-TU.

2018-10-31 Thread Eric Liu via Phabricator via cfe-commits
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/

[PATCH] D53433: [clangd] auto-index stores symbols per-file instead of per-TU.

2018-10-31 Thread Eric Liu via Phabricator via cfe-commits
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

[PATCH] D53433: [clangd] auto-index stores symbols per-file instead of per-TU.

2018-10-31 Thread Sam McCall via Phabricator via cfe-commits
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

[PATCH] D53433: [clangd] auto-index stores symbols per-file instead of per-TU.

2018-11-01 Thread Kadir Cetinkaya via Phabricator via cfe-commits
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

[PATCH] D53433: [clangd] auto-index stores symbols per-file instead of per-TU.

2018-11-01 Thread Sam McCall via Phabricator via cfe-commits
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

[PATCH] D53433: [clangd] auto-index stores symbols per-file instead of per-TU.

2018-11-01 Thread Kadir Cetinkaya via Phabricator via cfe-commits
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

[PATCH] D53433: [clangd] auto-index stores symbols per-file instead of per-TU.

2018-11-05 Thread Eric Liu via Phabricator via cfe-commits
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

[PATCH] D53433: [clangd] auto-index stores symbols per-file instead of per-TU.

2018-11-06 Thread Eric Liu via Phabricator via cfe-commits
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

[PATCH] D53433: [clangd] auto-index stores symbols per-file instead of per-TU.

2018-11-06 Thread Eric Liu via Phabricator via cfe-commits
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

[PATCH] D53433: [clangd] auto-index stores symbols per-file instead of per-TU.

2018-10-22 Thread Eric Liu via Phabricator via cfe-commits
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

[PATCH] D53433: [clangd] auto-index stores symbols per-file instead of per-TU.

2018-10-22 Thread Sam McCall via Phabricator via cfe-commits
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

[PATCH] D53433: [clangd] auto-index stores symbols per-file instead of per-TU.

2018-10-23 Thread Eric Liu via Phabricator via cfe-commits
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

[PATCH] D53433: [clangd] auto-index stores symbols per-file instead of per-TU.

2018-10-23 Thread Eric Liu via Phabricator via cfe-commits
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