[libcxx-commits] [PATCH] D110495: [libc++][format][2/6] Adds a __output_iterator.

[Victor Zverovich via Phabricator via libcxx-commits]

vitaut added inline comments.


================
Comment at: libcxx/include/__format/buffer.h:57-59
+        __flush_([](_CharT* __ptr, size_t __size, void* __o) {
+          static_cast<_Tp*>(__o)->flush(__ptr, __size);
+        }),
----------------
Mordante wrote:
> vitaut wrote:
> > As discussed in D112361, it would be good to avoid extra buffering for the common case of appending to a contiguous container such as string or vector.
> What do you mean with "appending"?
> `std::back_inserter(std::string)` or `std::string::begin()`
> 
> I think it makes sense to buffer the `back_inserter`. For example every `push_back` in a `std::string` NUL-terminates the string. The `insert` operation does one NUL-termination.
> 
> D110497 changes the behaviour for these containers to buffer in this object and then use `insert` in these containers.
> 
> But I can test with the benchmarks whether the extra buffering has a positive or negative effect.
> What do you mean with "appending"?

`std::back_inserter(std::string)`

> For example every push_back in a std::string NUL-terminates the string.

You should avoid character-by-character insertion in the first place. Also adding buffering for `std::string` is easy but you cannot remove it so the current approach is inherently less efficient than needed.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D110495