[PATCH] D99110: [Coverage] Load records immediately

[Vedant Kumar via Phabricator via llvm-commits]

vsk added a comment.

>> This should prevent the MemoryBuffer API from allocating a buffer for the object file, just to add a '\0' at the end. If the OS can just mmap() the buffer readonly, that should be essentially free.
>
> Yes, I have tried disabling the RequiresNullTerminator bit, but it didn't help. Actually, the readonly buffer itself was fine. There are some places reading and storing data from them.

I'm not sure where the memory problem is. From your patch summary, it sounded like it was from the open objectfile buffers, but it sounds like that's not the case. This is confusing by itself, because without the RequiresNullTerminator change, I'd expect a large part of the memory cost to show up here.

It would help if you could pause the program while it's at peak RSS and share the results from a memory profiler, so we can see what's taking up the most space on the heap.

>   std::string FuncRecords;
>   ...
>   for (SectionRef Section : *CoverageRecordsSections) {
>     auto CoverageRecordsOrErr = Section.getContents();
>     if (!CoverageRecordsOrErr)
>       return CoverageRecordsOrErr.takeError();
>     FuncRecords += CoverageRecordsOrErr.get();
>     while (FuncRecords.size() % 8 != 0)
>       FuncRecords += '\0';
>   }
>
> FuncRecords is stored as a new string. When I removed this part, I could save about 10-20% of memory. And, there are some other similar things such as uncompressed data. So, I just came back to this approach.

I don't understand what you replaced this with. For some context: this logic is only needed for non-MachO (non-Darwin) file formats, because the use of comdats splits function records into multiple sections. See 80cd518b809dd <https://reviews.llvm.org/rG80cd518b809ddb3caaf1dc59db3fbc6d1bcaf2e0>. Looking at this some more, I admit it's possible to optimize, but I don't see how this patch does it.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D99110