From 4155301015525a242a79b9b3134e66d366bc0ebd Mon Sep 17 00:00:00 2001 From: Monty Brandenberg Date: Fri, 1 Jun 2012 21:30:45 -0400 Subject: 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 791ee5f860..196011f953 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(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(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(data), do_size); - op->mCurlBodyPos += do_size; - return do_size; + const size_t read_size(op->mReqBody->read(op->mCurlBodyPos, static_cast(data), do_size)); + op->mCurlBodyPos += read_size; + return read_size; } diff --git a/indra/llcorehttp/bufferarray.cpp b/indra/llcorehttp/bufferarray.cpp index 4c20350b13..c13b1a3540 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 @@ public: 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 @@ public: 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 b26ad1b297..9ccd85d4f8 100644 --- a/indra/llcorehttp/bufferarray.h +++ b/indra/llcorehttp/bufferarray.h @@ -73,6 +73,9 @@ private: 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 @@ public: 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 @@ protected: typedef std::vector 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 ecbb5ef250..2ad9391d1c 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 daad463e0d..e27f0aec1e 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); -- cgit v1.2.3