[PATCH] D151891: Allow configuring BOLT runtime instrumentation library max. allocation size

[Rafael Auler via Phabricator via llvm-commits]

rafauler added a comment.

Thanks for the patch. This change is mostly OK, except that at the moment the arena size is not very simple. We actually have 3 different arenas during runtime, and by changing the default, you're changing the size of just one of them. I've highlighted in the code the different allocation arenas that we use. It would be weird to have a parameter that only changes the size of a single arena while we have two others that might overflow. What happens if we remove the calls to setMaxSize on the 2 other arenas, so all of them have the same space, the one you're parameterizing? Do we have a slowdown?



================
Comment at: bolt/runtime/instr.cpp:196
   /// Set mmap reservation size (only relevant before first allocation)
   void setMaxSize(uint64_t Size) { MaxSize = Size; }
 
----------------
Note: This is used to change the arena size. We have two other arenas that actually specify a custom size of 100MB using "setMaxSize", so you are overriding the size of just one of the 3 arenas.


================
Comment at: bolt/runtime/instr.cpp:1465
   BumpPtrAllocator HashAlloc;
   HashAlloc.setMaxSize(0x6400000);
   ProfileWriterContext Ctx = readDescriptions();
----------------
Setting hash arena to 100MB


================
Comment at: bolt/runtime/instr.cpp:1473
 
   BumpPtrAllocator Alloc;
   const uint8_t *FuncDesc = Ctx.FuncDescriptions;
----------------
Note: This is the only arena that does not call setMaxSize, so that's the one you're parameterizing. It is used to write function profile.


================
Comment at: bolt/runtime/instr.cpp:1564
   // Conservatively reserve 100MiB shared pages
   GlobalAlloc.setMaxSize(0x6400000);
   GlobalAlloc.setShared(true);
----------------
Note: setting global arena to 100MB


================
Comment at: bolt/runtime/instr.cpp:1637
   int FD = 2;
   BumpPtrAllocator Alloc;
   const uint8_t *FuncDesc = Ctx.FuncDescriptions;
----------------
This is just a clone of the other arena used for writing function profile, but for __apple__ . 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D151891