[PATCH] D99110: [Coverage] Load records immediately

[Vedant Kumar via Phabricator via llvm-commits]

vsk added a comment.

Have you determined why opening a readonly binary costs memory?

I think I see the problem, have you tried disabling the RequiresNullTerminator bit?

  diff --git a/llvm/lib/ProfileData/Coverage/CoverageMapping.cpp b/llvm/lib/ProfileData/Coverage/CoverageMapping.cpp
  index cdbcde50d33a..f34e29397b3a 100644
  --- a/llvm/lib/ProfileData/Coverage/CoverageMapping.cpp
  +++ b/llvm/lib/ProfileData/Coverage/CoverageMapping.cpp
  @@ -318,11 +318,12 @@ CoverageMapping::load(ArrayRef<StringRef> ObjectFilenames,
     auto ProfileReader = std::move(ProfileReaderOrErr.get());
  
     SmallVector<std::unique_ptr<CoverageMappingReader>, 4> Readers;
     SmallVector<std::unique_ptr<MemoryBuffer>, 4> Buffers;
     for (const auto &File : llvm::enumerate(ObjectFilenames)) {
  -    auto CovMappingBufOrErr = MemoryBuffer::getFileOrSTDIN(File.value());
  +    auto CovMappingBufOrErr = MemoryBuffer::getFileOrSTDIN(
  +        File.value(), /*FileSize=*/-1, /*RequiresNullTerminator=*/false);
       if (std::error_code EC = CovMappingBufOrErr.getError())
         return errorCodeToError(EC);
       StringRef Arch = Arches.empty() ? StringRef() : Arches[File.index()];
       MemoryBufferRef CovMappingBufRef =
           CovMappingBufOrErr.get()->getMemBufferRef();

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.



================
Comment at: llvm/include/llvm/ProfileData/Coverage/CoverageMapping.h:586
 public:
+  CoverageMapping() = default;
   CoverageMapping(const CoverageMapping &) = delete;
----------------
Public APIs shouldn't invite the user to make invalid state representable (https://hugotunius.se/2020/05/16/making-invalid-state-unrepresentable.html).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D99110