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 e24589f5a8 Reuse arena for header compression (#11024) e24589f5a8 is described below commit e24589f5a8ea383ce807eeccbe3c2c67a8de4f3f Author: Masakazu Kitajo <mas...@apache.org> AuthorDate: Wed Feb 7 09:02:58 2024 -0700 Reuse arena for header compression (#11024) --- include/proxy/http2/HPACK.h | 3 +++ include/proxy/http3/QPACK.h | 3 +++ src/proxy/http2/HPACK.cc | 8 ++++---- src/proxy/http3/QPACK.cc | 31 ++++++++++++++++++------------- 4 files changed, 28 insertions(+), 17 deletions(-) diff --git a/include/proxy/http2/HPACK.h b/include/proxy/http2/HPACK.h index f8e93856ce..ee8e25eba2 100644 --- a/include/proxy/http2/HPACK.h +++ b/include/proxy/http2/HPACK.h @@ -126,6 +126,9 @@ public: uint32_t size() const; void update_maximum_size(uint32_t new_size); + // Temporal buffer for internal use but it has to be public because many functions are not members of this class. + Arena arena; + private: XpackDynamicTable _dynamic_table; }; diff --git a/include/proxy/http3/QPACK.h b/include/proxy/http3/QPACK.h index 21b294149c..bae08bb7cf 100644 --- a/include/proxy/http3/QPACK.h +++ b/include/proxy/http3/QPACK.h @@ -282,4 +282,7 @@ private: MIOBuffer *_decoder_stream_sending_instructions; IOBufferReader *_encoder_stream_sending_instructions_reader; IOBufferReader *_decoder_stream_sending_instructions_reader; + + // Temporal buffer + Arena _arena; }; diff --git a/src/proxy/http2/HPACK.cc b/src/proxy/http2/HPACK.cc index db66c08387..ca98d30d43 100644 --- a/src/proxy/http2/HPACK.cc +++ b/src/proxy/http2/HPACK.cc @@ -592,8 +592,6 @@ decode_literal_header_field(MIMEFieldWrapper &header, const uint8_t *buf_start, p += len; - Arena arena; - // Decode header field name if (index) { if (indexing_table.get_header_field(index, header) == HPACK_ERROR_COMPRESSION_ERROR) { @@ -603,7 +601,7 @@ decode_literal_header_field(MIMEFieldWrapper &header, const uint8_t *buf_start, char *name_str = nullptr; uint64_t name_str_len = 0; - len = xpack_decode_string(arena, &name_str, name_str_len, p, buf_end); + len = xpack_decode_string(indexing_table.arena, &name_str, name_str_len, p, buf_end); if (len == XPACK_ERROR_COMPRESSION_ERROR) { return HPACK_ERROR_COMPRESSION_ERROR; } @@ -619,19 +617,21 @@ decode_literal_header_field(MIMEFieldWrapper &header, const uint8_t *buf_start, p += len; header.name_set(name_str, name_str_len); + indexing_table.arena.str_free(name_str); } // Decode header field value char *value_str = nullptr; uint64_t value_str_len = 0; - len = xpack_decode_string(arena, &value_str, value_str_len, p, buf_end); + len = xpack_decode_string(indexing_table.arena, &value_str, value_str_len, p, buf_end); if (len == XPACK_ERROR_COMPRESSION_ERROR) { return HPACK_ERROR_COMPRESSION_ERROR; } p += len; header.value_set(value_str, value_str_len); + indexing_table.arena.str_free(value_str); // Incremental Indexing adds header to header table as new entry if (isIncremental) { diff --git a/src/proxy/http3/QPACK.cc b/src/proxy/http3/QPACK.cc index 54cf3f5fac..1d316b10e0 100644 --- a/src/proxy/http3/QPACK.cc +++ b/src/proxy/http3/QPACK.cc @@ -346,10 +346,9 @@ QPACK::_encode_prefix(uint16_t largest_reference, uint16_t base_index, IOBufferB int QPACK::_encode_header(const MIMEField &field, uint16_t base_index, IOBufferBlock *compressed_header, uint16_t &referred_index) { - Arena arena; int name_len; const char *name = field.name_get(&name_len); - char *lowered_name = arena.str_store(name, name_len); + char *lowered_name = this->_arena.str_store(name, name_len); for (int i = 0; i < name_len; i++) { lowered_name[i] = ParseRules::ink_tolower(lowered_name[i]); } @@ -486,6 +485,8 @@ QPACK::_encode_header(const MIMEField &field, uint16_t base_index, IOBufferBlock value_len, value, never_index); } + this->_arena.str_free(lowered_name); + return 0; } @@ -739,10 +740,9 @@ QPACK::_decode_literal_header_field_with_name_ref(int16_t base_index, const uint } // Read value - Arena arena; char *value; uint64_t value_len; - if ((ret = xpack_decode_string(arena, &value, value_len, buf + read_len, buf + buf_len, 7)) < 0) { + if ((ret = xpack_decode_string(this->_arena, &value, value_len, buf + read_len, buf + buf_len, 7)) < 0) { return -1; } read_len += ret; @@ -754,6 +754,8 @@ QPACK::_decode_literal_header_field_with_name_ref(int16_t base_index, const uint QPACKDebug("Decoded Literal Header Field With Name Ref: base_index=%d, abs_index=%d, name=%.*s, value=%.*s", base_index, result.index, static_cast<int>(name_len), name, static_cast<int>(value_len), value); + this->_arena.str_free(value); + return read_len; } @@ -769,18 +771,17 @@ QPACK::_decode_literal_header_field_without_name_ref(const uint8_t *buf, size_t } // Read name and value - Arena arena; int64_t ret; char *name; uint64_t name_len; - if ((ret = xpack_decode_string(arena, &name, name_len, buf, buf + buf_len, 3)) < 0) { + if ((ret = xpack_decode_string(this->_arena, &name, name_len, buf, buf + buf_len, 3)) < 0) { return -1; } read_len += ret; char *value; uint64_t value_len; - if ((ret = xpack_decode_string(arena, &value, value_len, buf + read_len, buf + buf_len, 7)) < 0) { + if ((ret = xpack_decode_string(this->_arena, &value, value_len, buf + read_len, buf + buf_len, 7)) < 0) { return -1; } read_len += ret; @@ -792,6 +793,9 @@ QPACK::_decode_literal_header_field_without_name_ref(const uint8_t *buf, size_t QPACKDebug("Decoded Literal Header Field Without Name Ref: name=%.*s, value=%.*s", static_cast<uint16_t>(name_len), name, static_cast<uint16_t>(value_len), value); + this->_arena.str_free(name); + this->_arena.str_free(value); + return read_len; } @@ -865,10 +869,9 @@ QPACK::_decode_literal_header_field_with_postbase_name_ref(int16_t base_index, c } // Read value - Arena arena; char *value; uint64_t value_len; - if ((ret = xpack_decode_string(arena, &value, value_len, buf + read_len, buf + buf_len, 7)) < 0) { + if ((ret = xpack_decode_string(this->_arena, &value, value_len, buf + read_len, buf + buf_len, 7)) < 0) { return -1; } read_len += ret; @@ -880,6 +883,8 @@ QPACK::_decode_literal_header_field_with_postbase_name_ref(int16_t base_index, c QPACKDebug("Decoded Literal Header Field With Postbase Name Ref: base_index=%d, abs_index=%d, name=%.*s, value=%.*s", base_index, static_cast<uint16_t>(index), static_cast<int>(name_len), name, static_cast<int>(value_len), value); + this->_arena.str_free(value); + return read_len; } @@ -1110,14 +1115,13 @@ QPACK::_on_encoder_stream_read_ready(IOBufferReader &reader) if (buf & 0x80) { // Insert With Name Reference bool is_static; uint16_t index; - Arena arena; const char *name; size_t name_len; const char *dummy; size_t dummy_len; char *value; size_t value_len; - if (this->_read_insert_with_name_ref(reader, is_static, index, arena, &value, value_len) < 0) { + if (this->_read_insert_with_name_ref(reader, is_static, index, this->_arena, &value, value_len) < 0) { this->_abort_decode(); return EVENT_DONE; } @@ -1125,19 +1129,20 @@ QPACK::_on_encoder_stream_read_ready(IOBufferReader &reader) value); StaticTable::lookup(index, &name, &name_len, &dummy, &dummy_len); this->_dynamic_table.insert_entry(name, name_len, value, value_len); + this->_arena.str_free(value); } else if (buf & 0x40) { // Insert Without Name Reference - Arena arena; char *name; size_t name_len; char *value; size_t value_len; - if (this->_read_insert_without_name_ref(reader, arena, &name, name_len, &value, value_len) < 0) { + if (this->_read_insert_without_name_ref(reader, this->_arena, &name, name_len, &value, value_len) < 0) { this->_abort_decode(); return EVENT_DONE; } QPACKDebug("Received Insert Without Name Ref: name=%.*s, value=%.*s", static_cast<int>(name_len), name, static_cast<int>(value_len), value); this->_dynamic_table.insert_entry(name, name_len, value, value_len); + this->_arena.str_free(name); } else if (buf & 0x20) { // Dynamic Table Size Update uint16_t max_size; if (this->_read_dynamic_table_size_update(reader, max_size) < 0) {