Double buffering no more

CMakeLists.txt

@@ -101,7 +101,7 @@ if (CMAKE_CXX_COMPILER_ID MATCHES "GNU")
       -Wold-style-cast -Wundef
       -Wredundant-decls -Wwrite-strings -Wpointer-arith
       -Wcast-qual -Wformat=2 -Wmissing-include-dirs
-      -Wcast-align -Wnon-virtual-dtor
+      -Wcast-align
       -Wctor-dtor-privacy -Wdisabled-optimization
       -Winvalid-pch -Woverloaded-virtual
       -Wconversion -Wswitch-enum

include/fmt/compile.h

@@ -587,8 +587,7 @@ template <typename CompiledFormat, typename... Args,
 FMT_INLINE std::basic_string<Char> format(const CompiledFormat& cf,
                                           const Args&... args) {
   basic_memory_buffer<Char> buffer;
-  detail::buffer<Char>& base = buffer;
-  cf.format(std::back_inserter(base), args...);
+  cf.format(detail::buffer_appender<Char>(buffer), args...);
   return to_string(buffer);
 }
 

include/fmt/core.h

@@ -629,18 +629,18 @@ template <typename T> class buffer {
   T* ptr_;
   size_t size_;
   size_t capacity_;
-  bool fixed_;
 
  protected:
   // Don't initialize ptr_ since it is not accessed to save a few cycles.
   FMT_SUPPRESS_MSC_WARNING(26495)
-  buffer(size_t sz) FMT_NOEXCEPT : size_(sz), capacity_(sz), fixed_(false) {}
+  buffer(size_t sz) FMT_NOEXCEPT : size_(sz), capacity_(sz) {}
 
-  buffer(T* p = nullptr, size_t sz = 0, size_t cap = 0,
-         bool fixed = false) FMT_NOEXCEPT : ptr_(p),
-                                            size_(sz),
-                                            capacity_(cap),
-                                            fixed_(fixed) {}
+  buffer(T* p = nullptr, size_t sz = 0, size_t cap = 0) FMT_NOEXCEPT
+      : ptr_(p),
+        size_(sz),
+        capacity_(cap) {}
+
+  ~buffer() = default;
 
   /** Sets the buffer data and capacity. */
   void set(T* buf_data, size_t buf_capacity) FMT_NOEXCEPT {
@@ -657,7 +657,6 @@ template <typename T> class buffer {
 
   buffer(const buffer&) = delete;
   void operator=(const buffer&) = delete;
-  virtual ~buffer() = default;
 
   T* begin() FMT_NOEXCEPT { return ptr_; }
   T* end() FMT_NOEXCEPT { return ptr_ + size_; }
@@ -677,24 +676,26 @@ template <typename T> class buffer {
   /** Returns a pointer to the buffer data. */
   const T* data() const FMT_NOEXCEPT { return ptr_; }
 
-  /**
-    Resizes the buffer. If T is a POD type new elements may not be initialized.
-   */
-  void resize(size_t new_size) {
-    reserve(new_size);
-    size_ = new_size;
-  }
-
   /** Clears this buffer. */
   void clear() { size_ = 0; }
 
-  /** Reserves space to store at least *capacity* elements. */
-  void reserve(size_t new_capacity) {
+  // Tries resizing the buffer to contain *count* elements. If T is a POD type
+  // the new elements may not be initialized.
+  void try_resize(size_t count) {
+    try_reserve(count);
+    size_ = count <= capacity_ ? count : capacity_;
+  }
+
+  // Tries increasing the buffer capacity to *new_capacity*. It can increase the
+  // capacity by a smaller amount than requested but guarantees there is space
+  // for at least one additional element either by increasing the capacity or by
+  // flushing the buffer if it is full.
+  void try_reserve(size_t new_capacity) {
     if (new_capacity > capacity_) grow(new_capacity);
   }
 
   void push_back(const T& value) {
-    reserve(size_ + 1);
+    try_reserve(size_ + 1);
     ptr_[size_++] = value;
   }
 
@@ -707,12 +708,6 @@ template <typename T> class buffer {
   }
 };
 
-// A fixed capacity buffer.
-template <typename T> class fixed_buffer : buffer<T> {
- public:
-  fixed_buffer(T* data, size_t capacity) : buffer<T>(data, 0, capacity, true) {}
-};
-
 // A container-backed buffer.
 template <typename Container>
 class container_buffer : public buffer<typename Container::value_type> {
@@ -1815,7 +1810,10 @@ OutputIt vformat_to(
     OutputIt out, const S& format_str,
     basic_format_args<buffer_context<type_identity_t<Char>>> args) {
   auto& c = detail::get_container(out);
-  detail::container_buffer<remove_reference_t<decltype(c)>> buf(c);
+  using container = remove_reference_t<decltype(c)>;
+  typename std::conditional<
+      std::is_same<container, detail::buffer<Char>>::value,
+      detail::buffer<Char>&, detail::container_buffer<container>>::type buf(c);
   detail::vformat_to(buf, to_string_view(format_str), args);
   return out;
 }

include/fmt/format-inl.h

@@ -145,7 +145,7 @@ FMT_FUNC void format_error_code(detail::buffer<char>& out, int error_code,
   // Report error code making sure that the output fits into
   // inline_buffer_size to avoid dynamic memory allocation and potential
   // bad_alloc.
-  out.resize(0);
+  out.try_resize(0);
   static const char SEP[] = ": ";
   static const char ERROR_STR[] = "error ";
   // Subtract 2 to account for terminating null characters in SEP and ERROR_STR.
@@ -156,7 +156,7 @@ FMT_FUNC void format_error_code(detail::buffer<char>& out, int error_code,
     ++error_code_size;
   }
   error_code_size += detail::to_unsigned(detail::count_digits(abs_value));
-  auto it = std::back_inserter(out);
+  auto it = buffer_appender<char>(out);
   if (message.size() <= inline_buffer_size - error_code_size)
     format_to(it, "{}{}", message, SEP);
   format_to(it, "{}{}", ERROR_STR, error_code);
@@ -1051,7 +1051,7 @@ void fallback_format(Double d, buffer<char>& buf, int& exp10) {
         if (result > 0 || (result == 0 && (digit % 2) != 0))
           ++data[num_digits - 1];
       }
-      buf.resize(to_unsigned(num_digits));
+      buf.try_resize(to_unsigned(num_digits));
       exp10 -= num_digits - 1;
       return;
     }
@@ -1075,7 +1075,7 @@ int format_float(T value, int precision, float_specs specs, buffer<char>& buf) {
       buf.push_back('0');
       return 0;
     }
-    buf.resize(to_unsigned(precision));
+    buf.try_resize(to_unsigned(precision));
     std::uninitialized_fill_n(buf.data(), precision, '0');
     return -precision;
   }
@@ -1113,7 +1113,7 @@ int format_float(T value, int precision, float_specs specs, buffer<char>& buf) {
       fallback_format(value, buf, exp);
       return exp;
     }
-    buf.resize(to_unsigned(handler.size));
+    buf.try_resize(to_unsigned(handler.size));
   } else {
     if (precision > 17) return snprintf_float(value, precision, specs, buf);
     fp normalized = normalize(fp(value));
@@ -1131,7 +1131,7 @@ int format_float(T value, int precision, float_specs specs, buffer<char>& buf) {
         ++exp;
       }
     }
-    buf.resize(to_unsigned(num_digits));
+    buf.try_resize(to_unsigned(num_digits));
   }
   return exp - cached_exp10;
 }
@@ -1182,19 +1182,20 @@ int snprintf_float(T value, int precision, float_specs specs,
                      ? snprintf_ptr(begin, capacity, format, precision, value)
                      : snprintf_ptr(begin, capacity, format, value);
     if (result < 0) {
-      buf.reserve(buf.capacity() + 1);  // The buffer will grow exponentially.
+      // The buffer will grow exponentially.
+      buf.try_reserve(buf.capacity() + 1);
       continue;
     }
     auto size = to_unsigned(result);
     // Size equal to capacity means that the last character was truncated.
     if (size >= capacity) {
-      buf.reserve(size + offset + 1);  // Add 1 for the terminating '\0'.
+      buf.try_reserve(size + offset + 1);  // Add 1 for the terminating '\0'.
       continue;
     }
     auto is_digit = [](char c) { return c >= '0' && c <= '9'; };
     if (specs.format == float_format::fixed) {
       if (precision == 0) {
-        buf.resize(size);
+        buf.try_resize(size);
         return 0;
       }
       // Find and remove the decimal point.
@@ -1204,11 +1205,11 @@ int snprintf_float(T value, int precision, float_specs specs,
       } while (is_digit(*p));
       int fraction_size = static_cast<int>(end - p - 1);
       std::memmove(p, p + 1, to_unsigned(fraction_size));
-      buf.resize(size - 1);
+      buf.try_resize(size - 1);
       return -fraction_size;
     }
     if (specs.format == float_format::hex) {
-      buf.resize(size + offset);
+      buf.try_resize(size + offset);
       return 0;
     }
     // Find and parse the exponent.
@@ -1234,7 +1235,7 @@ int snprintf_float(T value, int precision, float_specs specs,
       fraction_size = static_cast<int>(fraction_end - begin - 1);
       std::memmove(begin + 1, begin + 2, to_unsigned(fraction_size));
     }
-    buf.resize(to_unsigned(fraction_size) + offset + 1);
+    buf.try_resize(to_unsigned(fraction_size) + offset + 1);
     return exp - fraction_size;
   }
 }
@@ -1373,7 +1374,8 @@ FMT_FUNC void format_system_error(detail::buffer<char>& out, int error_code,
       int result =
           detail::safe_strerror(error_code, system_message, buf.size());
       if (result == 0) {
-        format_to(std::back_inserter(out), "{}: {}", message, system_message);
+        format_to(detail::buffer_appender<char>(out), "{}: {}", message,
+                  system_message);
         return;
       }
       if (result != ERANGE)

include/fmt/format.h

@@ -374,7 +374,7 @@ reserve(std::back_insert_iterator<Container> it, size_t n) {
 template <typename T>
 inline buffer_appender<T> reserve(buffer_appender<T> it, size_t n) {
   buffer<T>& buf = get_container(it);
-  buf.reserve(buf.size() + n);
+  buf.try_reserve(buf.size() + n);
   return it;
 }
 
@@ -572,7 +572,7 @@ template <typename T>
 template <typename U>
 void buffer<T>::append(const U* begin, const U* end) {
   size_t new_size = size_ + to_unsigned(end - begin);
-  reserve(new_size);
+  try_reserve(new_size);
   std::uninitialized_copy(begin, end,
                           make_checked(ptr_ + size_, capacity_ - size_));
   size_ = new_size;
@@ -628,7 +628,7 @@ class basic_memory_buffer : public detail::buffer<T> {
   }
 
  protected:
-  void grow(size_t size) FMT_OVERRIDE;
+  void grow(size_t size) final;
 
  public:
   using value_type = T;
@@ -638,7 +638,7 @@ class basic_memory_buffer : public detail::buffer<T> {
       : alloc_(alloc) {
     this->set(store_, SIZE);
   }
-  ~basic_memory_buffer() FMT_OVERRIDE { deallocate(); }
+  ~basic_memory_buffer() { deallocate(); }
 
  private:
   // Move data from other to this buffer.
@@ -682,6 +682,15 @@ class basic_memory_buffer : public detail::buffer<T> {
 
   // Returns a copy of the allocator associated with this buffer.
   Allocator get_allocator() const { return alloc_; }
+
+  /**
+    Resizes the buffer to contain *count* elements. If T is a POD type new
+    elements may not be initialized.
+   */
+  void resize(size_t count) { this->try_resize(count); }
+
+  /** Increases the buffer capacity to *new_capacity*. */
+  void reserve(size_t new_capacity) { this->try_reserve(new_capacity); }
 };
 
 template <typename T, size_t SIZE, typename Allocator>

include/fmt/os.h

@@ -349,25 +349,26 @@ template <typename S, typename... Args>
 void print(direct_buffered_file& f, const S& format_str, const Args&... args);
 
 // A buffered file with a direct buffer access and no synchronization.
-class direct_buffered_file {
+class direct_buffered_file : private detail::buffer<char> {
  private:
   file file_;
 
-  enum { buffer_size = 4096 };
-  char buffer_[buffer_size];
-  int pos_;
+  char buffer_[BUFSIZ];
 
   void flush() {
-    if (pos_ == 0) return;
-    file_.write(buffer_, pos_);
-    pos_ = 0;
+    if (size() == 0) return;
+    file_.write(buffer_, size());
+    clear();
   }
 
-  int free_capacity() const { return buffer_size - pos_; }
+  int free_capacity() const { return static_cast<int>(BUFSIZ - size()); }
+
+ protected:
+  void grow(size_t) final;
 
  public:
-  direct_buffered_file(cstring_view path, int oflag)
-      : file_(path, oflag), pos_(0) {}
+  direct_buffered_file(const char* path, int oflag)
+      : buffer<char>(buffer_, 0, BUFSIZ), file_(path, oflag) {}
 
   ~direct_buffered_file() { flush(); }
 
@@ -379,21 +380,7 @@ class direct_buffered_file {
   template <typename S, typename... Args>
   friend void print(direct_buffered_file& f, const S& format_str,
                     const Args&... args) {
-    // We could avoid double buffering.
-    auto buf = fmt::memory_buffer();
-    fmt::format_to(std::back_inserter(buf), format_str, args...);
-    auto remaining_pos = 0;
-    auto remaining_size = buf.size();
-    while (remaining_size > detail::to_unsigned(f.free_capacity())) {
-      auto size = f.free_capacity();
-      memcpy(f.buffer_ + f.pos_, buf.data() + remaining_pos, size);
-      f.pos_ += size;
-      f.flush();
-      remaining_pos += size;
-      remaining_size -= size;
-    }
-    memcpy(f.buffer_ + f.pos_, buf.data() + remaining_pos, remaining_size);
-    f.pos_ += static_cast<int>(remaining_size);
+    fmt::format_to(detail::buffer_appender<char>(f), format_str, args...);
   }
 };
 #endif  // FMT_USE_FCNTL

include/fmt/ostream.h

@@ -103,7 +103,7 @@ void format_value(buffer<Char>& buf, const T& value,
 #endif
   output << value;
   output.exceptions(std::ios_base::failbit | std::ios_base::badbit);
-  buf.resize(buf.size());
+  buf.try_resize(buf.size());
 }
 
 // Formats an object of type T that has an overloaded ostream operator<<.

include/fmt/printf.h

@@ -181,7 +181,7 @@ template <typename Char> class printf_width_handler {
 template <typename Char, typename Context>
 void vprintf(buffer<Char>& buf, basic_string_view<Char> format,
              basic_format_args<Context> args) {
-  Context(std::back_inserter(buf), format, args).format();
+  Context(buffer_appender<Char>(buf), format, args).format();
 }
 }  // namespace detail
 
@@ -598,7 +598,7 @@ OutputIt basic_printf_context<OutputIt, Char>::format() {
 
 template <typename Char>
 using basic_printf_context_t =
-    basic_printf_context<std::back_insert_iterator<detail::buffer<Char>>, Char>;
+    basic_printf_context<detail::buffer_appender<Char>, Char>;
 
 using printf_context = basic_printf_context_t<char>;
 using wprintf_context = basic_printf_context_t<wchar_t>;

src/os.cc

@@ -124,7 +124,7 @@ void detail::format_windows_error(detail::buffer<char>& out, int error_code,
       if (result != 0) {
         utf16_to_utf8 utf8_message;
         if (utf8_message.convert(system_message) == ERROR_SUCCESS) {
-          format_to(std::back_inserter(out), "{}: {}", message, utf8_message);
+          format_to(buffer_appender<char>(out), "{}: {}", message, utf8_message);
           return;
         }
         break;
@@ -313,5 +313,9 @@ long getpagesize() {
   return size;
 #  endif
 }
+
+void direct_buffered_file::grow(size_t) {
+  if (this->size() == BUFSIZ) flush();
+}
 #endif  // FMT_USE_FCNTL
 FMT_END_NAMESPACE