This revision was automatically updated to reflect the committed changes.
Closed by commit rL340175: [clangd] DexIndex implementation prototype (authored
by omtcyfz, committed by ).
Herald added a subscriber: llvm-commits.
Changed prior to commit:
https://reviews.llvm.org/D50337?vs=161480&id=16
kbobyrev updated this revision to Diff 161480.
kbobyrev marked 6 inline comments as done.
kbobyrev added a comment.
Address post-LGTM comments.
https://reviews.llvm.org/D50337
Files:
clang-tools-extra/clangd/CMakeLists.txt
clang-tools-extra/clangd/index/dex/DexIndex.cpp
clang-tools-extra/
ioeric accepted this revision.
ioeric added a comment.
This revision is now accepted and ready to land.
LG. Last few nits and then good to go.
Comment at: clang-tools-extra/clangd/index/dex/DexIndex.cpp:97
+}
+TopLevelChildren.push_back(createAnd(move(TrigramIterators))
kbobyrev updated this revision to Diff 161432.
kbobyrev marked 2 inline comments as done.
kbobyrev added a comment.
Herald added a subscriber: kadircet.
Use TRUE iterator to ensure validity of the query processing.
https://reviews.llvm.org/D50337
Files:
clang-tools-extra/clangd/CMakeLists.txt
kbobyrev planned changes to this revision.
kbobyrev added a comment.
I should create another patch with True iterator to address the last comment.
Comment at: clang-tools-extra/clangd/index/dex/DexIndex.cpp:97
+// Add OR iterator for scopes if the request contains scopes.
+
kbobyrev updated this revision to Diff 161273.
kbobyrev marked 3 inline comments as done.
kbobyrev added a comment.
Address all the comment, except the one about True iterators.
https://reviews.llvm.org/D50337
Files:
clang-tools-extra/clangd/CMakeLists.txt
clang-tools-extra/clangd/index/dex
ioeric added inline comments.
Comment at: clang-tools-extra/clangd/index/dex/DexIndex.cpp:97
+}
+// FIXME(kbobyrev): Add True iterator as soon as it's implemented
otherwise.
+// If TopLevelChildren vector will be empty it will trigger an assertion.
A
kbobyrev updated this revision to Diff 161252.
kbobyrev marked 9 inline comments as done.
kbobyrev added a comment.
Address another round of comments.
https://reviews.llvm.org/D50337
Files:
clang-tools-extra/clangd/CMakeLists.txt
clang-tools-extra/clangd/index/dex/DexIndex.cpp
clang-tools
ioeric added a comment.
Almost LG! Just a few more nits.
Comment at: clang-tools-extra/clangd/index/dex/DexIndex.cpp:87
+ std::vector SymbolDocIDs;
+ std::priority_queue> Top;
+
nit: move `SymbolDocIDs` and `Top` closer to where they're used.
==
kbobyrev updated this revision to Diff 161235.
kbobyrev marked 7 inline comments as done.
kbobyrev added a comment.
Address another round of comments.
https://reviews.llvm.org/D50337
Files:
clang-tools-extra/clangd/CMakeLists.txt
clang-tools-extra/clangd/index/dex/DexIndex.cpp
clang-tools
ioeric added inline comments.
Comment at: clang-tools-extra/clangd/index/dex/DexIndex.cpp:86
+ const auto TrigramTokens = generateIdentifierTrigrams(Req.Query);
+ TopLevelChildren.push_back(createAnd(createTrigramIterators(TrigramTokens)));
+
`createTrigramIter
kbobyrev updated this revision to Diff 161202.
kbobyrev marked 6 inline comments as done.
kbobyrev added a comment.
Address a round of comments.
https://reviews.llvm.org/D50337
Files:
clang-tools-extra/clangd/CMakeLists.txt
clang-tools-extra/clangd/index/dex/DexIndex.cpp
clang-tools-extra
ioeric added inline comments.
Comment at: clang-tools-extra/clangd/index/dex/DexIndex.cpp:25
+ std::vector> &Scores) {
+ // First, sort retrieved symbols by the cumulative score to apply callbacks
+ // in the order of descending score.
Why is th
kbobyrev updated this revision to Diff 161017.
kbobyrev added a comment.
Sorry, the last diff was the old one. Should be correct now.
https://reviews.llvm.org/D50337
Files:
clang-tools-extra/clangd/CMakeLists.txt
clang-tools-extra/clangd/index/dex/DexIndex.cpp
clang-tools-extra/clangd/ind
ioeric added a comment.
In https://reviews.llvm.org/D50337#1198914, @kbobyrev wrote:
> Don't separate the logic for "long" and "short" queries:
> https://reviews.llvm.org/D50517 (https://reviews.llvm.org/rCTE339548)
> introduced incomplete trigrams which can be used on for "short" queries, too.
kbobyrev updated this revision to Diff 160576.
kbobyrev added a comment.
Don't separate the logic for "long" and "short" queries:
https://reviews.llvm.org/D50517 (https://reviews.llvm.org/rCTE339548)
introduced incomplete trigrams which can be used on for "short" queries, too.
https://reviews.
kbobyrev planned changes to this revision.
kbobyrev added a comment.
As discussed offline, I should update the patch to reflect changes accepted in
https://reviews.llvm.org/D50517.
https://reviews.llvm.org/D50337
___
cfe-commits mailing list
cfe-co
kbobyrev updated this revision to Diff 160146.
kbobyrev marked 2 inline comments as done.
kbobyrev added a comment.
Store symbol qualities (so that it's not computed each time when requested
which might be expensive). Use `operator[]` to construct the value for inverted
index when key is not ins
kbobyrev updated this revision to Diff 160104.
kbobyrev marked 12 inline comments as done.
kbobyrev added a comment.
Address most comments.
https://reviews.llvm.org/D50337
Files:
clang-tools-extra/clangd/CMakeLists.txt
clang-tools-extra/clangd/index/dex/DexIndex.cpp
clang-tools-extra/clan
ioeric added inline comments.
Comment at: clang-tools-extra/clangd/index/dex/DexIndex.cpp:29
+ // might be more efficient.
+ std::sort(begin(*Syms), end(*Syms), [](const Symbol *LHS, const Symbol *RHS)
{
+return quality(*LHS) > quality(*RHS);
Calculating `
kbobyrev updated this revision to Diff 159908.
kbobyrev marked 15 inline comments as done.
kbobyrev added a comment.
Address a round of comments. Also put `FIXME`s where appropriate for the future
changes.
https://reviews.llvm.org/D50337
Files:
clang-tools-extra/clangd/CMakeLists.txt
clang
ioeric added inline comments.
Comment at: clang-tools-extra/clangd/index/MemIndex.h:45
// Index is a set of symbols that are deduplicated by symbol IDs.
- // FIXME: build smarter index structure.
llvm::DenseMap Index;
I think this FIXME still applies here.
kbobyrev updated this revision to Diff 159658.
kbobyrev added a comment.
Minor code cleanup. This is now a fully functional symbol index.
I have reflected my concerns and uncertainties in `FIXME`s, please indicate if
you think there's something to improve in this patch. In general, I believe it
kbobyrev updated this revision to Diff 159515.
kbobyrev added a comment.
Continue implementing Proof of Concept Dex-based static index replacement.
This diff adds short query processing, the current solution does not utilize
iterators framework (unlike the general queries) yet and is a subject t
kbobyrev added a comment.
As discussed offline, incomplete trigrams (unigrams and bigrams generation)
should be a blocker for this patch, because otherwise it isn't functional. Once
incomplete trigrams are in, `MemIndex` tests can be reused for `DexIndex` to
ensure stability.
https://reviews.
kbobyrev updated this revision to Diff 159463.
kbobyrev added a comment.
Don't resize retrieved symbols vector, simply let callback process at most
`MaxCandidateCount` items.
https://reviews.llvm.org/D50337
Files:
clang-tools-extra/clangd/CMakeLists.txt
clang-tools-extra/clangd/index/MemIn
kbobyrev planned changes to this revision.
kbobyrev added a comment.
The patch is currently in preview-mode; I have to make few changes:
- Improve testing infrastructure; one possible way would be to use exactly the
same code `MemIndex` currently does as it is meant to be a drop-in replacement.
kbobyrev created this revision.
kbobyrev added reviewers: ioeric, ilya-biryukov.
kbobyrev added a project: clang-tools-extra.
Herald added subscribers: arphaman, mgrang, jkorous, MaskRay, mgorny.
This patch is a proof-of-concept Dex index implementation. It has several
flaws, which don't allow re
28 matches
Mail list logo
