Increase memory of BOLT runtime instrumentation bump allocator used for writing resulting profile

[Kobzol]

Summary

The BOLT instrumentation runtime uses a bump allocator that has a fixed amount of maximum memory. In some cases, this memory limit is not large enough (https://github.com/llvm/llvm-project/issues/59174). We are hitting this limit when instrumenting the Rust compiler with BOLT.

This change increases the memory of the bump allocator used for writing the resulting BOLT profile.

[rafauler]

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?

bolt/runtime/instr.cpp

196

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.

1465

Setting hash arena to 100MB

1473–1474

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.

1565

Note: setting global arena to 100MB

1638

This is just a clone of the other arena used for writing function profile, but for apple .

[Kobzol]

bolt/runtime/instr.cpp

1473–1474

Hmm, I wonder, since the arena size situation is a bit complex, maybe a simpler solution would just be to set this specific arena to 100 MiB too?

[rafauler]

bolt/runtime/instr.cpp

1473–1474

Yes, that makes sense

[Kobzol]

I removed the configuration and instead increased the size of the bump allocator that had the default size.

(I wasn't sure if I should upload a diff containing only the memory increase or if the diff should also "undo" the first diff. Hopefully I didn't mess this up).

[Kobzol]

Looks like I did mess up and I should have uploaded the complete diff without any relative changes to previous diffs. Did that now.

[Kobzol]

I checked that this change fixes our rustc issue, so the problem was indeed with this one specific bump allocator instance.

[rafauler]

Thanks!

[Kobzol]

Thank you! I'm not familiar with the landing process (and OFC I don't have merge rights) - do I have to do something else or will this eventually find its way into the main branch? :)

[Amir]

@Kobzol: "Jakub Beránek <[e-mail]>" as commit author line is OK?

[Kobzol]

Yes :)

bolt/runtime/instr.cpp

 #endif
 
   void clear() {
     Lock L(M);
     StackSize = 0;
   }
 
   /// Set mmap reservation size (only relevant before first allocation)
   void setMaxSize(uint64_t Size) { MaxSize = Size; }
 
   /// Set mmap reservation privacy (only relevant before first allocation)
   void setShared(bool S) { Shared = S; }
 
   void destroy() {
     if (StackBase == nullptr)
       return;
     __munmap(StackBase, MaxSize);

 ///
 extern "C" void __attribute((force_align_arg_pointer))
 __bolt_instr_data_dump() {
   // Already dumping
   if (!GlobalWriteProfileMutex->acquire())
     return;
 
   BumpPtrAllocator HashAlloc;
   HashAlloc.setMaxSize(0x6400000);
   ProfileWriterContext Ctx = readDescriptions();
   Ctx.CallFlowTable = new (HashAlloc, 0) CallFlowHashTable(HashAlloc);
 
   DEBUG(printStats(Ctx));
 
   int FD = openProfile();
 
   BumpPtrAllocator Alloc;
+  Alloc.setMaxSize(0x6400000);
   const uint8_t *FuncDesc = Ctx.FuncDescriptions;
   for (int I = 0, E = __bolt_instr_num_funcs; I < E; ++I) {
     FuncDesc = writeFunctionProfile(FD, Ctx, FuncDesc, Alloc);
     Alloc.clear();
     DEBUG(reportNumber("FuncDesc now: ", (uint64_t)FuncDesc, 16));
   }
   assert(FuncDesc == (void *)Ctx.Strings,
          "FuncDesc ptr must be equal to stringtable");
extern "C" void __attribute((force_align_arg_pointer)) __bolt_instr_setup() {
   // forked processes
   __mmap(CountersStart, CountersEnd - CountersStart,
          0x3 /*PROT_READ|PROT_WRITE*/,
          0x31 /*MAP_ANONYMOUS | MAP_SHARED | MAP_FIXED*/, -1, 0);
 
   __bolt_ind_call_counter_func_pointer = __bolt_instr_indirect_call;
   __bolt_ind_tailcall_counter_func_pointer = __bolt_instr_indirect_tailcall;
   // Conservatively reserve 100MiB shared pages
   GlobalAlloc.setMaxSize(0x6400000);
   GlobalAlloc.setShared(true);
   GlobalWriteProfileMutex = new (GlobalAlloc, 0) Mutex();
   if (__bolt_instr_num_ind_calls > 0)
     GlobalIndCallCounters =
         new (GlobalAlloc, 0) IndirectCallHashTable[__bolt_instr_num_ind_calls];
 
   if (__bolt_instr_sleep_time != 0) {
     // Separate instrumented process to the own process group

 #endif
 
 #if defined(__APPLE__)
 
 extern "C" void __bolt_instr_data_dump() {
   ProfileWriterContext Ctx = readDescriptions();
 
   int FD = 2;
   BumpPtrAllocator Alloc;
   const uint8_t *FuncDesc = Ctx.FuncDescriptions;
   uint32_t bolt_instr_num_funcs = _bolt_instr_num_funcs_getter();
 
   for (int I = 0, E = bolt_instr_num_funcs; I < E; ++I) {
     FuncDesc = writeFunctionProfile(FD, Ctx, FuncDesc, Alloc);
     Alloc.clear();
     DEBUG(reportNumber("FuncDesc now: ", (uint64_t)FuncDesc, 16));
   }