Commit c462b064 authored by Alan Frindell's avatar Alan Frindell Committed by Jordan DeLong

Fix IOBufQueue::preallocate for callers who really wanted a hard max

Summary: D633912 broke unicorn because it relied on preallocate having a hard max.  Add an optional hard max parameter and rename maxHint to newAllocationSize.  Pass the hard max in for unicorn

Test Plan: folly and unicorn unit tests

Reviewed By: tudorb@fb.com

FB internal diff: D634894
parent 01e1af4a
...@@ -161,23 +161,24 @@ IOBufQueue::wrapBuffer(const void* buf, size_t len, uint32_t blockSize) { ...@@ -161,23 +161,24 @@ IOBufQueue::wrapBuffer(const void* buf, size_t len, uint32_t blockSize) {
} }
pair<void*,uint32_t> pair<void*,uint32_t>
IOBufQueue::preallocate(uint32_t min, uint32_t maxHint) { IOBufQueue::preallocate(uint32_t min, uint32_t newAllocationSize,
uint32_t max) {
if (head_ != NULL) { if (head_ != NULL) {
// If there's enough space left over at the end of the queue, use that. // If there's enough space left over at the end of the queue, use that.
IOBuf* last = head_->prev(); IOBuf* last = head_->prev();
if (!last->isSharedOne()) { if (!last->isSharedOne()) {
uint32_t avail = last->tailroom(); uint32_t avail = last->tailroom();
if (avail >= min) { if (avail >= min) {
return make_pair(last->writableTail(), avail); return make_pair(last->writableTail(), std::min(max, avail));
} }
} }
} }
// Allocate a new buffer of the requested max size. // Allocate a new buffer of the requested max size.
unique_ptr<IOBuf> newBuf(IOBuf::create(std::max(min, maxHint))); unique_ptr<IOBuf> newBuf(IOBuf::create(std::max(min, newAllocationSize)));
appendToChain(head_, std::move(newBuf)); appendToChain(head_, std::move(newBuf));
IOBuf* last = head_->prev(); IOBuf* last = head_->prev();
return make_pair(last->writableTail(), return make_pair(last->writableTail(),
last->tailroom() /* may exceed maxHint */); std::min(max, last->tailroom()));
} }
void void
......
...@@ -115,9 +115,10 @@ class IOBufQueue { ...@@ -115,9 +115,10 @@ class IOBufQueue {
* Obtain a writable block of contiguous bytes at the end of this * Obtain a writable block of contiguous bytes at the end of this
* queue, allocating more space if necessary. The amount of space * queue, allocating more space if necessary. The amount of space
* reserved will be at least min. If min contiguous space is not * 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 * available at the end of the queue, and IOBuf with size newAllocationSize
* appended to the chain and returned. The actual available space * is appended to the chain and returned. The actual available space
* may be larger than maxHint. * may be larger than newAllocationSize, but will be truncated to max,
* if specified.
* *
* If the caller subsequently writes anything into the returned space, * If the caller subsequently writes anything into the returned space,
* it must call the postallocate() method. * it must call the postallocate() method.
...@@ -130,7 +131,9 @@ class IOBufQueue { ...@@ -130,7 +131,9 @@ class IOBufQueue {
* callback, tell the application how much of the buffer they've * callback, tell the application how much of the buffer they've
* filled with data. * filled with data.
*/ */
std::pair<void*,uint32_t> preallocate(uint32_t min, uint32_t maxHint); std::pair<void*,uint32_t> preallocate(
uint32_t min, uint32_t newAllocationSize,
uint32_t max = std::numeric_limits<uint32_t>::max());
/** /**
* Tell the queue that the caller has written data into the first n * Tell the queue that the caller has written data into the first n
......
...@@ -149,10 +149,11 @@ TEST(IOBufQueue, Split) { ...@@ -149,10 +149,11 @@ TEST(IOBufQueue, Split) {
TEST(IOBufQueue, Preallocate) { TEST(IOBufQueue, Preallocate) {
IOBufQueue queue(clOptions); IOBufQueue queue(clOptions);
queue.append(string("Hello")); queue.append(string("Hello"));
pair<void*,uint32_t> writable = queue.preallocate(2, 64); pair<void*,uint32_t> writable = queue.preallocate(2, 64, 64);
checkConsistency(queue); checkConsistency(queue);
EXPECT_NE((void*)NULL, writable.first); EXPECT_NE((void*)NULL, writable.first);
EXPECT_LE(2, writable.second); EXPECT_LE(2, writable.second);
EXPECT_GE(64, writable.second);
memcpy(writable.first, SCL(", ")); memcpy(writable.first, SCL(", "));
queue.postallocate(2); queue.postallocate(2);
checkConsistency(queue); checkConsistency(queue);
...@@ -160,15 +161,18 @@ TEST(IOBufQueue, Preallocate) { ...@@ -160,15 +161,18 @@ TEST(IOBufQueue, Preallocate) {
queue.append(SCL("World")); queue.append(SCL("World"));
checkConsistency(queue); checkConsistency(queue);
EXPECT_EQ(12, queue.front()->computeChainDataLength()); EXPECT_EQ(12, queue.front()->computeChainDataLength());
writable = queue.preallocate(1024, 4096); // There are not 2048 bytes available, this will alloc a new buf
writable = queue.preallocate(2048, 4096);
checkConsistency(queue); checkConsistency(queue);
EXPECT_LE(1024, writable.second); EXPECT_LE(2048, writable.second);
// IOBuf allocates more than newAllocationSize, and we didn't cap it
EXPECT_GE(writable.second, 4096);
queue.postallocate(writable.second); queue.postallocate(writable.second);
// queue has no empty space, make sure we allocate at least min, even if // queue has no empty space, make sure we allocate at least min, even if
// maxHint < min // newAllocationSize < min
writable = queue.preallocate(1024, 1); writable = queue.preallocate(1024, 1, 1024);
checkConsistency(queue); checkConsistency(queue);
EXPECT_LE(1024, writable.second); EXPECT_EQ(1024, writable.second);
} }
TEST(IOBufQueue, Wrap) { TEST(IOBufQueue, Wrap) {
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment