[clangd] Relation slabs should not be accounted when computing backing storage size

[sammccall]

Can you give a little context? Why not?

[ilya-golovenko]

In D92484#2430851, @sammccall wrote:

Can you give a little context? Why not?

It seems Relations should be not taken into account because they don't reference a backing storage.
Only SymStorage and RefsStorage are passed to the respective index in the Payload argument.

return std::make_unique<MemIndex>(
    llvm::make_pointee_range(AllSymbols), std::move(AllRefs),
    std::move(AllRelations),
    std::make_tuple(std::move(SymbolSlabs), std::move(RefSlabs),  <<<<
                    std::move(RefsStorage), std::move(SymsStorage)), <<<<
    StorageSize);

Also, there is a comment regarding Rels slab in Dex.cpp:

std::unique_ptr<SymbolIndex> Dex::build(SymbolSlab Symbols, RefSlab Refs, RelationSlab Rels) {
  auto Size = Symbols.bytes() + Refs.bytes();
  // There is no need to include "Rels" in Data because the relations are self-  <<<<<
  // contained, without references into a backing store.                         <<<<<
  auto Data = std::make_pair(std::move(Symbols), std::move(Refs));
  return std::make_unique<Dex>(Data.first, Data.second, Rels, std::move(Data),
                                Size);
}

MemIndex also uses only Symbols and Refs slabs to compute backing storage size.

[sammccall]

Thanks, you're right. There's no "payload" memory here, and MemIndex::estimateMemoryUsage should already be accounting for all the memory used.

(Sorry, it's been a while since I looked at this code)

[ilya-golovenko]

In D92484#2430985, @sammccall wrote:

Thanks, you're right. There's no "payload" memory here, and MemIndex::estimateMemoryUsage should already be accounting for all the memory used.

(Sorry, it's been a while since I looked at this code)

Thank you for review!

clang-tools-extra/clangd/index/FileIndex.cpp

AllRelations.erase(std::unique(AllRelations.begin(), AllRelations.end()),
                      AllRelations.end());
 
   size_t StorageSize =
       RefsStorage.size() * sizeof(Ref) + SymsStorage.size() * sizeof(Symbol);
   for (const auto &Slab : SymbolSlabs)
     StorageSize += Slab->bytes();
   for (const auto &RefSlab : RefSlabs)
     StorageSize += RefSlab->bytes();
-  for (const auto &RelationSlab : RelationSlabs)
-    StorageSize += RelationSlab->bytes();
 
   // Index must keep the slabs and contiguous ranges alive.
   switch (Type) {
   case IndexType::Light:
     return std::make_unique<MemIndex>(
         llvm::make_pointee_range(AllSymbols), std::move(AllRefs),
         std::move(AllRelations),
         std::make_tuple(std::move(SymbolSlabs), std::move(RefSlabs),