This is an automated email from the ASF dual-hosted git repository.
maskit 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 d7097633f6 http3: Remove an unnecessary copy (#11775)
d7097633f6 is described below
commit d7097633f67bc45d9be8113701ca6f55f9f0d967
Author: Masakazu Kitajo <mas...@apache.org>
AuthorDate: Wed Sep 18 08:55:31 2024 -0600
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.
---
include/proxy/http3/Http3Frame.h | 12 ++---
src/proxy/http3/Http3Frame.cc | 85 +++++++++++----------------------
src/proxy/http3/test/test_Http3Frame.cc | 11 +++--
3 files changed, 38 insertions(+), 70 deletions(-)
diff --git a/include/proxy/http3/Http3Frame.h b/include/proxy/http3/Http3Frame.h
index 08ac54115a..d1de4e5efd 100644
--- a/include/proxy/http3/Http3Frame.h
+++ b/include/proxy/http3/Http3Frame.h
@@ -85,20 +85,19 @@ class Http3DataFrame : public Http3Frame
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 @@ public:
/*
* 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 9eb99fb1b7..8fe815b217 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 @@ Http3UnknownFrame::to_io_buffer_block() const
//
// 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)
-{
- this->_length = this->_payload_len;
- this->_payload = this->_payload_uptr.get();
-}
-
-Ptr<IOBufferBlock>
-Http3DataFrame::to_io_buffer_block() const
+// For sending frame
+Http3DataFrame::Http3DataFrame(IOBufferReader &reader, size_t payload_len)
+ : Http3Frame(Http3FrameType::DATA), _payload_len(payload_len)
{
- Ptr<IOBufferBlock> block;
- size_t n = 0;
- size_t written = 0;
+ uint8_t header[HEADER_OVERHEAD];
+ size_t n = 0;
+ size_t written = 0;
+ MIOBuffer *buf;
- 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());
+ this->_length = this->_payload_len;
- QUICVariableInt::encode(block_start, UINT64_MAX, n,
static_cast<uint64_t>(this->_type));
+ 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(block_start + written, UINT64_MAX, n, this->_length);
+ QUICVariableInt::encode(header + 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;
+ buf->write(header, written);
+ buf->write(&reader, payload_len);
+ free_MIOBuffer(buf);
+}
+
+Ptr<IOBufferBlock>
+Http3DataFrame::to_io_buffer_block() const
+{
+ return this->_whole_frame;
}
void
@@ -242,16 +245,10 @@ Http3DataFrame::reset(IOBufferReader &reader)
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 *
@@ -589,40 +586,12 @@ Http3FrameFactory::create_headers_frame(IOBufferReader
*header_block_reader, siz
return Http3HeadersFrameUPtr(frame,
&Http3FrameDeleter::delete_headers_frame);
}
-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 c28267f27d..86140db700 100644
--- a/src/proxy/http3/test/test_Http3Frame.cc
+++ b/src/proxy/http3/test/test_Http3Frame.cc
@@ -75,11 +75,12 @@ TEST_CASE("Store DATA Frame", "[http3]")
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();
