This is an archive of the discontinued LLVM Phabricator instance.

[clangd] Force Dex to respect symbol collector flags
ClosedPublic

Authored by kbobyrev on Sep 21 2018, 6:31 AM.

Details

Summary

Dex should utilize FuzzyFindRequest.RestrictForCodeCompletion flags and omit symbols not meant for code completion when asked for it.

The measurements below were conducted with setting FuzzyFindRequest.RestrictForCodeCompletion to true (so that it's more realistic). Sadly, the average latency goes down, I suspect that is mostly because of the empty queries where the number of posting lists is critical.

MetricsBeforeAfterRelative difference
Cumulative query latency (7000 FuzzyFindRequests over LLVM static index)6182735043 ns7202442053 ns+16%
Whole Index size81.24 MB81.79 MB+0.6%

Out of 292252 symbols collected from LLVM codebase 136926 appear to be restricted for code completion.

Diff Detail

Event Timeline

kbobyrev created this revision.Sep 21 2018, 6:31 AM
ioeric added inline comments.Sep 21 2018, 6:36 AM
clang-tools-extra/clangd/index/dex/Dex.cpp
27

Why do we need NotRestrictedForCodeCompletion token? If RestrictedForCodeCompletion is false, all symbols can match.

kbobyrev updated this revision to Diff 166474.Sep 21 2018, 7:02 AM
kbobyrev marked an inline comment as done.
kbobyrev edited the summary of this revision. (Show Details)

RestirctForCodeCompletion and !RestrictForCodeCompletion are not mutually exclusive.

kbobyrev edited the summary of this revision. (Show Details)Sep 21 2018, 7:07 AM
kbobyrev edited the summary of this revision. (Show Details)
ioeric accepted this revision.Sep 21 2018, 7:34 AM
ioeric added inline comments.
clang-tools-extra/clangd/index/dex/Dex.cpp
185

nit: the comment seems trivial from the if-condition. Maybe remove?

clang-tools-extra/unittests/clangd/DexTests.cpp
594

I think the symbol flags should be set before both requests.

This revision is now accepted and ready to land.Sep 21 2018, 7:34 AM
kbobyrev updated this revision to Diff 166482.Sep 21 2018, 7:38 AM
kbobyrev marked 2 inline comments as done.
This revision was automatically updated to reflect the committed changes.