From 01e1af4ac5b08d57c51a3c647e3b9844458abde8 Mon Sep 17 00:00:00 2001
From: Alan Frindell <afrind@fb.com>
Date: Fri, 16 Nov 2012 15:49:56 -0800
Subject: [PATCH] Return the maximum available space from
 IOBufQueue::preallocate

Summary: IOBufQueue::preallocate currently takes a min and max size, and will return to the caller a buffer with N writable bytes with min <= N <= max.  This is a bit wasteful, since there may be more than max bytes available at the end if the queue with no extra cost.  Now preallocate will return the full amount of contigious space to the caller, even if it is larger than the requested max.

Test Plan: folly and proxygen unit tests

Reviewed By: simpkins@fb.com

FB internal diff: D633912
---
 folly/experimental/io/IOBufQueue.cpp          | 9 ++++-----
 folly/experimental/io/IOBufQueue.h            | 9 +++++----
 folly/experimental/io/test/IOBufQueueTest.cpp | 8 ++++++--
 3 files changed, 15 insertions(+), 11 deletions(-)

diff --git a/folly/experimental/io/IOBufQueue.cpp b/folly/experimental/io/IOBufQueue.cpp
index bca8a1ce..422121e7 100644
--- a/folly/experimental/io/IOBufQueue.cpp
+++ b/folly/experimental/io/IOBufQueue.cpp
@@ -161,24 +161,23 @@ IOBufQueue::wrapBuffer(const void* buf, size_t len, uint32_t blockSize) {
 }
 
 pair<void*,uint32_t>
-IOBufQueue::preallocate(uint32_t min, uint32_t max) {
+IOBufQueue::preallocate(uint32_t min, uint32_t maxHint) {
   if (head_ != NULL) {
     // If there's enough space left over at the end of the queue, use that.
     IOBuf* last = head_->prev();
     if (!last->isSharedOne()) {
       uint32_t avail = last->tailroom();
       if (avail >= min) {
-        return make_pair(
-            last->writableTail(), std::min(max, avail));
+        return make_pair(last->writableTail(), avail);
       }
     }
   }
   // Allocate a new buffer of the requested max size.
-  unique_ptr<IOBuf> newBuf(IOBuf::create(max));
+  unique_ptr<IOBuf> newBuf(IOBuf::create(std::max(min, maxHint)));
   appendToChain(head_, std::move(newBuf));
   IOBuf* last = head_->prev();
   return make_pair(last->writableTail(),
-      std::min(max, last->tailroom()));
+                   last->tailroom() /* may exceed maxHint */);
 }
 
 void
diff --git a/folly/experimental/io/IOBufQueue.h b/folly/experimental/io/IOBufQueue.h
index 2dfc7e79..b9c1bac9 100644
--- a/folly/experimental/io/IOBufQueue.h
+++ b/folly/experimental/io/IOBufQueue.h
@@ -114,9 +114,10 @@ class IOBufQueue {
   /**
    * Obtain a writable block of contiguous bytes at the end of this
    * queue, allocating more space if necessary.  The amount of space
-   * reserved will be between min and max, inclusive; the IOBufQueue
-   * implementation may pick a value in that range that makes efficient
-   * use of already-allocated internal space.
+   * reserved will be at least min.  If min contiguous space is not
+   * available at the end of the queue, and IOBuf with size maxHint is
+   * appended to the chain and returned.  The actual available space
+   * may be larger than maxHint.
    *
    * If the caller subsequently writes anything into the returned space,
    * it must call the postallocate() method.
@@ -129,7 +130,7 @@ class IOBufQueue {
    *       callback, tell the application how much of the buffer they've
    *       filled with data.
    */
-  std::pair<void*,uint32_t> preallocate(uint32_t min, uint32_t max);
+  std::pair<void*,uint32_t> preallocate(uint32_t min, uint32_t maxHint);
 
   /**
    * Tell the queue that the caller has written data into the first n
diff --git a/folly/experimental/io/test/IOBufQueueTest.cpp b/folly/experimental/io/test/IOBufQueueTest.cpp
index 221beaac..e769ab5c 100644
--- a/folly/experimental/io/test/IOBufQueueTest.cpp
+++ b/folly/experimental/io/test/IOBufQueueTest.cpp
@@ -153,7 +153,6 @@ TEST(IOBufQueue, Preallocate) {
   checkConsistency(queue);
   EXPECT_NE((void*)NULL, writable.first);
   EXPECT_LE(2, writable.second);
-  EXPECT_GE(64, writable.second);
   memcpy(writable.first, SCL(", "));
   queue.postallocate(2);
   checkConsistency(queue);
@@ -164,7 +163,12 @@ TEST(IOBufQueue, Preallocate) {
   writable = queue.preallocate(1024, 4096);
   checkConsistency(queue);
   EXPECT_LE(1024, writable.second);
-  EXPECT_GE(4096, writable.second);
+  queue.postallocate(writable.second);
+  // queue has no empty space, make sure we allocate at least min, even if
+  // maxHint < min
+  writable = queue.preallocate(1024, 1);
+  checkConsistency(queue);
+  EXPECT_LE(1024, writable.second);
 }
 
 TEST(IOBufQueue, Wrap) {
-- 
2.34.1