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

Pavel Labath via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Fri Nov 20 00:25:37 PST 2020


labath added a comment.

In D91693#2405908 <https://reviews.llvm.org/D91693#2405908>, @avl wrote:

>> I second the separate class idea. It seems like it could be much cleaner. The normal treatment of reserve-like methods is that of a hint -- one that the implementation could ignore or adjust in some circumstances -- I wouldn't expect that calling this method would completely change the way in which a file is accessed, nor that writing "beyond" the reserved storage would result in an assertion...
>
> What do you think of WriteToStreamLibrayFunc use case? In that context reserve() is not just a hint, but it limits the size of resulting stream. Probably the method name is not good. Would resize() be better ?

Not really. :( For `resize`, I would expect that it has a physical effect on the underlying object (changing file size), which is good, I guess. But I would still expect that it is possible to write more data than that size, and have it be appended, as that's what our other streams do. I would also expect this operation to also have some effect on raw_string_ostream (changing the underlying string size) and such. And I am still worried about using mmap(2)/write(2) interchangeably. The two apis have different characteristics, and I don't think that can be conveyed by a single method call. E.g., the mmap approach will not respect O_APPEND, it will cause SIGBUS if the file is concurrently truncated (that's why our mmap APIs like MemoryBuffer have IsVolatile arguments), etc.

TBH, I am not sure that raw_ostream is the best class for this use case. How much of it's functionality do you need? If you don't need the formatted output capabilities, then maybe a different interface might be better suited. (I realize you're trying to remove one.)

In D91693#2402455 <https://reviews.llvm.org/D91693#2402455>, @avl wrote:

> Usage 2:
>
>   raw_fd_ostream?raw_mmap_ostream? Out();
>   WriteToStreamLibrayFunc (Out);

One way to get around that would be to make raw_mmap_ostream usable even without an explicit `reserve` call. So, if the user does not call `reserve` (or if he goes past the buffer he has already reserved), the stream would automatically mmap successive chunks of the file, and write to them.

That would make make the mmap approach harder to implement, but otoh, it would at least behave rougly as a normal stream. And the usage of a distinct class would make it clear that something funky is happening. Also, this approach might give you a performance boost (reduced copying) even for the cases where the size is not known ahead of time. We may not even need to add an new `reserve` method to the class, as we could have the class mmap buffer-sized chunks, and the code could adjust the buffer size when the size is known.....


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91693



More information about the llvm-commits mailing list