[PATCH] D64312: [clangd] Simpler/more compact in-memory representation of RefSlab/Builder

[Sam McCall via Phabricator via llvm-commits]

sammccall planned changes to this revision.
sammccall marked 3 inline comments as done.
sammccall added inline comments.


================
Comment at: clang-tools-extra/clangd/index/Ref.cpp:36
 void RefSlab::Builder::insert(const SymbolID &ID, const Ref &S) {
-  auto &M = Refs[ID];
-  if (M.count(S))
-    return;
-  Ref R = S;
-  R.Location.FileURI =
-      UniqueStrings.save(R.Location.FileURI).data();
-  M.insert(std::move(R));
+  Refs.emplace_back(ID, S);
+  Refs.back().second.Location.FileURI =
----------------
hokein wrote:
> before this changes, we were doing deduplications on the fly, now we deduplicate them only at the final/reducing stage.
> 
> Note that with this patch, `clangd-indexer` will be OOMed when indexing large scale projects like chromium, but I think `clangd-indexer` is not a tool we'd recommend users to use, that's probably fine? 
Ugh, you're right - those are huge slabs with lots of duplication.

I mean, in principle we could document the limitation here and add a deduplicating map there (that site isn't very performance-sensitive). But that's kind of a mess. Let me think about this one.


Repository:
  rL LLVM

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D64312/new/

https://reviews.llvm.org/D64312