[PATCH] D88827: [llvm-objcopy][NFC] Move core implementation of llvm-objcopy into separate library.

[Alexey Lapshin via Phabricator via llvm-commits]

avl added a comment.

@alexshap @jhenderson @echristo

Folks, Before creating a patch I would like to consult what would be the best option to refactor the Buffer class.
One of the alternatives is described here https://reviews.llvm.org/D88827#2321871 ("Adapter" approach).

I think the better solution would be to use raw_ostream instead of buffers:

current:

  Error executeObjcopyOnBinary(const CopyConfig &Config,
                               object::ELFObjectFileBase &In, Buffer &Out);

new:

  Error executeObjcopyOnBinary(const CopyConfig &Config,
                               object::ELFObjectFileBase &In, raw_ostream &Out);

Generally, using streams could allow us to reduce memory usages. No need to load all data into the memory - the data could be streamed through a smaller buffer. Opposite, the current WritableMemoryBuffer, used in llvm-objcopy, would allocate whole data into the memory. Thus replacing WritableMemoryBuffer with raw_ostream
would allow minimizing memory requirements.

FileOutputBuffer(used by llvm-objcopy) has an advantage over raw_fd_ostream(which might be used if we would like 
to store data into the file). FileOutputBuffer::createOnDiskBuffer() allows to use memory mapped file. The similar functionality 
could be implemented for raw_fd_ostream. It is possible to add preallocate() method into raw_ostream.

class raw_ostream {

  void preallocate(uint64_t size);

}

That method, implemented for raw_fd_ostream, could create a memory-mapped file. The streamed data would be written
into that memory file then. Thus we would be able to use memory-mapped files with raw_fd_ostream.

So, it seems we could use raw_ostream instead of Buffer without losing functionality.
It seems to me that raw_ostream is a good abstraction here and it would be good to use it for llvm-objcopy.

So what is your opinion, would it be OK to use raw_ostream? 
Or should we use "Adapter" approach, from https://reviews.llvm.org/D88827#2321871 ?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D88827