diff options
| -rw-r--r-- | indra/llmessage/llbuffer.cpp | 120 | ||||
| -rw-r--r-- | indra/llmessage/llbuffer.h | 90 | ||||
| -rw-r--r-- | indra/llmessage/llbufferstream.cpp | 95 | ||||
| -rw-r--r-- | indra/test/io.cpp | 128 |
4 files changed, 388 insertions, 45 deletions
diff --git a/indra/llmessage/llbuffer.cpp b/indra/llmessage/llbuffer.cpp index e4200b914b..6b0c4f8fab 100644 --- a/indra/llmessage/llbuffer.cpp +++ b/indra/llmessage/llbuffer.cpp @@ -65,24 +65,44 @@ S32 LLSegment::size() const return mSize; } +bool LLSegment::operator==(const LLSegment& rhs) const +{ + if((mData != rhs.mData)||(mSize != rhs.mSize)||(mChannel != rhs.mChannel)) + { + return false; + } + return true; +} /** * LLHeapBuffer */ -LLHeapBuffer::LLHeapBuffer() +LLHeapBuffer::LLHeapBuffer() : + mBuffer(NULL), + mSize(0), + mNextFree(NULL), + mReclaimedBytes(0) { LLMemType m1(LLMemType::MTYPE_IO_BUFFER); const S32 DEFAULT_HEAP_BUFFER_SIZE = 16384; allocate(DEFAULT_HEAP_BUFFER_SIZE); } -LLHeapBuffer::LLHeapBuffer(S32 size) +LLHeapBuffer::LLHeapBuffer(S32 size) : + mBuffer(NULL), + mSize(0), + mNextFree(NULL), + mReclaimedBytes(0) { LLMemType m1(LLMemType::MTYPE_IO_BUFFER); allocate(size); } -LLHeapBuffer::LLHeapBuffer(const U8* src, S32 len) +LLHeapBuffer::LLHeapBuffer(const U8* src, S32 len) : + mBuffer(NULL), + mSize(0), + mNextFree(NULL), + mReclaimedBytes(0) { LLMemType m1(LLMemType::MTYPE_IO_BUFFER); if((len > 0) && src) @@ -93,12 +113,6 @@ LLHeapBuffer::LLHeapBuffer(const U8* src, S32 len) memcpy(mBuffer, src, len); /*Flawfinder: ignore*/ } } - else - { - mBuffer = NULL; - mSize = 0; - mNextFree = NULL; - } } // virtual @@ -111,11 +125,10 @@ LLHeapBuffer::~LLHeapBuffer() mNextFree = NULL; } -// virtual -//S32 LLHeapBuffer::bytesLeft() const -//{ -// return (mSize - (mNextFree - mBuffer)); -//} +S32 LLHeapBuffer::bytesLeft() const +{ + return (mSize - (mNextFree - mBuffer)); +} // virtual bool LLHeapBuffer::createSegment( @@ -139,9 +152,47 @@ bool LLHeapBuffer::createSegment( return true; } +// virtual +bool LLHeapBuffer::reclaimSegment(const LLSegment& segment) +{ + if(containsSegment(segment)) + { + mReclaimedBytes += segment.size(); + if(mReclaimedBytes == mSize) + { + // We have reclaimed all of the memory from this + // buffer. Therefore, we can reset the mNextFree to the + // start of the buffer, and reset the reclaimed bytes. + mReclaimedBytes = 0; + mNextFree = mBuffer; + } + else if(mReclaimedBytes > mSize) + { + llwarns << "LLHeapBuffer reclaimed more memory than allocated." + << " This is probably programmer error." << llendl; + } + return true; + } + return false; +} + +// virtual +bool LLHeapBuffer::containsSegment(const LLSegment& segment) const +{ + // *NOTE: this check is fairly simple because heap buffers are + // simple contiguous chunks of heap memory. + if((mBuffer > segment.data()) + || ((mBuffer + mSize) < (segment.data() + segment.size()))) + { + return false; + } + return true; +} + void LLHeapBuffer::allocate(S32 size) { LLMemType m1(LLMemType::MTYPE_IO_BUFFER); + mReclaimedBytes = 0; mBuffer = new U8[size]; if(mBuffer) { @@ -180,6 +231,18 @@ LLChannelDescriptors LLBufferArray::nextChannel() return rv; } +S32 LLBufferArray::capacity() const +{ + S32 total = 0; + const_buffer_iterator_t iter = mBuffers.begin(); + const_buffer_iterator_t end = mBuffers.end(); + for(; iter != end; ++iter) + { + total += (*iter)->capacity(); + } + return total; +} + bool LLBufferArray::append(S32 channel, const U8* src, S32 len) { LLMemType m1(LLMemType::MTYPE_IO_BUFFER); @@ -684,14 +747,31 @@ LLBufferArray::segment_iterator_t LLBufferArray::makeSegment( return send; } -bool LLBufferArray::eraseSegment(const segment_iterator_t& iter) +bool LLBufferArray::eraseSegment(const segment_iterator_t& erase_iter) { LLMemType m1(LLMemType::MTYPE_IO_BUFFER); - // *FIX: in theory, we could reclaim the memory. We are leaking a - // bit of buffered memory into an unusable but still referenced - // location. - (void)mSegments.erase(iter); - return true; + + // Find out which buffer contains the segment, and if it is found, + // ask it to reclaim the memory. + bool rv = false; + LLSegment segment(*erase_iter); + buffer_iterator_t iter = mBuffers.begin(); + buffer_iterator_t end = mBuffers.end(); + for(; iter != end; ++iter) + { + // We can safely call reclaimSegment on every buffer, and once + // it returns true, the segment was found. + if((*iter)->reclaimSegment(segment)) + { + rv = true; + break; + } + } + + // No need to get the return value since we are not interested in + // the interator retured by the call. + (void)mSegments.erase(erase_iter); + return rv; } diff --git a/indra/llmessage/llbuffer.h b/indra/llmessage/llbuffer.h index 3d7f209123..0a0a457d56 100644 --- a/indra/llmessage/llbuffer.h +++ b/indra/llmessage/llbuffer.h @@ -13,7 +13,7 @@ /** * Declaration of classes used for minimizing calls to new[], - * memcpy(), and delete[]. Typically, you would create an LLHeapArray, + * memcpy(), and delete[]. Typically, you would create an LLBufferArray, * feed it data, modify and add segments as you process it, and feed * it to a sink. */ @@ -89,6 +89,16 @@ public: */ S32 size() const; + /** + * @brief Check if two segments are the same. + * + * Two segments are considered equal if they are on the same + * channel and cover the exact same address range. + * @param rhs the segment to compare with this segment. + * @return Returns true if they are equal. + */ + bool operator==(const LLSegment& rhs) const; + protected: S32 mChannel; U8* mData; @@ -126,6 +136,35 @@ public: * @return Returns true if a segment was found. */ virtual bool createSegment(S32 channel, S32 size, LLSegment& segment) = 0; + + /** + * @brief Reclaim a segment from this buffer. + * + * This method is called on a buffer object when a caller is done + * with a contiguous segment of memory inside this buffer. Since + * segments can be cut arbitrarily outside of the control of the + * buffer, this segment may not match any segment returned from + * <code>createSegment()</code>. + * @param segment The contiguous buffer segment to reclaim. + * @return Returns true if the call was successful. + */ + virtual bool reclaimSegment(const LLSegment& segment) = 0; + + /** + * @brief Test if a segment is inside this buffer. + * + * @param segment The contiguous buffer segment to test. + * @return Returns true if the segment is in the bufffer. + */ + virtual bool containsSegment(const LLSegment& segment) const = 0; + + /** + * @brief Return the current number of bytes allocated. + * + * This was implemented as a debugging tool, and it is not + * necessarily a good idea to use it for anything else. + */ + virtual S32 capacity() const = 0; }; /** @@ -167,9 +206,11 @@ public: /** * @brief Get the number of bytes left in the buffer. * + * Note that this is not a virtual function, and only available in + * the LLHeapBuffer as a debugging aid. * @return Returns the number of bytes left. */ - //virtual S32 bytesLeft() const; + S32 bytesLeft() const; /** * @brief Generate a segment for this buffer. @@ -186,10 +227,40 @@ public: */ virtual bool createSegment(S32 channel, S32 size, LLSegment& segment); + /** + * @brief reclaim a segment from this buffer. + * + * This method is called on a buffer object when a caller is done + * with a contiguous segment of memory inside this buffer. Since + * segments can be cut arbitrarily outside of the control of the + * buffer, this segment may not match any segment returned from + * <code>createSegment()</code>. + * This call will fail if the segment passed in is note completely + * inside the buffer, eg, if the segment starts before this buffer + * in memory or ends after it. + * @param segment The contiguous buffer segment to reclaim. + * @return Returns true if the call was successful. + */ + virtual bool reclaimSegment(const LLSegment& segment); + + /** + * @brief Test if a segment is inside this buffer. + * + * @param segment The contiguous buffer segment to test. + * @return Returns true if the segment is in the bufffer. + */ + virtual bool containsSegment(const LLSegment& segment) const; + + /** + * @brief Return the current number of bytes allocated. + */ + virtual S32 capacity() const { return mSize; } + protected: U8* mBuffer; S32 mSize; U8* mNextFree; + S32 mReclaimedBytes; private: /** @@ -204,13 +275,14 @@ private: * @brief Class to represent scattered memory buffers and in-order segments * of that buffered data. * - * NOTE: This class needs to have an iovec interface + * *NOTE: This class needs to have an iovec interface */ class LLBufferArray { public: typedef std::vector<LLBuffer*> buffer_list_t; typedef buffer_list_t::iterator buffer_iterator_t; + typedef buffer_list_t::const_iterator const_buffer_iterator_t; typedef std::list<LLSegment> segment_list_t; typedef segment_list_t::const_iterator const_segment_iterator_t; typedef segment_list_t::iterator segment_iterator_t; @@ -241,11 +313,16 @@ public: */ LLChannelDescriptors nextChannel(); //@} - + /* @name Data methods */ //@{ + /** + * @brief Return the sum of all allocated bytes. + */ + S32 capacity() const; + // These methods will be useful once there is any kind of buffer // besides a heap buffer. //bool append(EBufferChannel channel, LLBuffer* data); @@ -275,7 +352,6 @@ public: * new segment is created and put in the front of the array. This * object will internally allocate new buffers if necessary. * @param channel The channel for this data - * @param src The start of memory for the data to be copied * @param len The number of bytes of data to copy * @return Returns true if the method worked. @@ -359,7 +435,7 @@ public: bool takeContents(LLBufferArray& source); //@} - /* @name Segment methods + /* @name Segment methods */ //@{ /** @@ -449,7 +525,7 @@ public: * endSegment() on failure. */ segment_iterator_t makeSegment(S32 channel, S32 length); - + /** * @brief Erase the segment if it is in the buffer array. * diff --git a/indra/llmessage/llbufferstream.cpp b/indra/llmessage/llbufferstream.cpp index 684548b408..e86620c438 100644 --- a/indra/llmessage/llbufferstream.cpp +++ b/indra/llmessage/llbufferstream.cpp @@ -43,34 +43,64 @@ int LLBufferStreamBuf::underflow() { return EOF; } - LLSegment segment; - LLBufferArray::segment_iterator_t it; - U8* last_pos = (U8*)gptr(); - if(last_pos) --last_pos; - - LLBufferArray::segment_iterator_t end = mBuffer->endSegment(); - // Get iterator to full segment containing last_pos - // and construct sub-segment starting at last_pos. - // Note: segment may != *it at this point - it = mBuffer->constructSegmentAfter(last_pos, segment); - if(it == end) + LLBufferArray::segment_iterator_t iter; + LLBufferArray::segment_iterator_t end = mBuffer->endSegment(); + U8* last_pos = (U8*)gptr(); + LLSegment segment; + if(last_pos) + { + // Back up into a piece of memory we know that we have + // allocated so that calls for the next segment based on + // 'after' will succeed. + --last_pos; + iter = mBuffer->splitAfter(last_pos); + if(iter != end) + { + // We need to clear the read segment just in case we have + // an early exit in the function and never collect the + // next segment. Calling eraseSegment() with the same + // segment twice is just like double deleting -- nothing + // good comes from it. + mBuffer->eraseSegment(iter++); + if(iter != end) segment = (*iter); + } + else + { + // This should never really happen, but somehow, the + // istream is telling the buf that it just finished + // reading memory that is not in the buf. I think this + // would only happen if there were a bug in the c++ stream + // class. Just bail. + // *TODO: can we set the fail bit on the stream somehow? + return EOF; + } + } + else + { + // Get iterator to full segment containing last_pos + // and construct sub-segment starting at last_pos. + // Note: segment may != *it at this point + iter = mBuffer->constructSegmentAfter(last_pos, segment); + } + if(iter == end) { return EOF; } - + // Iterate through segments to find a non-empty segment on input channel. while((!segment.isOnChannel(mChannels.in()) || (segment.size() == 0))) { - ++it; - if(it == end) + ++iter; + if(iter == end) { return EOF; } - segment = *it; + segment = *(iter); } - + + // set up the stream to read from the next segment. char* start = (char*)segment.data(); setg(start, start, start + segment.size()); return *gptr(); @@ -125,12 +155,43 @@ int LLBufferStreamBuf::sync() return return_value; } + // This chunk of code is not necessary because typically, users of + // the stream will read until EOF. Therefore, underflow was called + // and the segment was discarded before the sync() was called in + // the destructor. Theoretically, we could keep some more data + // around and detect the rare case where an istream was deleted + // before reading to the end, but that will only leave behind some + // unavailable but still referenced memory. Also, if another + // istream is constructed, it will re-read that segment, and then + // discard it. + //U8* last_pos = (U8*)gptr(); + //if(last_pos) + //{ + // // Looks like we read something. Discard what we have read. + // // gptr() actually returns the currrent position, but we call + // // it last_pos because of how it is used in the split call + // // below. + // --last_pos; + // LLBufferArray::segment_iterator_t iter; + // iter = mBuffer->splitAfter(last_pos); + // if(iter != mBuffer->endSegment()) + // { + // // We need to clear the read segment just in case we have + // // an early exit in the function and never collect the + // // next segment. Calling eraseSegment() with the same + // // segment twice is just like double deleting -- nothing + // // good comes from it. + // mBuffer->eraseSegment(iter); + // } + //} + // set the put pointer so that we force an overflow on the next // write. U8* address = (U8*)pptr(); setp(NULL, NULL); - // *NOTE: I bet we could just --address. Need to think about that. + // *NOTE: I bet we could just --address if address is not NULL. + // Need to think about that. address = mBuffer->seek(mChannels.out(), address, -1); if(address) { diff --git a/indra/test/io.cpp b/indra/test/io.cpp index 4908653f0f..034472fa0b 100644 --- a/indra/test/io.cpp +++ b/indra/test/io.cpp @@ -31,6 +31,101 @@ namespace tut { + struct heap_buffer_data + { + heap_buffer_data() : mBuffer(NULL) {} + ~heap_buffer_data() { if(mBuffer) delete mBuffer; } + LLHeapBuffer* mBuffer; + }; + typedef test_group<heap_buffer_data> heap_buffer_test; + typedef heap_buffer_test::object heap_buffer_object; + tut::heap_buffer_test thb("heap_buffer"); + + template<> template<> + void heap_buffer_object::test<1>() + { + const S32 BUF_SIZE = 100; + mBuffer = new LLHeapBuffer(BUF_SIZE); + ensure_equals("empty buffer capacity", mBuffer->capacity(), BUF_SIZE); + const S32 SEGMENT_SIZE = 50; + LLSegment segment; + mBuffer->createSegment(0, SEGMENT_SIZE, segment); + ensure_equals("used buffer capacity", mBuffer->capacity(), BUF_SIZE); + } + + template<> template<> + void heap_buffer_object::test<2>() + { + const S32 BUF_SIZE = 10; + mBuffer = new LLHeapBuffer(BUF_SIZE); + LLSegment segment; + mBuffer->createSegment(0, BUF_SIZE, segment); + ensure("segment is in buffer", mBuffer->containsSegment(segment)); + ensure_equals("buffer consumed", mBuffer->bytesLeft(), 0); + bool created; + created = mBuffer->createSegment(0, 0, segment); + ensure("Create zero size segment fails", !created); + created = mBuffer->createSegment(0, BUF_SIZE, segment); + ensure("Create segment fails", !created); + } + + template<> template<> + void heap_buffer_object::test<3>() + { + const S32 BUF_SIZE = 10; + mBuffer = new LLHeapBuffer(BUF_SIZE); + LLSegment segment; + mBuffer->createSegment(0, BUF_SIZE, segment); + ensure("segment is in buffer", mBuffer->containsSegment(segment)); + ensure_equals("buffer consumed", mBuffer->bytesLeft(), 0); + bool reclaimed = mBuffer->reclaimSegment(segment); + ensure("buffer reclaimed.", reclaimed); + ensure_equals("buffer available", mBuffer->bytesLeft(), BUF_SIZE); + bool created; + created = mBuffer->createSegment(0, 0, segment); + ensure("Create zero size segment fails", !created); + created = mBuffer->createSegment(0, BUF_SIZE, segment); + ensure("Create another segment succeeds", created); + } + + template<> template<> + void heap_buffer_object::test<4>() + { + const S32 BUF_SIZE = 10; + const S32 SEGMENT_SIZE = 4; + mBuffer = new LLHeapBuffer(BUF_SIZE); + LLSegment seg1; + mBuffer->createSegment(0, SEGMENT_SIZE, seg1); + ensure("segment is in buffer", mBuffer->containsSegment(seg1)); + LLSegment seg2; + mBuffer->createSegment(0, SEGMENT_SIZE, seg2); + ensure("segment is in buffer", mBuffer->containsSegment(seg2)); + LLSegment seg3; + mBuffer->createSegment(0, SEGMENT_SIZE, seg3); + ensure("segment is in buffer", mBuffer->containsSegment(seg3)); + ensure_equals("segment is truncated", seg3.size(), 2); + LLSegment seg4; + bool created; + created = mBuffer->createSegment(0, SEGMENT_SIZE, seg4); + ensure("Create segment fails", !created); + bool reclaimed; + reclaimed = mBuffer->reclaimSegment(seg1); + ensure("buffer reclaim succeed.", reclaimed); + ensure_equals("no buffer available", mBuffer->bytesLeft(), 0); + reclaimed = mBuffer->reclaimSegment(seg2); + ensure("buffer reclaim succeed.", reclaimed); + ensure_equals("buffer reclaimed", mBuffer->bytesLeft(), 0); + reclaimed = mBuffer->reclaimSegment(seg3); + ensure("buffer reclaim succeed.", reclaimed); + ensure_equals("buffer reclaimed", mBuffer->bytesLeft(), BUF_SIZE); + created = mBuffer->createSegment(0, SEGMENT_SIZE, seg1); + ensure("segment is in buffer", mBuffer->containsSegment(seg1)); + ensure("Create segment succeds", created); + } +} + +namespace tut +{ struct buffer_data { LLBufferArray mBuffer; @@ -209,6 +304,37 @@ namespace tut delete[] temp; } + template<> template<> + void buffer_object::test<9>() + { + LLChannelDescriptors ch = mBuffer.nextChannel(); + mBuffer.append(ch.in(), (U8*)"1", 1); + S32 capacity = mBuffer.capacity(); + ensure("has capacity", capacity > 0); + U8* temp = new U8[capacity - 1]; + mBuffer.append(ch.in(), temp, capacity - 1); + capacity = mBuffer.capacity(); + ensure("has capacity when full", capacity > 0); + S32 used = mBuffer.countAfter(ch.in(), NULL); + ensure_equals("used equals capacity", used, capacity); + + LLBufferArray::segment_iterator_t iter = mBuffer.beginSegment(); + while(iter != mBuffer.endSegment()) + { + mBuffer.eraseSegment(iter++); + } + + used = mBuffer.countAfter(ch.in(), NULL); + ensure_equals("used is zero", used, 0); + S32 capacity2 = mBuffer.capacity(); + ensure_equals("capacity the same after erase", capacity2, capacity); + mBuffer.append(ch.in(), temp, capacity - 1); + capacity2 = mBuffer.capacity(); + ensure_equals("capacity the same after append", capacity2, capacity); + + delete[] temp; + } + #if 0 template<> template<> void buffer_object::test<9>() @@ -268,7 +394,7 @@ namespace tut void bas_object::test<1>() { const char HELLO_WORLD[] = "hello world"; - const S32 str_len = strlen(HELLO_WORLD); /* Flawfinder: ignore */ + const S32 str_len = strlen(HELLO_WORLD); /* Flawfinder: ignore */ LLChannelDescriptors ch = mBuffer.nextChannel(); LLBufferStream str(ch, &mBuffer); mBuffer.append(ch.in(), (U8*)HELLO_WORLD, str_len); |