[PATCH] D91693: [Support] Add reserve() method to the raw_ostream.

[Alexey Lapshin via Phabricator via llvm-commits]

avl added a comment.

@jhenderson Summary of proposed approach:

1. Current solution, which uses Buffer/MemoryBuffer/FileBuffer assumes that the buffer for the whole file should be pre-allocated. This assumes that the whole file should be loaded into the memory. For the library, it might be an inconvenient requirement. Some tools might want to have a possibility to reduce memory usage.

  Using streams allows us to reduce memory usages. No need to load all data into the memory - the data could be streamed through a smaller buffer.

2. Passing file name for the executeObjcopyOnBinary might be inconvenient if we would like to have a possibility to replace destination.

  executeObjcopyOnBinary(CopyConfig &Config, object::Binary &In, StringRef OutName);

i.e. when we would like to write data not in the file but in the memory, or just discard output, 
or calculate the hash for output we might want to replace destination. That would be hard 
if the only file name is specified. Opposite, following design:

  executeObjcopyOnBinary(CopyConfig &Config, object::Binary &In, raw_ostream &Out);

allows to use various kinds of destinations;

  SmallVector<char, 0> Buffer;
  raw_svector_ostream MemStream(Buffer);
  executeObjcopyOnBinary(Config, In, Out);
  
  raw_fd_ostream Out(OutputFilename);
  executeObjcopyOnBinary(Config, In, Out);
  
  raw_null_ostream Out;
  executeObjcopyOnBinary(Config, In, Out);
  
  raw_sha1_ostream Out;
  executeObjcopyOnBinary(Config, In, Out);



3. Current objcopy implementation assumes that data is already pre-allocated. Thus, to have the advantage of using streams it should be rewritten in such a way that streams were used directly(without creating intermediate buffers).

4. If llvm-objcopy needs to use memory-mapped files then we might implement resizable raw_mmap_ostream. Though I do not know at the current moment whether the effective implementation of resizable memory-mapped file could be done.

5. The fact that llvm-objcopy knows the size of resulting data could be used to make implementation a bit more efficient. We could tell reserve() for the streams and the more effective buffers could be used:



  SmallVector<char, 0> Buffer;
  raw_svector_ostream MemStream(Buffer);
  executeObjcopyOnBinary(Config, In, Out);
  
  executeObjcopyOnBinary (Config, In, Out) {
    Out.reserve(XX);   /// <<< calls Buffer.reserve(XX); 
  }

Finally, we can support all current functionality and have the advantages of using streams.   
So my current suggestion is

1. to use raw_ostream for output parameter of executeObjcopyOnBinary interface.
2. add reserve() method to the raw_ostream, so that streams were able to optimize internal buffers if possible.

What do you think?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91693