[libc-commits] [libc] [wip] [libc] Implement vasprintf (PR #98824)

Michael Jones via libc-commits libc-commits at lists.llvm.org
Fri Jul 19 10:49:32 PDT 2024


================
@@ -23,60 +23,73 @@ namespace printf_core {
 
 struct WriteBuffer {
   using StreamWriter = int (*)(cpp::string_view, void *);
+  using OverflowWriter = int (WriteBuffer::*)(cpp::string_view);
   char *buff;
-  const size_t buff_len;
+  size_t buff_len;
   size_t buff_cur = 0;
 
   // The stream writer will be called when the buffer is full. It will be passed
   // string_views to write to the stream.
   StreamWriter stream_writer;
   void *output_target;
+  OverflowWriter overflowWriter{nullptr};
 
   LIBC_INLINE WriteBuffer(char *Buff, size_t Buff_len, StreamWriter hook,
                           void *target)
       : buff(Buff), buff_len(Buff_len), stream_writer(hook),
-        output_target(target) {}
+        output_target(target), overflowWriter(&WriteBuffer::flush_to_stream) {}
 
   LIBC_INLINE WriteBuffer(char *Buff, size_t Buff_len)
       : buff(Buff), buff_len(Buff_len), stream_writer(nullptr),
-        output_target(nullptr) {}
+        output_target(nullptr),
+        overflowWriter(&WriteBuffer::fill_remaining_to_buff) {}
 
-  // The overflow_write method is intended to be called to write the contents of
-  // the buffer and new_str to the stream_writer if it exists, else it will
-  // write as much of new_str to the buffer as it can. The current position in
-  // the buffer will be reset iff stream_writer is called. Calling this with an
-  // empty string will flush the buffer if relevant.
-  LIBC_INLINE int overflow_write(cpp::string_view new_str) {
-    // If there is a stream_writer, write the contents of the buffer, then
-    // new_str, then clear the buffer.
-    if (stream_writer != nullptr) {
-      if (buff_cur > 0) {
-        int retval = stream_writer({buff, buff_cur}, output_target);
-        if (retval < 0) {
-          return retval;
-        }
+  LIBC_INLINE WriteBuffer(char *Buff, size_t Buff_len, StreamWriter hook)
+      : buff(Buff), buff_len(Buff_len), stream_writer(hook),
+        output_target(this), overflowWriter(&WriteBuffer::resize_and_write) {}
+
+  LIBC_INLINE int flush_to_stream(cpp::string_view new_str) {
+    if (buff_cur > 0) {
+      int retval = stream_writer({buff, buff_cur}, output_target);
+      if (retval < 0) {
+        return retval;
       }
-      if (new_str.size() > 0) {
-        int retval = stream_writer(new_str, output_target);
-        if (retval < 0) {
-          return retval;
-        }
+    }
+    if (new_str.size() > 0) {
+      int retval = stream_writer(new_str, output_target);
+      if (retval < 0) {
+        return retval;
       }
-      buff_cur = 0;
-      return WRITE_OK;
-    } else {
-      // We can't flush to the stream, so fill the rest of the buffer, then drop
-      // the overflow.
-      if (buff_cur < buff_len) {
-        size_t bytes_to_write = buff_len - buff_cur;
-        if (bytes_to_write > new_str.size()) {
-          bytes_to_write = new_str.size();
-        }
-        inline_memcpy(buff + buff_cur, new_str.data(), bytes_to_write);
-        buff_cur += bytes_to_write;
+    }
+    buff_cur = 0;
+    return WRITE_OK;
+  }
+
+  LIBC_INLINE int fill_remaining_to_buff(cpp::string_view new_str) {
+    if (buff_cur < buff_len) {
+      size_t bytes_to_write = buff_len - buff_cur;
+      if (bytes_to_write > new_str.size()) {
+        bytes_to_write = new_str.size();
       }
-      return WRITE_OK;
+      inline_memcpy(buff + buff_cur, new_str.data(), bytes_to_write);
+      buff_cur += bytes_to_write;
     }
+    return WRITE_OK;
+  }
+
+  LIBC_INLINE int resize_and_write(cpp::string_view new_str) {
+    return stream_writer(new_str, output_target);
+  }
+
+  // The overflow_write method is intended to be called to write the contents of
+  // the buffer and new_str to the stream_writer if it exists. If a resizing
+  // hook is provided, it will resize the buffer and write the contents. If
+  // neither a stream_writer nor a resizing hook is provided, it will fill the
+  // remaining space in the buffer with new_str and drop the overflow. Calling
+  // this with an empty string will flush the buffer if relevant.
+
+  LIBC_INLINE int overflow_write(cpp::string_view new_str) {
+    return (this->*overflowWriter)(new_str);
----------------
michaelrj-google wrote:

I like the idea of selecting the type of overflow write when creating the object, but I think it would be better to store the type as an enum and select it with a switch statement here instead of using a function pointer.

When a function pointer is called the compiler may not be able to determine where it points and so may not be able to perform some optimizations. Additionally the CPU's branch predictor may have a hard time with it.

Here's a little demonstration of this: https://godbolt.org/z/x65ehbnnj
You can see that in `test_foo` even though some of the code is inlined, there are still `call` instructions, whereas in `test_bar` it's managed to inline all of the code.

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


More information about the libc-commits mailing list