http3: Remove an unnecessary copy (#11775)
This uses more IOBufferBlocks, but it wouldn't be problematic unlike H2 because the size of Data frame is much larger than H2.
diff --git a/include/proxy/http3/Http3Frame.h b/include/proxy/http3/Http3Frame.h
index 08ac541..d1de4e5 100644
--- a/include/proxy/http3/Http3Frame.h
+++ b/include/proxy/http3/Http3Frame.h
@@ -85,20 +85,19 @@
public:
Http3DataFrame() : Http3Frame() {}
Http3DataFrame(IOBufferReader &reader);
- Http3DataFrame(ats_unique_buf payload, size_t payload_len);
+ Http3DataFrame(IOBufferReader &reader, size_t payload_len);
Ptr<IOBufferBlock> to_io_buffer_block() const override;
void reset(IOBufferReader &reader) override;
- const uint8_t *payload() const;
- uint64_t payload_length() const;
+ uint64_t payload_length() const;
IOBufferReader *data() const;
private:
- const uint8_t *_payload = nullptr;
- ats_unique_buf _payload_uptr = {nullptr};
- size_t _payload_len = 0;
+ // Head of IOBufferBlock chain to send
+ Ptr<IOBufferBlock> _whole_frame;
+ uint64_t _payload_len = 0;
};
//
@@ -251,7 +250,6 @@
/*
* Creates a DATA frame.
*/
- static Http3DataFrameUPtr create_data_frame(const uint8_t *data, size_t data_len);
static Http3DataFrameUPtr create_data_frame(IOBufferReader *reader, size_t data_len);
private:
diff --git a/src/proxy/http3/Http3Frame.cc b/src/proxy/http3/Http3Frame.cc
index 9eb99fb..8fe815b 100644
--- a/src/proxy/http3/Http3Frame.cc
+++ b/src/proxy/http3/Http3Frame.cc
@@ -23,6 +23,7 @@
#include "tscore/Diags.h"
#include "iocore/net/quic/QUICIntUtil.h"
+#include "iocore/eventsystem/IOBuffer.h"
#include "proxy/http3/Http3Frame.h"
#include "proxy/http3/Http3Config.h"
@@ -201,38 +202,40 @@
//
// DATA Frame
//
+
+// For receiving frame
Http3DataFrame::Http3DataFrame(IOBufferReader &reader) : Http3Frame(reader)
{
this->_payload_len = this->_length;
}
-Http3DataFrame::Http3DataFrame(ats_unique_buf payload, size_t payload_len)
- : Http3Frame(Http3FrameType::DATA), _payload_uptr(std::move(payload)), _payload_len(payload_len)
+// For sending frame
+Http3DataFrame::Http3DataFrame(IOBufferReader &reader, size_t payload_len)
+ : Http3Frame(Http3FrameType::DATA), _payload_len(payload_len)
{
- this->_length = this->_payload_len;
- this->_payload = this->_payload_uptr.get();
+ uint8_t header[HEADER_OVERHEAD];
+ size_t n = 0;
+ size_t written = 0;
+ MIOBuffer *buf;
+
+ this->_length = this->_payload_len;
+
+ buf = new_MIOBuffer(BUFFER_SIZE_INDEX_32K);
+ this->_whole_frame = buf->alloc_reader()->get_current_block();
+ QUICVariableInt::encode(header, UINT64_MAX, n, static_cast<uint64_t>(this->_type));
+ written += n;
+ QUICVariableInt::encode(header + written, UINT64_MAX, n, this->_length);
+ written += n;
+
+ buf->write(header, written);
+ buf->write(&reader, payload_len);
+ free_MIOBuffer(buf);
}
Ptr<IOBufferBlock>
Http3DataFrame::to_io_buffer_block() const
{
- Ptr<IOBufferBlock> block;
- size_t n = 0;
- size_t written = 0;
-
- block = make_ptr<IOBufferBlock>(new_IOBufferBlock());
- block->alloc(iobuffer_size_to_index(HEADER_OVERHEAD + this->length(), BUFFER_SIZE_INDEX_32K));
- uint8_t *block_start = reinterpret_cast<uint8_t *>(block->start());
-
- QUICVariableInt::encode(block_start, UINT64_MAX, n, static_cast<uint64_t>(this->_type));
- written += n;
- QUICVariableInt::encode(block_start + written, UINT64_MAX, n, this->_length);
- written += n;
- memcpy(block_start + written, this->_payload, this->_payload_len);
- written += this->_payload_len;
-
- block->fill(written);
- return block;
+ return this->_whole_frame;
}
void
@@ -242,16 +245,10 @@
new (this) Http3DataFrame(reader);
}
-const uint8_t *
-Http3DataFrame::payload() const
-{
- return this->_payload;
-}
-
uint64_t
Http3DataFrame::payload_length() const
{
- return this->_payload_len;
+ return this->_length;
}
IOBufferReader *
@@ -590,39 +587,11 @@
}
Http3DataFrameUPtr
-Http3FrameFactory::create_data_frame(const uint8_t *payload, size_t payload_len)
-{
- ats_unique_buf buf = ats_unique_malloc(payload_len);
- memcpy(buf.get(), payload, payload_len);
-
- Http3DataFrame *frame = http3DataFrameAllocator.alloc();
- new (frame) Http3DataFrame(std::move(buf), payload_len);
- return Http3DataFrameUPtr(frame, &Http3FrameDeleter::delete_data_frame);
-}
-
-// TODO: This should clone IOBufferBlock chain to avoid memcpy
-Http3DataFrameUPtr
Http3FrameFactory::create_data_frame(IOBufferReader *reader, size_t payload_len)
{
- ats_unique_buf buf = ats_unique_malloc(payload_len);
- size_t written = 0;
-
- while (written < payload_len) {
- int64_t len = reader->block_read_avail();
-
- if (written + len > payload_len) {
- len = payload_len - written;
- }
-
- memcpy(buf.get() + written, reinterpret_cast<uint8_t *>(reader->start()), len);
- reader->consume(len);
- written += len;
- }
-
- ink_assert(written == payload_len);
-
Http3DataFrame *frame = http3DataFrameAllocator.alloc();
- new (frame) Http3DataFrame(std::move(buf), payload_len);
+ new (frame) Http3DataFrame(*reader, payload_len);
+ reader->consume(payload_len);
return Http3DataFrameUPtr(frame, &Http3FrameDeleter::delete_data_frame);
}
diff --git a/src/proxy/http3/test/test_Http3Frame.cc b/src/proxy/http3/test/test_Http3Frame.cc
index c28267f..86140db 100644
--- a/src/proxy/http3/test/test_Http3Frame.cc
+++ b/src/proxy/http3/test/test_Http3Frame.cc
@@ -75,11 +75,12 @@
0x11, 0x22, 0x33, 0x44, // Payload
};
- uint8_t raw1[] = "\x11\x22\x33\x44";
- ats_unique_buf payload1 = ats_unique_malloc(4);
- memcpy(payload1.get(), raw1, 4);
-
- Http3DataFrame data_frame(std::move(payload1), 4);
+ uint8_t raw1[] = "\x11\x22\x33\x44";
+ MIOBuffer *payload1 = new_MIOBuffer(BUFFER_SIZE_INDEX_8K);
+ payload1->set(raw1, 4);
+ IOBufferReader *payload1_reader = payload1->alloc_reader();
+ Http3DataFrame data_frame(*payload1_reader, 4);
+ free_MIOBuffer(payload1);
CHECK(data_frame.length() == 4);
auto ibb = data_frame.to_io_buffer_block();
