Skip to content

Commit

Permalink
[clangd] Clear the semantic of RefSlab::size.
Browse files Browse the repository at this point in the history
Summary:
The RefSlab::size can easily cause confusions, it returns the number of
different symbols, rahter than the number of all references.

- add numRefs() method and cache it, since calculating it everytime is nontrivial.
- clear misused places.

Reviewers: sammccall

Subscribers: ilya-biryukov, ioeric, MaskRay, jkorous, arphaman, kadircet, cfe-commits

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

llvm-svn: 344745
  • Loading branch information
hokein committed Oct 18, 2018
1 parent c3439b0 commit 6ece6e7
Show file tree
Hide file tree
Showing 5 changed files with 19 additions and 12 deletions.
4 changes: 2 additions & 2 deletions clang-tools-extra/clangd/index/Background.cpp
Expand Up @@ -174,9 +174,9 @@ llvm::Error BackgroundIndex::index(tooling::CompileCommand Cmd) {
Action->EndSourceFile();

log("Indexed {0} ({1} symbols, {2} refs)", Inputs.CompileCommand.Filename,
Symbols.size(), Refs.size());
Symbols.size(), Refs.numRefs());
SPAN_ATTACH(Tracer, "symbols", int(Symbols.size()));
SPAN_ATTACH(Tracer, "refs", int(Refs.size()));
SPAN_ATTACH(Tracer, "refs", int(Refs.numRefs()));
// FIXME: partition the symbols by file rather than TU, to avoid duplication.
IndexedSymbols.update(AbsolutePath,
llvm::make_unique<SymbolSlab>(std::move(Symbols)),
Expand Down
4 changes: 2 additions & 2 deletions clang-tools-extra/clangd/index/FileIndex.cpp
Expand Up @@ -63,9 +63,9 @@ indexSymbols(ASTContext &AST, std::shared_ptr<Preprocessor> PP,
auto Refs = Collector.takeRefs();
vlog("index AST for {0} (main={1}): \n"
" symbol slab: {2} symbols, {3} bytes\n"
" ref slab: {4} symbols, {5} bytes",
" ref slab: {4} symbols, {5} refs, {6} bytes",
FileName, IsIndexMainAST, Syms.size(), Syms.bytes(), Refs.size(),
Refs.bytes());
Refs.numRefs(), Refs.bytes());
return {std::move(Syms), std::move(Refs)};
}

Expand Down
4 changes: 3 additions & 1 deletion clang-tools-extra/clangd/index/Index.cpp
Expand Up @@ -172,17 +172,19 @@ RefSlab RefSlab::Builder::build() && {
// Reallocate refs on the arena to reduce waste and indirections when reading.
std::vector<std::pair<SymbolID, ArrayRef<Ref>>> Result;
Result.reserve(Refs.size());
size_t NumRefs = 0;
for (auto &Sym : Refs) {
auto &SymRefs = Sym.second;
llvm::sort(SymRefs);
// FIXME: do we really need to dedup?
SymRefs.erase(std::unique(SymRefs.begin(), SymRefs.end()), SymRefs.end());

NumRefs += SymRefs.size();
auto *Array = Arena.Allocate<Ref>(SymRefs.size());
std::uninitialized_copy(SymRefs.begin(), SymRefs.end(), Array);
Result.emplace_back(Sym.first, ArrayRef<Ref>(Array, SymRefs.size()));
}
return RefSlab(std::move(Result), std::move(Arena));
return RefSlab(std::move(Result), std::move(Arena), NumRefs);
}

void SwapIndex::reset(std::unique_ptr<SymbolIndex> Index) {
Expand Down
9 changes: 7 additions & 2 deletions clang-tools-extra/clangd/index/Index.h
Expand Up @@ -407,7 +407,9 @@ class RefSlab {

const_iterator begin() const { return Refs.begin(); }
const_iterator end() const { return Refs.end(); }
/// Gets the number of symbols.
size_t size() const { return Refs.size(); }
size_t numRefs() const { return NumRefs; }
bool empty() const { return Refs.empty(); }

size_t bytes() const {
Expand All @@ -431,11 +433,14 @@ class RefSlab {
};

private:
RefSlab(std::vector<value_type> Refs, llvm::BumpPtrAllocator Arena)
: Arena(std::move(Arena)), Refs(std::move(Refs)) {}
RefSlab(std::vector<value_type> Refs, llvm::BumpPtrAllocator Arena,
size_t NumRefs)
: Arena(std::move(Arena)), Refs(std::move(Refs)), NumRefs(NumRefs) {}

llvm::BumpPtrAllocator Arena;
std::vector<value_type> Refs;
// Number of all references.
size_t NumRefs = 0;
};

struct FuzzyFindRequest {
Expand Down
10 changes: 5 additions & 5 deletions clang-tools-extra/clangd/index/Serialization.cpp
Expand Up @@ -496,18 +496,18 @@ std::unique_ptr<SymbolIndex> loadIndex(llvm::StringRef SymbolFilename,
}
}

size_t SymSize = Symbols.size();
size_t RefSize = Refs.size();
size_t NumSym = Symbols.size();
size_t NumRefs = Refs.numRefs();

trace::Span Tracer("BuildIndex");
auto Index =
UseDex ? dex::Dex::build(std::move(Symbols), std::move(Refs), URISchemes)
: MemIndex::build(std::move(Symbols), std::move(Refs));
vlog("Loaded {0} from {1} with estimated memory usage {2} bytes\n"
" - number of symbos: {3}\n"
" - number of symbols: {3}\n"
" - number of refs: {4}\n",
UseDex ? "Dex" : "MemIndex", SymbolFilename,
Index->estimateMemoryUsage(),
SymSize, RefSize);
Index->estimateMemoryUsage(), NumSym, NumRefs);
return Index;
}

Expand Down

0 comments on commit 6ece6e7

Please sign in to comment.