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

Tue Dec 22 14:50:08 PST 2020

dblaikie added a comment.

In D91693#2425585 <https://reviews.llvm.org/D91693#2425585>, @jhenderson wrote:

> In D91693#2425369 <https://reviews.llvm.org/D91693#2425369>, @avl wrote:
>
>> @jhenderson James, what do you think of using streams as suggested by D91028 <https://reviews.llvm.org/D91028> and D91693 <https://reviews.llvm.org/D91693>? Would it be useful? It seems it would reduce amount of copied data.
>
> Hi @avl,
>
> Honestly, I don't have the time to look at this process in detail, and refactoring things to support an objcopy library is not high on my priority list. I'm not convinced that your suggestions are actually going to be workable/useful in practice, if I'm honest, and have tried to outline my concerns already. My biggest concern is how do you stream an ELF header without already knowing where your section header table will live. If you know where your section header table will live, you have all the information you need for presizing your output buffer, so being able to reserve post stream creation becomes pointless.

I guess that depends on the API - if the API is "write this object to this stream" then that API implementation has a "Oh, I know how big my output is, I can pass that hint to the stream and, if it has use for that (such as presizing a memory mapped output file to back storage) it can do that" - if the API is defined more in terms of "write this object to the file specified by this file name", then, yeah, you can change the logic about how the file is opened in the first place - presize, open memory mapped, and use a MemoryBuffer instead of a stream, potentially.

But a stream API is more general - allows for streaming out to stdout or other places that can't be identified by a file name, for instance.

@avl - it might be this and the related patch/usage in llvm-objcopy deserve an llvm-dev design thread about the overall issues you're trying to solve and the design directions you've explored and the ones you are proposing.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91693