This revision was automatically updated to reflect the committed changes.
Closed by commit rCTE321272: [clangd] Index symbols share storage within a
slab. (authored by sammccall, committed by ).
Changed prior to commit:
https://reviews.llvm.org/D41483?vs=127877&id=127879#toc
Repository:
rCTE
sammccall updated this revision to Diff 127877.
sammccall marked 4 inline comments as done.
sammccall added a comment.
Comment changes requiested in review.
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D41483
Files:
clangd/index/Index.cpp
clangd/index/Index.h
unittests/c
sammccall marked an inline comment as done.
sammccall added inline comments.
Comment at: clangd/index/Index.h:136
+ // Intern table for strings. Not StringPool as we don't refcount, just
insert.
+ llvm::StringSet Strings;
llvm::DenseMap Symbols;
ilya-biryuk
hokein accepted this revision.
hokein added a comment.
This revision is now accepted and ready to land.
Nice, LGTM.
Comment at: clangd/index/Index.h:108
//
// FIXME: Use a space-efficient implementation, a lot of Symbol fields could
// share the same storage.
---
ilya-biryukov added inline comments.
Comment at: clangd/index/Index.h:136
+ // Intern table for strings. Not StringPool as we don't refcount, just
insert.
+ llvm::StringSet Strings;
llvm::DenseMap Symbols;
A comment on why we use `BumpPtrAllocator` here mig
sammccall updated this revision to Diff 127849.
sammccall added a comment.
Don't intern unless the symbol was actually inserted.
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D41483
Files:
clangd/index/Index.cpp
clangd/index/Index.h
unittests/clangd/FileIndexTests.cpp
u
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 bef