sammccall created this revision.
sammccall added a reviewer: hokein.
Herald added subscribers: cfe-commits, ilya-biryukov, klimek.
Symbols are not self-contained - it's only safe to hand them out if you
guarantee the lifetime of the underlying data.
Before this lands, I'm going to measure the before/after memory usage of the
LLVM index loaded into memory in a single slab.
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D41483
Files:
clangd/index/Index.cpp
clangd/index/Index.h
unittests/clangd/FileIndexTests.cpp
unittests/clangd/IndexTests.cpp
unittests/clangd/SymbolCollectorTests.cpp
Index: unittests/clangd/SymbolCollectorTests.cpp
===================================================================
--- unittests/clangd/SymbolCollectorTests.cpp
+++ unittests/clangd/SymbolCollectorTests.cpp
@@ -33,7 +33,7 @@
// GMock helpers for matching Symbol.
MATCHER_P(QName, Name, "") {
return (arg.second.Scope + (arg.second.Scope.empty() ? "" : "::") +
- arg.second.Name) == Name;
+ arg.second.Name).str() == Name;
}
namespace clang {
Index: unittests/clangd/IndexTests.cpp
===================================================================
--- unittests/clangd/IndexTests.cpp
+++ unittests/clangd/IndexTests.cpp
@@ -75,7 +75,8 @@
std::vector<std::string> Matches;
auto Ctx = Context::empty();
I.fuzzyFind(Ctx, Req, [&](const Symbol &Sym) {
- Matches.push_back(Sym.Scope + (Sym.Scope.empty() ? "" : "::") + Sym.Name);
+ Matches.push_back(
+ (Sym.Scope + (Sym.Scope.empty() ? "" : "::") + Sym.Name).str());
});
return Matches;
}
Index: unittests/clangd/FileIndexTests.cpp
===================================================================
--- unittests/clangd/FileIndexTests.cpp
+++ unittests/clangd/FileIndexTests.cpp
@@ -93,7 +93,8 @@
std::vector<std::string> Matches;
auto Ctx = Context::empty();
I.fuzzyFind(Ctx, Req, [&](const Symbol &Sym) {
- Matches.push_back(Sym.Scope + (Sym.Scope.empty() ? "" : "::") + Sym.Name);
+ Matches.push_back(
+ (Sym.Scope + (Sym.Scope.empty() ? "" : "::") + Sym.Name).str());
});
return Matches;
}
Index: clangd/index/Index.h
===================================================================
--- clangd/index/Index.h
+++ clangd/index/Index.h
@@ -15,15 +15,16 @@
#include "llvm/ADT/DenseMap.h"
#include "llvm/ADT/Hashing.h"
#include "llvm/ADT/StringExtras.h"
+#include "llvm/ADT/StringSet.h"
#include <array>
#include <string>
namespace clang {
namespace clangd {
struct SymbolLocation {
// The absolute path of the source file where a symbol occurs.
- std::string FilePath;
+ llvm::StringRef FilePath;
// The 0-based offset to the first character of the symbol from the beginning
// of the source file.
unsigned StartOffset;
@@ -71,16 +72,19 @@
// The class presents a C++ symbol, e.g. class, function.
//
-// FIXME: instead of having own copy fields for each symbol, we can share
-// storage from SymbolSlab.
+// WARNING: Symbols do now own much of their underlying data - typically strings
+// are owned by a SymbolSlab. They should be treated as non-owning references.
+// Copies are shallow.
+// When adding new unowned data fields to Symbol, remember to update
+// SymbolSlab::insert to copy them to the slab's storage.
struct Symbol {
// The ID of the symbol.
SymbolID ID;
// The unqualified name of the symbol, e.g. "bar" (for "n1::n2::bar").
- std::string Name;
+ llvm::StringRef Name;
// The scope (e.g. namespace) of the symbol, e.g. "n1::n2" (for
// "n1::n2::bar").
- std::string Scope;
+ llvm::StringRef Scope;
// The symbol information, like symbol kind.
index::SymbolInfo SymInfo;
// The location of the canonical declaration of the symbol.
@@ -117,11 +121,17 @@
// operation is irreversible.
void freeze();
- void insert(Symbol S);
+ void insert(const Symbol& S);
private:
+ void intern(llvm::StringRef &S) {
+ S = S.empty() ? llvm::StringRef() : Strings.insert(S).first->getKey();
+ }
+
bool Frozen = false;
+ // Intern table for strings. Not StringPool as we don't refcount, just insert.
+ llvm::StringSet<llvm::BumpPtrAllocator> Strings;
llvm::DenseMap<SymbolID, Symbol> Symbols;
};
Index: clangd/index/Index.cpp
===================================================================
--- clangd/index/Index.cpp
+++ clangd/index/Index.cpp
@@ -38,9 +38,13 @@
void SymbolSlab::freeze() { Frozen = true; }
-void SymbolSlab::insert(Symbol S) {
+void SymbolSlab::insert(const Symbol &S) {
assert(!Frozen && "Can't insert a symbol after the slab has been frozen!");
- Symbols[S.ID] = std::move(S);
+ auto &Sym = Symbols[S.ID] = S;
+
+ intern(Sym.Name);
+ intern(Sym.Scope);
+ intern(Sym.CanonicalDeclaration.FilePath);
}
} // namespace clangd
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
