[PATCH] D23836: [ThinLTO/gold] Add caching support to gold-plugin

[Mehdi AMINI via llvm-commits]

mehdi_amini added inline comments.

================
Comment at: llvm/trunk/tools/gold/gold-plugin.cpp:812
@@ -801,1 +811,3 @@
+          *LTOOutput(OutputName).getStream() << Buffer->getBuffer();
+        });
   };
----------------
tejohnson wrote:
> mehdi_amini wrote:
> > mehdi_amini wrote:
> > > I just noticed: this is largely suboptimal, the Buffer will be a mmap'd file in memory, and you're duplicating it here.
> > I didn't notice earlier because the libLTO interface returns a pointer to the memory to the linker, so the MemoryBuffer is untouched till the linker needs it (it does not need to be paged in memory, which can contributes to explains why you have much more memory pressure)
> > I just noticed: this is largely suboptimal, the Buffer will be a mmap'd file in memory, and you're duplicating it here.
> 
> Dumb question - where is the duplication happening? getBuffer will give back a StringRef to the memory pointed to by the MemoryBuffer. And we then stream it to an output file (which we would have done earlier without caching).
The StringRef points to the memory mapped from the cache. So this is data on disk that you load in memory while you access it here.
The duplication is that you're creating a new file, while there is already a file on disk, this wouldn't happen in the libLTO implementation.



Repository:
  rL LLVM

https://reviews.llvm.org/D23836