[PATCH] D76594: [clang][AST] Support AST files larger than 512M

Mon Mar 30 12:30:22 PDT 2020

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

================
Comment at: clang/include/clang/Serialization/ASTBitCodes.h:220
     /// Source range/offset of a preprocessed entity.
     struct DeclOffset {
+      /// Raw source location. The unsigned i.e. 32-bit integer is enough for
----------------
sammccall wrote:
> Is there one of these for every decl in the module? It seems like we're probably giving up a good fraction of the 4% increase for just using absolute 64 bit offsets everywhere :-( Is there still a significant gain from using section-relative elsewhere?
> 
> If there are indeed lots of these, giving up 4 bytes to padding (in addition to the wide offset) seems unfortunate and because we memcpy the structs into the AST file seems like a sad reason :-)
> Can we align this to 4 bytes instead?
> (e.g. by splitting into two fields and encapsulating the few direct accesses, though there's probably a neater way)
Yes, it seems that all referenced decls should have an entry in this table. 4% increase was counted on previous diff before I discovered DeclOffsets and TypeOffsets. This diff uses relative offsets for previous cases and increase is only 2 additional 64-bit integers per file. DeclOffsets are about 4.7% and TypeOffsets are about 2.5% of the whole file. With this diff both number will be 2x, splitting DeclOffset in two separate arrays could make the increase only 1.5x for DeclOffsets. If approach from this diff looks fine, I can split array of DeclOffset into 2 separate arrays. It could be a bit less efficient because of worse memory locality but will save some space.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76594