From 4155301015525a242a79b9b3134e66d366bc0ebd Mon Sep 17 00:00:00 2001
From: Monty Brandenberg <monty@lindenlab.com>
Date: Fri, 1 Jun 2012 21:30:45 -0400
Subject: [PATCH] Do some work on BufferArray to make it a bit less naive about
chunking data. Remove the stateful use of a seek pointer so that shared read
is possible (though maybe not interesting).
---
indra/llcorehttp/_httpoprequest.cpp | 14 +--
indra/llcorehttp/bufferarray.cpp | 126 ++++++++++++++------
indra/llcorehttp/bufferarray.h | 53 +-------
indra/llcorehttp/tests/test_bufferarray.hpp | 89 +++++---------
indra/newview/lltexturefetch.cpp | 3 +-
5 files changed, 133 insertions(+), 152 deletions(-)
diff --git a/indra/llcorehttp/_httpoprequest.cpp b/indra/llcorehttp/_httpoprequest.cpp
index 791ee5f8600..196011f9536 100644
--- a/indra/llcorehttp/_httpoprequest.cpp
+++ b/indra/llcorehttp/_httpoprequest.cpp
@@ -340,7 +340,6 @@ HttpStatus HttpOpRequest::prepareRequest(HttpService * service)
long data_size(0);
if (mReqBody)
{
- mReqBody->seek(0);
data_size = mReqBody->size();
}
curl_easy_setopt(mCurlHandle, CURLOPT_POSTFIELDS, static_cast<void *>(NULL));
@@ -356,7 +355,6 @@ HttpStatus HttpOpRequest::prepareRequest(HttpService * service)
long data_size(0);
if (mReqBody)
{
- mReqBody->seek(0);
data_size = mReqBody->size();
}
curl_easy_setopt(mCurlHandle, CURLOPT_INFILESIZE, data_size);
@@ -423,10 +421,8 @@ size_t HttpOpRequest::writeCallback(void * data, size_t size, size_t nmemb, void
op->mReplyBody = new BufferArray();
}
const size_t req_size(size * nmemb);
- char * lump(op->mReplyBody->appendBufferAlloc(req_size));
- memcpy(lump, data, req_size);
-
- return req_size;
+ const size_t write_size(op->mReplyBody->append(static_cast<char *>(data), req_size));
+ return write_size;
}
@@ -450,9 +446,9 @@ size_t HttpOpRequest::readCallback(void * data, size_t size, size_t nmemb, void
}
const size_t do_size((std::min)(req_size, body_size - op->mCurlBodyPos));
- op->mReqBody->read(static_cast<char *>(data), do_size);
- op->mCurlBodyPos += do_size;
- return do_size;
+ const size_t read_size(op->mReqBody->read(op->mCurlBodyPos, static_cast<char *>(data), do_size));
+ op->mCurlBodyPos += read_size;
+ return read_size;
}
diff --git a/indra/llcorehttp/bufferarray.cpp b/indra/llcorehttp/bufferarray.cpp
index 4c20350b139..c13b1a35404 100644
--- a/indra/llcorehttp/bufferarray.cpp
+++ b/indra/llcorehttp/bufferarray.cpp
@@ -27,6 +27,18 @@
#include "bufferarray.h"
+// BufferArray is a list of chunks, each a BufferArray::Block, of contiguous
+// data presented as a single array. Chunks are at least BufferArray::BLOCK_ALLOC_SIZE
+// in length and can be larger. Any chunk may be partially filled or even
+// empty.
+//
+// The BufferArray itself is sharable as a RefCounted entity. As shared
+// reads don't work with the concept of a current position/seek value,
+// none is kept with the object. Instead, the read and write operations
+// all take position arguments. Single write/shared read isn't supported
+// directly and any such attempts have to be serialized outside of this
+// implementation.
+
namespace LLCore
{
@@ -58,7 +70,8 @@ class BufferArray::Block
static Block * alloc(size_t len);
public:
- size_t mLen;
+ size_t mUsed;
+ size_t mAlloced;
// *NOTE: Must be last member of the object. We'll
// overallocate as requested via operator new and index
@@ -74,7 +87,6 @@ class BufferArray::Block
BufferArray::BufferArray()
: LLCoreInt::RefCounted(true),
- mPos(0),
mLen(0)
{}
@@ -94,19 +106,45 @@ BufferArray::~BufferArray()
size_t BufferArray::append(const char * src, size_t len)
{
- if (len)
+ const size_t ret(len);
+
+ // First, try to copy into the last block
+ if (len && ! mBlocks.empty())
+ {
+ Block & last(*mBlocks.back());
+ if (last.mUsed < last.mAlloced)
+ {
+ // Some will fit...
+ const size_t copy_len((std::min)(len, (last.mAlloced - last.mUsed)));
+
+ memcpy(&last.mData[last.mUsed], src, copy_len);
+ last.mUsed += copy_len;
+ llassert_always(last.mUsed <= last.mAlloced);
+ mLen += copy_len;
+ src += copy_len;
+ len -= copy_len;
+ }
+ }
+
+ // Then get new blocks as needed
+ while (len)
{
+ const size_t copy_len((std::min)(len, BLOCK_ALLOC_SIZE));
+
if (mBlocks.size() >= mBlocks.capacity())
{
mBlocks.reserve(mBlocks.size() + 5);
}
- Block * block = Block::alloc(len);
- memcpy(block->mData, src, len);
+ Block * block = Block::alloc(BLOCK_ALLOC_SIZE);
+ memcpy(block->mData, src, copy_len);
+ block->mUsed = copy_len;
+ llassert_always(block->mUsed <= block->mAlloced);
mBlocks.push_back(block);
- mLen += len;
- mPos = mLen;
+ mLen += copy_len;
+ src += copy_len;
+ len -= copy_len;
}
- return len;
+ return ret;
}
@@ -117,41 +155,33 @@ char * BufferArray::appendBufferAlloc(size_t len)
{
mBlocks.reserve(mBlocks.size() + 5);
}
- Block * block = Block::alloc(len);
+ Block * block = Block::alloc((std::max)(BLOCK_ALLOC_SIZE, len));
+ block->mUsed = len;
mBlocks.push_back(block);
mLen += len;
- mPos = mLen;
return block->mData;
}
-size_t BufferArray::seek(size_t pos)
+size_t BufferArray::read(size_t pos, char * dst, size_t len)
{
- if (pos > mLen)
- pos = mLen;
- mPos = pos;
- return mPos;
-}
-
-
-size_t BufferArray::read(char * dst, size_t len)
-{
- size_t result(0), offset(0);
- size_t len_limit(mLen - mPos);
+ if (pos >= mLen)
+ return 0;
+ size_t len_limit(mLen - pos);
len = std::min(len, len_limit);
-
- if (mPos >= mLen || 0 == len)
+ if (0 == len)
return 0;
+ size_t result(0), offset(0);
const int block_limit(mBlocks.size());
- int block_start(findBlock(mPos, &offset));
+ int block_start(findBlock(pos, &offset));
if (block_start < 0)
return 0;
do
{
Block & block(*mBlocks[block_start]);
- size_t block_limit(block.mLen - offset);
+ size_t block_limit(block.mUsed - offset);
size_t block_len(std::min(block_limit, len));
memcpy(dst, &block.mData[offset], block_len);
@@ -163,19 +193,18 @@ size_t BufferArray::read(char * dst, size_t len)
}
while (len && block_start < block_limit);
- mPos += result;
return result;
}
-size_t BufferArray::write(const char * src, size_t len)
+size_t BufferArray::write(size_t pos, const char * src, size_t len)
{
- size_t result(0), offset(0);
- if (mPos > mLen || 0 == len)
+ if (pos > mLen || 0 == len)
return 0;
+ size_t result(0), offset(0);
const int block_limit(mBlocks.size());
- int block_start(findBlock(mPos, &offset));
+ int block_start(findBlock(pos, &offset));
if (block_start >= 0)
{
@@ -184,20 +213,39 @@ size_t BufferArray::write(const char * src, size_t len)
do
{
Block & block(*mBlocks[block_start]);
- size_t block_limit(block.mLen - offset);
+ size_t block_limit(block.mUsed - offset);
size_t block_len(std::min(block_limit, len));
memcpy(&block.mData[offset], src, block_len);
result += block_len;
- len -= block_len;
src += block_len;
+ len -= block_len;
offset = 0;
++block_start;
}
while (len && block_start < block_limit);
}
- mPos += result;
+ // Something left, see if it will fit in the free
+ // space of the last block.
+ if (len && ! mBlocks.empty())
+ {
+ Block & last(*mBlocks.back());
+ if (last.mUsed < last.mAlloced)
+ {
+ // Some will fit...
+ const size_t copy_len((std::min)(len, (last.mAlloced - last.mUsed)));
+
+ memcpy(&last.mData[last.mUsed], src, copy_len);
+ last.mUsed += copy_len;
+ result += copy_len;
+ llassert_always(last.mUsed <= last.mAlloced);
+ mLen += copy_len;
+ src += copy_len;
+ len -= copy_len;
+ }
+ }
+
if (len)
{
// Some or all of the remaining write data will
@@ -218,12 +266,12 @@ int BufferArray::findBlock(size_t pos, size_t * ret_offset)
const int block_limit(mBlocks.size());
for (int i(0); i < block_limit; ++i)
{
- if (pos < mBlocks[i]->mLen)
+ if (pos < mBlocks[i]->mUsed)
{
*ret_offset = pos;
return i;
}
- pos -= mBlocks[i]->mLen;
+ pos -= mBlocks[i]->mUsed;
}
// Shouldn't get here but...
@@ -237,7 +285,8 @@ int BufferArray::findBlock(size_t pos, size_t * ret_offset)
BufferArray::Block::Block(size_t len)
- : mLen(len)
+ : mUsed(0),
+ mAlloced(len)
{
memset(mData, 0, len);
}
@@ -245,7 +294,8 @@ BufferArray::Block::Block(size_t len)
BufferArray::Block::~Block()
{
- mLen = 0;
+ mUsed = 0;
+ mAlloced = 0;
}
diff --git a/indra/llcorehttp/bufferarray.h b/indra/llcorehttp/bufferarray.h
index b26ad1b2976..9ccd85d4f8b 100644
--- a/indra/llcorehttp/bufferarray.h
+++ b/indra/llcorehttp/bufferarray.h
@@ -73,6 +73,9 @@ class BufferArray : public LLCoreInt::RefCounted
void operator=(const BufferArray &); // Not defined
public:
+ // Internal magic number, may be used by unit tests.
+ static const size_t BLOCK_ALLOC_SIZE = 1504;
+
/// Appends the indicated data to the BufferArray
/// modifying current position and total size. New
/// position is one beyond the final byte of the buffer.
@@ -96,24 +99,16 @@ class BufferArray : public LLCoreInt::RefCounted
return mLen;
}
- /// Set the current position for subsequent read and
- /// write operations. 'pos' values before the beginning
- /// or greater than the size of the buffer are coerced
- /// to a value within the buffer.
- ///
- /// @return Actual current position after seek.
- size_t seek(size_t pos);
-
- /// Copies data from the current position in the instance
+ /// Copies data from the given position in the instance
/// to the caller's buffer. Will return a short count of
/// bytes copied if the 'len' extends beyond the data.
- size_t read(char * dst, size_t len);
+ size_t read(size_t pos, char * dst, size_t len);
/// Copies data from the caller's buffer to the instance
/// at the current position. May overwrite existing data,
/// append data when current position is equal to the
/// size of the instance or do a mix of both.
- size_t write(const char * src, size_t len);
+ size_t write(size_t pos, const char * src, size_t len);
protected:
int findBlock(size_t pos, size_t * ret_offset);
@@ -123,46 +118,10 @@ class BufferArray : public LLCoreInt::RefCounted
typedef std::vector<Block *> container_t;
container_t mBlocks;
- size_t mPos;
size_t mLen;
}; // end class BufferArray
-#if 0
-
-// Conceptual for now. Another possibility is going with
-// something like Boost::asio's buffers interface. They're
-// trying to achieve the same thing above and below....
-
-class BufferStream : public std::streambuf
-{
-public:
- BufferStream(BufferArray * buffer);
- virtual ~BufferStream();
-
-private:
- BufferStream(const BufferStream &); // Not defined
- void operator=(const BufferStream &); // Not defined
-
-public:
- // Types
- typedef std::streambuf::pos_type pos_type;
- typedef std::streambuf::off_type off_type;
-
- virtual int underflow();
-
- virtual int overflow(int c);
-
- virtual int sync();
-
- virtual pos_type seekoff(off_type off, std::ios::seekdir way, std::ios::openmode which);
-
-protected:
- BufferArray * mBufferArray;
-}; // end class BufferStream
-
-#endif // 0
-
} // end namespace LLCore
#endif // _LLCORE_BUFFER_ARRAY_H_
diff --git a/indra/llcorehttp/tests/test_bufferarray.hpp b/indra/llcorehttp/tests/test_bufferarray.hpp
index ecbb5ef2506..2ad9391d1cb 100644
--- a/indra/llcorehttp/tests/test_bufferarray.hpp
+++ b/indra/llcorehttp/tests/test_bufferarray.hpp
@@ -67,7 +67,7 @@ void BufferArrayTestObjectType::test<1>()
// Try to read
char buffer[20];
- size_t read_len(ba->read(buffer, sizeof(buffer)));
+ size_t read_len(ba->read(0, buffer, sizeof(buffer)));
ensure("Read returns empty", 0 == read_len);
// release the implicit reference, causing the object to be released
@@ -92,16 +92,13 @@ void BufferArrayTestObjectType::test<2>()
char str1[] = "abcdefghij";
char buffer[256];
- size_t len = ba->write(str1, strlen(str1));
+ size_t len = ba->write(0, str1, strlen(str1));
ensure("Wrote length correct", strlen(str1) == len);
ensure("Recorded size correct", strlen(str1) == ba->size());
// read some data back
- len = ba->seek(2);
- ensure("Seek worked", 2 == len);
-
memset(buffer, 'X', sizeof(buffer));
- len = ba->read(buffer, 2);
+ len = ba->read(2, buffer, 2);
ensure("Read length correct", 2 == len);
ensure("Read content correct", 'c' == buffer[0] && 'd' == buffer[1]);
ensure("Read didn't overwrite", 'X' == buffer[2]);
@@ -130,32 +127,26 @@ void BufferArrayTestObjectType::test<3>()
size_t str1_len(strlen(str1));
char buffer[256];
- size_t len = ba->write(str1, str1_len);
+ size_t len = ba->write(0, str1, str1_len);
ensure("Wrote length correct", str1_len == len);
ensure("Recorded size correct", str1_len == ba->size());
// again...
- len = ba->write(str1, strlen(str1));
+ len = ba->write(str1_len, str1, strlen(str1));
ensure("Wrote length correct", str1_len == len);
ensure("Recorded size correct", (2 * str1_len) == ba->size());
// read some data back
- len = ba->seek(8);
- ensure("Seek worked", 8 == len);
-
memset(buffer, 'X', sizeof(buffer));
- len = ba->read(buffer, 4);
+ len = ba->read(8, buffer, 4);
ensure("Read length correct", 4 == len);
ensure("Read content correct", 'i' == buffer[0] && 'j' == buffer[1]);
ensure("Read content correct", 'a' == buffer[2] && 'b' == buffer[3]);
ensure("Read didn't overwrite", 'X' == buffer[4]);
// Read whole thing
- len = ba->seek(0);
- ensure("Seek worked (2)", 0 == len);
-
memset(buffer, 'X', sizeof(buffer));
- len = ba->read(buffer, sizeof(buffer));
+ len = ba->read(0, buffer, sizeof(buffer));
ensure("Read length correct", (2 * str1_len) == len);
ensure("Read content correct (3)", 0 == strncmp(buffer, str1, str1_len));
ensure("Read content correct (4)", 0 == strncmp(&buffer[str1_len], str1, str1_len));
@@ -185,33 +176,29 @@ void BufferArrayTestObjectType::test<4>()
char str2[] = "ABCDEFGHIJ";
char buffer[256];
- size_t len = ba->write(str1, str1_len);
+ size_t len = ba->write(0, str1, str1_len);
ensure("Wrote length correct", str1_len == len);
ensure("Recorded size correct", str1_len == ba->size());
// again...
- len = ba->write(str1, strlen(str1));
+ len = ba->write(str1_len, str1, strlen(str1));
ensure("Wrote length correct", str1_len == len);
ensure("Recorded size correct", (2 * str1_len) == ba->size());
// reposition and overwrite
- len = ba->seek(8);
- ensure("Seek worked", 8 == len);
- len = ba->write(str2, 4);
+ len = ba->write(8, str2, 4);
ensure("Overwrite length correct", 4 == len);
- // Leave position and read verifying content
+ // Leave position and read verifying content (stale really from seek() days)
memset(buffer, 'X', sizeof(buffer));
- len = ba->read(buffer, 4);
+ len = ba->read(12, buffer, 4);
ensure("Read length correct", 4 == len);
ensure("Read content correct", 'c' == buffer[0] && 'd' == buffer[1]);
ensure("Read content correct.2", 'e' == buffer[2] && 'f' == buffer[3]);
ensure("Read didn't overwrite", 'X' == buffer[4]);
// reposition and check
- len = ba->seek(6);
- memset(buffer, 'X', sizeof(buffer));
- len = ba->read(buffer, 8);
+ len = ba->read(6, buffer, 8);
ensure("Read length correct.2", 8 == len);
ensure("Read content correct.3", 'g' == buffer[0] && 'h' == buffer[1]);
ensure("Read content correct.4", 'A' == buffer[2] && 'B' == buffer[3]);
@@ -242,21 +229,18 @@ void BufferArrayTestObjectType::test<5>()
size_t str1_len(strlen(str1));
char buffer[256];
- size_t len = ba->write(str1, str1_len);
+ size_t len = ba->write(0, str1, str1_len);
ensure("Wrote length correct", str1_len == len);
ensure("Recorded size correct", str1_len == ba->size());
// again...
- len = ba->write(str1, strlen(str1));
+ len = ba->write(str1_len, str1, str1_len);
ensure("Wrote length correct", str1_len == len);
ensure("Recorded size correct", (2 * str1_len) == ba->size());
// read some data back
- len = ba->seek(8);
- ensure("Seek worked", 8 == len);
-
memset(buffer, 'X', sizeof(buffer));
- len = ba->read(buffer, 4);
+ len = ba->read(8, buffer, 4);
ensure("Read length correct", 4 == len);
ensure("Read content correct", 'i' == buffer[0] && 'j' == buffer[1]);
ensure("Read content correct.2", 'a' == buffer[2] && 'b' == buffer[3]);
@@ -264,7 +248,7 @@ void BufferArrayTestObjectType::test<5>()
// Read some more without repositioning
memset(buffer, 'X', sizeof(buffer));
- len = ba->read(buffer, sizeof(buffer));
+ len = ba->read(12, buffer, sizeof(buffer));
ensure("Read length correct", (str1_len - 2) == len);
ensure("Read content correct.3", 0 == strncmp(buffer, str1+2, str1_len-2));
ensure("Read didn't overwrite.2", 'X' == buffer[str1_len-1]);
@@ -294,31 +278,27 @@ void BufferArrayTestObjectType::test<6>()
size_t str2_len(strlen(str2));
char buffer[256];
- size_t len = ba->write(str1, str1_len);
+ size_t len = ba->write(0, str1, str1_len);
ensure("Wrote length correct", str1_len == len);
ensure("Recorded size correct", str1_len == ba->size());
// again...
- len = ba->write(str1, strlen(str1));
+ len = ba->write(str1_len, str1, strlen(str1));
ensure("Wrote length correct", str1_len == len);
ensure("Recorded size correct", (2 * str1_len) == ba->size());
// reposition and overwrite
- len = ba->seek(8);
- ensure("Seek worked", 8 == len);
- len = ba->write(str2, str2_len);
+ len = ba->write(8, str2, str2_len);
ensure("Overwrite length correct", str2_len == len);
// Leave position and read verifying content
memset(buffer, 'X', sizeof(buffer));
- len = ba->read(buffer, 0);
+ len = ba->read(8 + str2_len, buffer, 0);
ensure("Read length correct", 0 == len);
ensure("Read didn't overwrite", 'X' == buffer[0]);
// reposition and check
- len = ba->seek(0);
- memset(buffer, 'X', sizeof(buffer));
- len = ba->read(buffer, sizeof(buffer));
+ len = ba->read(0, buffer, sizeof(buffer));
ensure("Read length correct.2", (str1_len + str2_len - 2) == len);
ensure("Read content correct", 0 == strncmp(buffer, str1, str1_len-2));
ensure("Read content correct.2", 0 == strncmp(buffer+str1_len-2, str2, str2_len));
@@ -350,18 +330,17 @@ void BufferArrayTestObjectType::test<7>()
char buffer[256];
// 2x str1
- size_t len = ba->write(str1, str1_len);
- len = ba->write(str1, strlen(str1));
+ size_t len = ba->write(0, str1, str1_len);
+ len = ba->write(str1_len, str1, str1_len);
// reposition and overwrite
- len = ba->seek(6);
- len = ba->write(str2, 2);
+ len = ba->write(6, str2, 2);
ensure("Overwrite length correct", 2 == len);
- len = ba->write(str2, 2);
+ len = ba->write(8, str2, 2);
ensure("Overwrite length correct.2", 2 == len);
- len = ba->write(str2, 2);
+ len = ba->write(10, str2, 2);
ensure("Overwrite length correct.3", 2 == len);
// append some data
@@ -373,13 +352,12 @@ void BufferArrayTestObjectType::test<7>()
memcpy(out_buf, str1, str1_len);
// And some final writes
- len = ba->write(str2, 2);
+ len = ba->write(3 * str1_len + str2_len, str2, 2);
ensure("Write length correct.2", 2 == len);
// Check contents
memset(buffer, 'X', sizeof(buffer));
- ba->seek(0);
- len = ba->read(buffer, sizeof(buffer));
+ len = ba->read(0, buffer, sizeof(buffer));
ensure("Final buffer length correct", (3 * str1_len + str2_len + 2) == len);
ensure("Read content correct", 0 == strncmp(buffer, str1, 6));
ensure("Read content correct.2", 0 == strncmp(buffer + 6, str2, 2));
@@ -417,8 +395,8 @@ void BufferArrayTestObjectType::test<8>()
char buffer[256];
// 2x str1
- size_t len = ba->write(str1, str1_len);
- len = ba->write(str1, strlen(str1));
+ size_t len = ba->write(0, str1, str1_len);
+ len = ba->write(str1_len, str1, str1_len);
// zero-length allocate (we allow this with a valid pointer returned)
char * out_buf(ba->appendBufferAlloc(0));
@@ -430,12 +408,11 @@ void BufferArrayTestObjectType::test<8>()
ensure("Two zero-length appendBufferAlloc buffers distinct", out_buf != out_buf2);
// And some final writes
- len = ba->write(str2, str2_len);
+ len = ba->write(2 * str1_len, str2, str2_len);
// Check contents
memset(buffer, 'X', sizeof(buffer));
- ba->seek(0);
- len = ba->read(buffer, sizeof(buffer));
+ len = ba->read(0, buffer, sizeof(buffer));
ensure("Final buffer length correct", (2 * str1_len + str2_len) == len);
ensure("Read content correct.1", 0 == strncmp(buffer, str1, str1_len));
ensure("Read content correct.2", 0 == strncmp(buffer + str1_len, str1, str1_len));
diff --git a/indra/newview/lltexturefetch.cpp b/indra/newview/lltexturefetch.cpp
index daad463e0d7..e27f0aec1ec 100644
--- a/indra/newview/lltexturefetch.cpp
+++ b/indra/newview/lltexturefetch.cpp
@@ -1255,8 +1255,7 @@ bool LLTextureFetchWorker::doWork(S32 param)
{
memcpy(buffer, mFormattedImage->getData(), cur_size);
}
- mHttpBufferArray->seek(0);
- mHttpBufferArray->read((char *) buffer + cur_size, append_size);
+ mHttpBufferArray->read(0, (char *) buffer + cur_size, append_size);
// NOTE: setData releases current data and owns new data (buffer)
mFormattedImage->setData(buffer, total_size);
--
GitLab