[PATCH] D67249: [Modules][PCH] Hash input files content

[Bruno Cardoso Lopes via Phabricator via cfe-commits]

bruno marked an inline comment as done.
bruno added inline comments.


================
Comment at: clang/lib/Serialization/ASTWriter.cpp:1767
   bool IsTopLevelModuleMap;
+  uint32_t ContentHash[2];
 };
----------------
ributzka wrote:
> bruno wrote:
> > aprantl wrote:
> > > Why is this not a uint64_t?
> > Serializing a `uint64_t` directly instead of two `uint32_t` gives me a slightly bigger final cache. One could argue that the value is negligible (+800 bytes for a 40MB cache), but here's the rationale :)
> Creating an abbrev might fix this, because this should be a fixed size field. The generic emission code for a record without an abbrev is not very space efficient for single fields.
Right, I did more experiments, and although VBR::64 or Fixed::64 aren't supported, I could shave off 10KB by using abbrev + two ::Fixed, 32. Using two ::VBR:32 is actually 400 bytes worse than the current approach. Thanks for the suggestion, gonna update the patch.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D67249