Skip to content

Commit

Permalink
[clangd] Don't store completion info if the symbol is not used for co…
Browse files Browse the repository at this point in the history
…de completion.

Summary:
This would save us some memory and disk space:
  - Dex usage (261 MB vs 266 MB)
  - Disk (75 MB vs 76 MB)

It would save more when we index the main file symbol D55185.

Reviewers: ilya-biryukov

Reviewed By: ilya-biryukov

Subscribers: nridge, ioeric, MaskRay, jkorous, arphaman, kadircet, cfe-commits

Differential Revision: https://reviews.llvm.org/D56314

llvm-svn: 350803
  • Loading branch information
hokein committed Jan 10, 2019
1 parent 1c69ab2 commit 8f85b9f
Show file tree
Hide file tree
Showing 3 changed files with 38 additions and 14 deletions.
7 changes: 7 additions & 0 deletions clang-tools-extra/clangd/index/Index.h
Expand Up @@ -185,19 +185,23 @@ struct Symbol {
SymbolOrigin Origin = SymbolOrigin::Unknown;
/// A brief description of the symbol that can be appended in the completion
/// candidate list. For example, "(X x, Y y) const" is a function signature.
/// Only set when the symbol is indexed for completion.
llvm::StringRef Signature;
/// What to insert when completing this symbol, after the symbol name.
/// This is in LSP snippet syntax (e.g. "({$0})" for a no-args function).
/// (When snippets are disabled, the symbol name alone is used).
/// Only set when the symbol is indexed for completion.
llvm::StringRef CompletionSnippetSuffix;
/// Documentation including comment for the symbol declaration.
llvm::StringRef Documentation;
/// Type when this symbol is used in an expression. (Short display form).
/// e.g. return type of a function, or type of a variable.
/// Only set when the symbol is indexed for completion.
llvm::StringRef ReturnType;

/// Raw representation of the OpaqueType of the symbol, used for scoring
/// purposes.
/// Only set when the symbol is indexed for completion.
llvm::StringRef Type;

struct IncludeHeaderWithReferences {
Expand All @@ -223,12 +227,15 @@ struct Symbol {
/// - If we haven't seen a definition, this covers all declarations.
/// - If we have seen a definition, this covers declarations visible from
/// any definition.
/// Only set when the symbol is indexed for completion.
llvm::SmallVector<IncludeHeaderWithReferences, 1> IncludeHeaders;

enum SymbolFlag : uint8_t {
None = 0,
/// Whether or not this symbol is meant to be used for the code completion.
/// See also isIndexedForCodeCompletion().
/// Note that we don't store completion information (signature, snippet,
/// type, inclues) if the symbol is not indexed for code completion.
IndexedForCodeCompletion = 1 << 0,
/// Indicates if the symbol is deprecated.
Deprecated = 1 << 1,
Expand Down
32 changes: 22 additions & 10 deletions clang-tools-extra/clangd/index/SymbolCollector.cpp
Expand Up @@ -530,6 +530,10 @@ const Symbol *SymbolCollector::addDeclaration(const NamedDecl &ND,
getTokenLocation(Loc, SM, Opts, ASTCtx->getLangOpts(), FileURI))
S.CanonicalDeclaration = *DeclLoc;

S.Origin = Opts.Origin;
if (ND.getAvailability() == AR_Deprecated)
S.Flags |= Symbol::Deprecated;

// Add completion info.
// FIXME: we may want to choose a different redecl, or combine from several.
assert(ASTCtx && PP.get() && "ASTContext and Preprocessor must be set.");
Expand All @@ -539,13 +543,28 @@ const Symbol *SymbolCollector::addDeclaration(const NamedDecl &ND,
*ASTCtx, *PP, CodeCompletionContext::CCC_Symbol, *CompletionAllocator,
*CompletionTUInfo,
/*IncludeBriefComments*/ false);
std::string Signature;
std::string SnippetSuffix;
getSignature(*CCS, &Signature, &SnippetSuffix);
std::string Documentation =
formatDocumentation(*CCS, getDocComment(Ctx, SymbolCompletion,
/*CommentsFromHeaders=*/true));
// For symbols not indexed for completion (class members), we also store their
// docs in the index, because Sema doesn't load the docs from the preamble, we
// rely on the index to get the docs.
// FIXME: this can be optimized by only storing the docs in dynamic index --
// dynamic index should index these symbols when Sema completes a member
// completion.
S.Documentation = Documentation;
if (!(S.Flags & Symbol::IndexedForCodeCompletion)) {
Symbols.insert(S);
return Symbols.find(S.ID);
}

std::string Signature;
std::string SnippetSuffix;
getSignature(*CCS, &Signature, &SnippetSuffix);
S.Signature = Signature;
S.CompletionSnippetSuffix = SnippetSuffix;
std::string ReturnType = getReturnType(*CCS);
S.ReturnType = ReturnType;

std::string Include;
if (Opts.CollectIncludePath && shouldCollectIncludePath(S.SymInfo.Kind)) {
Expand All @@ -555,10 +574,6 @@ const Symbol *SymbolCollector::addDeclaration(const NamedDecl &ND,
QName, SM, SM.getExpansionLoc(ND.getLocation()), Opts))
Include = std::move(*Header);
}
S.Signature = Signature;
S.CompletionSnippetSuffix = SnippetSuffix;
S.Documentation = Documentation;
S.ReturnType = ReturnType;
if (!Include.empty())
S.IncludeHeaders.emplace_back(Include, 1);

Expand All @@ -569,9 +584,6 @@ const Symbol *SymbolCollector::addDeclaration(const NamedDecl &ND,
S.Type = TypeStorage->raw();
}

S.Origin = Opts.Origin;
if (ND.getAvailability() == AR_Deprecated)
S.Flags |= Symbol::Deprecated;
Symbols.insert(S);
return Symbols.find(S.ID);
}
Expand Down
13 changes: 9 additions & 4 deletions clang-tools-extra/unittests/clangd/SymbolCollectorTests.cpp
Expand Up @@ -654,10 +654,15 @@ TEST_F(SymbolCollectorTest, ClassMembers) {
void Foo::ssf() {}
)";
runSymbolCollector(Header, Main);
EXPECT_THAT(Symbols,
UnorderedElementsAre(QName("Foo"), QName("Foo::f"),
QName("Foo::g"), QName("Foo::sf"),
QName("Foo::ssf"), QName("Foo::x")));
EXPECT_THAT(
Symbols,
UnorderedElementsAre(
QName("Foo"),
AllOf(QName("Foo::f"), ReturnType(""), ForCodeCompletion(false)),
AllOf(QName("Foo::g"), ReturnType(""), ForCodeCompletion(false)),
AllOf(QName("Foo::sf"), ReturnType(""), ForCodeCompletion(false)),
AllOf(QName("Foo::ssf"), ReturnType(""), ForCodeCompletion(false)),
AllOf(QName("Foo::x"), ReturnType(""), ForCodeCompletion(false))));
}

TEST_F(SymbolCollectorTest, Scopes) {
Expand Down

0 comments on commit 8f85b9f

Please sign in to comment.