[PATCH] D51585: [clangd] Define a compact binary serialization fomat for symbol slab/index.

[Sam McCall via Phabricator via cfe-commits]

sammccall marked 2 inline comments as done.
sammccall added inline comments.


================
Comment at: clangd/index/Serialization.cpp:50
+
+void writeVar(uint32_t I, raw_ostream &OS) {
+  constexpr static uint8_t More = 1 << 7;
----------------
ioeric wrote:
> This function could use a comment. What's the difference between this and `write32`?
Added comment describing varint encoding.


================
Comment at: clangd/index/Serialization.cpp:96
+  std::vector<StringRef> Sorted;
+  DenseMap<std::pair<const char *, size_t>, unsigned> Index;
+
----------------
ioeric wrote:
> Any reason to use `std::pair<const char *, size_t>` instead of `StringRef`?
It's a performance hack: DenseMap<StringRef, T> does a lookup by content which requires hashing the string. We intern the strings as we gather them so there's no need to hash the string twice. Added a comment.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D51585