vsk accepted this revision. vsk added a comment. This revision is now accepted and ready to land.
LGTM. ================ Comment at: clang/lib/CodeGen/CoverageMappingGen.cpp:1393 + size_t CoverageMappingSize = 0; + for (auto &S : CoverageMappings) { + CoverageMappingSize += S.size(); ---------------- serge-sans-paille wrote: > vsk wrote: > > It doesn't look like the CoverageMappings std::vector is needed at all. > > Consider moving `FilenamesAndCoverageMappings` into > > CoverageMappingModuleGen? > > > > That should reduce the memory requirements a bit more. > It's not possible to directly store into `FilenamesAndCoverageMappings` > because filenames need to be stored before all mappings. It's possible to use > a `std::string` instead of a `std::vector` but it means that at some point, > memory usage will be `| CoverageMappings| x 2` while if we empty the vector > elements as we push them to the stream, we should keep `|CoverageMappings|`. > I've updated the diff to enforce this stream behavior. Ah, thanks for explaining. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D62623/new/ https://reviews.llvm.org/D62623 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits