[llvm-branch-commits] [libcxx] [libc++][format][3/7] Improves std::format performance. (PR #101817)

[Louis Dionne via llvm-branch-commits]

================
@@ -499,11 +652,78 @@ struct _LIBCPP_TEMPLATE_VIS __format_to_n_buffer final
   _LIBCPP_HIDE_FROM_ABI auto __make_output_iterator() { return this->__output_.__make_output_iterator(); }
 
   _LIBCPP_HIDE_FROM_ABI format_to_n_result<_OutIt> __result() && {
-    this->__output_.__flush();
+    this->__output_.__flush(0);
     return {std::move(this->__writer_).__out_it(), this->__size_};
   }
 };
 
+// ***** ***** ***** LLVM-20 classes ***** ***** *****
+
+// A dynamically growing buffer.
+template <__fmt_char_type _CharT>
+class _LIBCPP_TEMPLATE_VIS __allocating_buffer : public __output_buffer<_CharT> {
+public:
+  __allocating_buffer(const __allocating_buffer&)            = delete;
+  __allocating_buffer& operator=(const __allocating_buffer&) = delete;
+
+  [[nodiscard]] _LIBCPP_HIDE_FROM_ABI __allocating_buffer()
+      : __output_buffer<_CharT>{__buffer_, __buffer_size_, __prepare_write} {}
+
+  _LIBCPP_HIDE_FROM_ABI ~__allocating_buffer() {
+    if (__ptr_ != __buffer_) {
+      ranges::destroy_n(__ptr_, this->__size());
+      allocator_traits<_Alloc>::deallocate(__alloc_, __ptr_, this->__capacity());
+    }
+  }
+
+  [[nodiscard]] _LIBCPP_HIDE_FROM_ABI basic_string_view<_CharT> __view() { return {__ptr_, this->__size()}; }
+
+private:
+  // At the moment the allocator is hard-code. There might be reasons to have
+  // an allocator trait in the future. This ensures forward compatibility.
+  using _Alloc = allocator<_CharT>;
+  _LIBCPP_NO_UNIQUE_ADDRESS _Alloc __alloc_;
+
+  // Since allocating is expensive the class has a small internal buffer. When
+  // its capacity is exceeded a dynamic buffer will be allocated.
+  static constexpr size_t __buffer_size_ = 256;
+  _CharT __buffer_[__buffer_size_];
----------------
ldionne wrote:

After looking at the patches later in the series, I am not certain these comments about implementing this as `vector` (i.e. elements beyond `size()` not having their lifetime started yet) is the best approach. I think your current approach may be better.

However, I think that the usage of `back_insert_iterator<__output_buffer>` + `__output_buffer::push_back` is misleading -- that's what caused me to think this should behave as a `vector` when in reality this is a different beast. If instead we had a method called `__push_back` (or `__insert_back`) and we used our own small iterator type to output to the buffer, it would make the design of this class clearer. It would also put `__push_back`/`__insert_back` on the same level as the other "insertion" functions like `__fill` and others. So IMO the current design might be fine lifetime-wise, but I think it would be nice to avoid using `back_insert_iterator<__output_buffer>` since `__output_buffer` is fundamentally not a container.

I understand that using `back_inserter` greatly simplifies the boilerplate needed, but IMO that's still the right design.

https://github.com/llvm/llvm-project/pull/101817