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

diff --git a/folly/experimental/io/IOBufQueue.cpp b/folly/experimental/io/IOBufQueue.cpp
index bca8a1ce675acf606378001174bd0beec5dbd99c..422121e7920bb8476a9cf13bf1d3a8607dc37800 100644 (file)
--- 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 2dfc7e794ad3f81534958b2c38362ace4422363e..b9c1bac912e93349c7057622a5495e25d6eced91 100644 (file)
--- 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 221beaaccae37d8df26ded81d7fa12991f20fe56..e769ab5ce1d97c80dcf94dd161a49aa25f1697ad 100644 (file)
--- 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) {