You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@trafficserver.apache.org by ma...@apache.org on 2019/09/09 22:57:41 UTC
[trafficserver] branch master updated: Reduce unnecesary
IOBufferBlock allocation
This is an automated email from the ASF dual-hosted git repository.
masaori pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/trafficserver.git
The following commit(s) were added to refs/heads/master by this push:
new c40271a Reduce unnecesary IOBufferBlock allocation
c40271a is described below
commit c40271ae7543a87bc90b4278954b79d304bb70b0
Author: Masaori Koshiba <ma...@apache.org>
AuthorDate: Tue Sep 3 16:23:00 2019 +0900
Reduce unnecesary IOBufferBlock allocation
- Add unit tests for MIOBuffer::write_avail()
- Do nothing when the next block of the current writer exists
---
iocore/eventsystem/IOBuffer.cc | 13 +-
iocore/eventsystem/I_IOBuffer.h | 5 +-
iocore/eventsystem/P_IOBuffer.h | 4 +-
iocore/eventsystem/unit_tests/test_IOBuffer.cc | 162 +++++++++++++++++++++++++
4 files changed, 179 insertions(+), 5 deletions(-)
diff --git a/iocore/eventsystem/IOBuffer.cc b/iocore/eventsystem/IOBuffer.cc
index e1a255a..d2c130b 100644
--- a/iocore/eventsystem/IOBuffer.cc
+++ b/iocore/eventsystem/IOBuffer.cc
@@ -60,6 +60,9 @@ init_buffer_allocators(int iobuffer_advice)
}
}
+//
+// MIOBuffer
+//
int64_t
MIOBuffer::remove_append(IOBufferReader *r)
{
@@ -173,6 +176,9 @@ MIOBuffer::puts(char *s, int64_t len)
return 0;
}
+//
+// IOBufferReader
+//
int64_t
IOBufferReader::read(void *ab, int64_t len)
{
@@ -262,6 +268,9 @@ IOBufferReader::memcpy(void *ap, int64_t len, int64_t offset)
return p;
}
+//
+// IOBufferChain
+//
int64_t
IOBufferChain::write(IOBufferBlock *blocks, int64_t length, int64_t offset)
{
@@ -349,7 +358,9 @@ IOBufferChain::consume(int64_t size)
return zret;
}
-//-- MIOBufferWriter
+//
+// MIOBufferWriter
+//
MIOBufferWriter &
MIOBufferWriter::write(const void *data_, size_t length)
{
diff --git a/iocore/eventsystem/I_IOBuffer.h b/iocore/eventsystem/I_IOBuffer.h
index 75a26a9..164ec1a 100644
--- a/iocore/eventsystem/I_IOBuffer.h
+++ b/iocore/eventsystem/I_IOBuffer.h
@@ -933,9 +933,8 @@ public:
void append_block(int64_t asize_index);
/**
- Adds new block to the end of block list using the block size for
- the buffer specified when the buffer was allocated.
-
+ Adds a new block to the end of the block list. Note that this does nothing when the next block of the current writer exists.
+ The block size is the same as specified size when the buffer was allocated.
*/
void add_block();
diff --git a/iocore/eventsystem/P_IOBuffer.h b/iocore/eventsystem/P_IOBuffer.h
index 6f15d9c..49e6555 100644
--- a/iocore/eventsystem/P_IOBuffer.h
+++ b/iocore/eventsystem/P_IOBuffer.h
@@ -961,7 +961,9 @@ MIOBuffer::append_block(int64_t asize_index)
TS_INLINE void
MIOBuffer::add_block()
{
- append_block(size_index);
+ if (this->_writer == nullptr || this->_writer->next == nullptr) {
+ append_block(size_index);
+ }
}
TS_INLINE void
diff --git a/iocore/eventsystem/unit_tests/test_IOBuffer.cc b/iocore/eventsystem/unit_tests/test_IOBuffer.cc
index f380bf0..8128ffb 100644
--- a/iocore/eventsystem/unit_tests/test_IOBuffer.cc
+++ b/iocore/eventsystem/unit_tests/test_IOBuffer.cc
@@ -173,6 +173,168 @@ TEST_CASE("MIOBuffer", "[iocore]")
free_MIOBuffer(miob);
}
+
+ SECTION("write_avail")
+ {
+ MIOBuffer *miob = new_MIOBuffer();
+ IOBufferReader *miob_r = miob->alloc_reader();
+ uint8_t buf[8192];
+ memset(buf, 0xAA, sizeof(buf));
+
+ // initial state
+ CHECK(miob->block_size() == 4096);
+ CHECK(miob->current_write_avail() == 4096);
+ CHECK(miob->write_avail() == 4096);
+
+ SECTION("water_mark == 0 (default)")
+ {
+ REQUIRE(miob->water_mark == 0);
+
+ // fill half of the current buffer
+ miob->write(buf, 2048);
+ CHECK(miob->max_read_avail() == 2048);
+ CHECK(miob->current_write_avail() == 2048);
+ CHECK(miob->high_water() == true);
+ CHECK(miob->current_low_water() == false);
+ CHECK(miob->write_avail() == 2048); ///< should have no side effect
+
+ // fill all of the current buffer
+ miob->write(buf, 2048);
+ CHECK(miob->max_read_avail() == 4096);
+ CHECK(miob->current_write_avail() == 0);
+ CHECK(miob->high_water() == true);
+ CHECK(miob->current_low_water() == true);
+ CHECK(miob->write_avail() == 0); ///< should have no side effect
+
+ // consume half of the data
+ miob_r->consume(2048);
+ CHECK(miob->max_read_avail() == 2048);
+ CHECK(miob->current_write_avail() == 0);
+ CHECK(miob->high_water() == true);
+ CHECK(miob->current_low_water() == true);
+ CHECK(miob->write_avail() == 0); ///< should have no side effect
+
+ // consume all of the data
+ miob_r->consume(2048);
+ CHECK(miob->max_read_avail() == 0);
+ CHECK(miob->current_write_avail() == 0);
+ CHECK(miob->high_water() == false);
+ CHECK(miob->current_low_water() == true);
+ CHECK(miob->write_avail() == 4096); ///< should have a side effect: add a new block
+
+ CHECK(miob->max_read_avail() == 0);
+ CHECK(miob->current_write_avail() == 4096);
+ CHECK(miob->high_water() == false);
+ CHECK(miob->current_low_water() == false);
+ CHECK(miob->write_avail() == 4096); ///< should have no side effect
+ }
+
+ SECTION("water_mark == half of block size")
+ {
+ miob->water_mark = 2048;
+ REQUIRE(miob->water_mark * 2 == miob->block_size());
+
+ // fill half of the current buffer
+ miob->write(buf, 2048);
+ CHECK(miob->max_read_avail() == 2048);
+ CHECK(miob->current_write_avail() == 2048);
+ CHECK(miob->high_water() == false);
+ CHECK(miob->current_low_water() == true);
+ CHECK(miob->write_avail() == 6144); ///< should have a side effect: add a new block
+
+ CHECK(miob->max_read_avail() == 2048);
+ CHECK(miob->current_write_avail() == 6144);
+ CHECK(miob->high_water() == false);
+ CHECK(miob->current_low_water() == false);
+ CHECK(miob->write_avail() == 6144); ///< should have no side effect
+
+ // fill all of the current buffer
+ miob->write(buf, 6144);
+ CHECK(miob->max_read_avail() == 8192);
+ CHECK(miob->current_write_avail() == 0);
+ CHECK(miob->high_water() == true);
+ CHECK(miob->current_low_water() == true);
+ CHECK(miob->write_avail() == 0); ///< should have no side effect
+
+ // consume half of the data
+ miob_r->consume(4096);
+ CHECK(miob->max_read_avail() == 4096);
+ CHECK(miob->current_write_avail() == 0);
+ CHECK(miob->high_water() == true);
+ CHECK(miob->current_low_water() == true);
+ CHECK(miob->write_avail() == 0); ///< should have no side effect
+
+ // consume all of the data
+ miob_r->consume(4096);
+ CHECK(miob->max_read_avail() == 0);
+ CHECK(miob->current_write_avail() == 0);
+ CHECK(miob->high_water() == false);
+ CHECK(miob->current_low_water() == true);
+ CHECK(miob->write_avail() == 4096); ///< should have a side effect: add a new block
+
+ CHECK(miob->max_read_avail() == 0);
+ CHECK(miob->current_write_avail() == 4096);
+ CHECK(miob->high_water() == false);
+ CHECK(miob->current_low_water() == false);
+ CHECK(miob->write_avail() == 4096); ///< should have no side effect
+ }
+
+ SECTION("water_mark == block_size()")
+ {
+ miob->water_mark = 4096;
+ REQUIRE(miob->water_mark == miob->block_size());
+
+ // fill half of the current buffer
+ miob->write(buf, 2048);
+ CHECK(miob->max_read_avail() == 2048);
+ CHECK(miob->current_write_avail() == 2048);
+ CHECK(miob->high_water() == false);
+ CHECK(miob->current_low_water() == true);
+ CHECK(miob->write_avail() == 6144); ///< should have a side effect: add a new block
+
+ CHECK(miob->max_read_avail() == 2048);
+ CHECK(miob->current_write_avail() == 6144);
+ CHECK(miob->high_water() == false);
+ CHECK(miob->current_low_water() == false);
+ CHECK(miob->write_avail() == 6144); ///< should have no side effect
+
+ // fill all of the current buffer
+ miob->write(buf, 6144);
+ CHECK(miob->max_read_avail() == 8192);
+ CHECK(miob->current_write_avail() == 0);
+ CHECK(miob->high_water() == true);
+ CHECK(miob->current_low_water() == true);
+ CHECK(miob->write_avail() == 0); ///< should have no side effect
+
+ // consume half of the data
+ miob_r->consume(4096);
+ CHECK(miob->max_read_avail() == 4096);
+ CHECK(miob->current_write_avail() == 0);
+ CHECK(miob->high_water() == false);
+ CHECK(miob->current_low_water() == true);
+ CHECK(miob->write_avail() == 4096); ///< should have a side effect: add a new block
+ IOBufferBlock *tail = miob->_writer->next.get();
+ CHECK(tail != nullptr);
+
+ CHECK(miob->max_read_avail() == 4096);
+ CHECK(miob->current_write_avail() == 4096);
+ CHECK(miob->high_water() == false);
+ CHECK(miob->current_low_water() == true);
+ CHECK(miob->write_avail() == 4096); ///< should have no side effect
+ CHECK(tail == miob->_writer->next.get()); ///< the tail block should not be changed
+
+ // consume all of the data
+ miob_r->consume(4096);
+ CHECK(miob->max_read_avail() == 0);
+ CHECK(miob->current_write_avail() == 4096);
+ CHECK(miob->high_water() == false);
+ CHECK(miob->current_low_water() == true);
+ CHECK(miob->write_avail() == 4096); ///< should have no side effect
+ CHECK(tail == miob->_writer->next.get()); ///< the tail block should not be changed
+ }
+
+ free_MIOBuffer(miob);
+ }
}
struct EventProcessorListener : Catch::TestEventListenerBase {