From 68a6d8c50b8e1e61690880b417958e61e67355d9 Mon Sep 17 00:00:00 2001 From: Tatsuhiro Tsujikawa Date: Sat, 1 Oct 2016 18:18:50 +0900 Subject: [PATCH] nghttpx: Realloc header buffer --- src/allocator.h | 88 ++++++++++++++++++++++++++++++++---- src/shrpx_downstream.cc | 39 ++++++++++++---- src/shrpx_downstream.h | 10 ++++ src/shrpx_downstream_test.cc | 21 ++++++--- src/shrpx_https_upstream.cc | 9 +--- 5 files changed, 137 insertions(+), 30 deletions(-) diff --git a/src/allocator.h b/src/allocator.h index c3302efc..7fa0b8ec 100644 --- a/src/allocator.h +++ b/src/allocator.h @@ -29,6 +29,8 @@ #include +#include + #include "template.h" namespace nghttp2 { @@ -45,14 +47,18 @@ struct MemBlock { // BlockAllocator allocates memory block with given size at once, and // cuts the region from it when allocation is requested. If the -// requested size is larger than given threshold, it will be allocated -// in a distinct buffer on demand. +// requested size is larger than given threshold (plus small internal +// overhead), it will be allocated in a distinct buffer on demand. +// The |isolation_threshold| must be less than or equal to +// |block_size|. struct BlockAllocator { BlockAllocator(size_t block_size, size_t isolation_threshold) : retain(nullptr), head(nullptr), block_size(block_size), - isolation_threshold(std::min(block_size, isolation_threshold)) {} + isolation_threshold(std::min(block_size, isolation_threshold)) { + assert(isolation_threshold <= block_size); + } ~BlockAllocator() { reset(); } @@ -105,20 +111,60 @@ struct BlockAllocator { } void *alloc(size_t size) { - if (size >= isolation_threshold) { - auto mb = alloc_mem_block(size); + if (size + sizeof(size_t) >= isolation_threshold) { + auto len = std::max(static_cast(16), size); + // We will store the allocated size in size_t field. + auto mb = alloc_mem_block(len + sizeof(size_t)); + auto sp = reinterpret_cast(mb->begin); + *sp = len; mb->last = mb->end; - return mb->begin; + return mb->begin + sizeof(size_t); } - if (!head || head->end - head->last < static_cast(size)) { + if (!head || + head->end - head->last < static_cast(size + sizeof(size_t))) { head = alloc_mem_block(block_size); } - auto res = head->last; + // We will store the allocated size in size_t field. + auto res = head->last + sizeof(size_t); + auto sp = reinterpret_cast(head->last); + *sp = size; head->last = reinterpret_cast( - (reinterpret_cast(head->last + size) + 0xf) & ~0xf); + (reinterpret_cast(res + size) + 0xf) & ~0xf); + + return res; + } + + // Returns allocated size for memory pointed by |ptr|. We assume + // that |ptr| was returned from alloc() or realloc(). + size_t get_alloc_length(void *ptr) { + return *reinterpret_cast(static_cast(ptr) - + sizeof(size_t)); + } + + // Allocates memory of at least |size| bytes. If |ptr| is nullptr, + // this is equivalent to alloc(size). If |ptr| is not nullptr, + // obtain the allocated size for |ptr|, assuming that |ptr| was + // returned from alloc() or realloc(). If the allocated size is + // greater than or equal to size, |ptr| is returned. Otherwise, + // allocates at least |size| bytes of memory, and the original + // content pointed by |ptr| is copied to the newly allocated memory. + void *realloc(void *ptr, size_t size) { + if (!ptr) { + return alloc(size); + } + auto alloclen = get_alloc_length(ptr); + auto p = reinterpret_cast(ptr); + if (size <= alloclen) { + return ptr; + } + + auto nalloclen = std::max(size + 1, alloclen * 2); + + auto res = alloc(nalloclen); + std::copy_n(p, alloclen, static_cast(res)); return res; } @@ -186,6 +232,30 @@ StringRef concat_string_ref(BlockAllocator &alloc, Args &&... args) { return StringRef{dst, len}; } +// Returns the string which is the concatenation of |value| and |args| +// in the given order. The resulting string will be NULL-terminated. +// This function assumes that the pointer value value.c_str() was +// obtained from alloc.alloc() or alloc.realloc(), and attempts to use +// unused memory region by using alloc.realloc(). If value is empty, +// then just call concat_string_ref(). +template +StringRef realloc_concat_string_ref(BlockAllocator &alloc, + const StringRef &value, Args &&... args) { + if (value.empty()) { + return concat_string_ref(alloc, std::forward(args)...); + } + + auto len = + value.size() + concat_string_ref_count(0, std::forward(args)...); + auto dst = static_cast( + alloc.realloc(const_cast(value.byte()), len + 1)); + auto p = dst + value.size(); + p = concat_string_ref_copy(p, std::forward(args)...); + *p = '\0'; + + return StringRef{dst, len}; +} + struct ByteRef { // The pointer to the beginning of the buffer. uint8_t *base; diff --git a/src/shrpx_downstream.cc b/src/shrpx_downstream.cc index 27cb8a18..3f8dfdba 100644 --- a/src/shrpx_downstream.cc +++ b/src/shrpx_downstream.cc @@ -359,21 +359,31 @@ void add_header(bool &key_prev, size_t &sum, HeaderRefs &headers, } } // namespace +namespace { +StringRef alloc_header_name(BlockAllocator &balloc, const StringRef &name) { + auto iov = make_byte_ref(balloc, name.size() + 1); + auto p = iov.base; + p = std::copy(std::begin(name), std::end(name), p); + util::inp_strlower(iov.base, p); + *p = '\0'; + + return StringRef{iov.base, p}; +} +} // namespace + namespace { void append_last_header_key(BlockAllocator &balloc, bool &key_prev, size_t &sum, HeaderRefs &headers, const char *data, size_t len) { assert(key_prev); sum += len; auto &item = headers.back(); - auto iov = make_byte_ref(balloc, item.name.size() + len + 1); - auto p = iov.base; - p = std::copy(std::begin(item.name), std::end(item.name), p); - p = std::copy_n(data, len, p); - util::inp_strlower(p - len, p); - *p = '\0'; + auto name = + realloc_concat_string_ref(balloc, item.name, StringRef{data, len}); - item.name = StringRef{iov.base, p}; + auto p = const_cast(name.byte()); + util::inp_strlower(p + name.size() - len, p + name.size()); + item.name = name; item.token = http2::lookup_token(item.name); } } // namespace @@ -385,7 +395,8 @@ void append_last_header_value(BlockAllocator &balloc, bool &key_prev, key_prev = false; sum += len; auto &item = headers.back(); - item.value = concat_string_ref(balloc, item.value, StringRef{data, len}); + item.value = + realloc_concat_string_ref(balloc, item.value, StringRef{data, len}); } } // namespace @@ -439,6 +450,12 @@ void FieldStore::add_header_token(const StringRef &name, const StringRef &value, no_index, token); } +void FieldStore::alloc_add_header_name(const StringRef &name) { + auto name_ref = alloc_header_name(balloc_, name); + auto token = http2::lookup_token(name_ref); + add_header_token(name_ref, StringRef{}, false, token); +} + void FieldStore::append_last_header_key(const char *data, size_t len) { shrpx::append_last_header_key(balloc_, header_key_prev_, buffer_size_, headers_, data, len); @@ -460,6 +477,12 @@ void FieldStore::add_trailer_token(const StringRef &name, no_index, token); } +void FieldStore::alloc_add_trailer_name(const StringRef &name) { + auto name_ref = alloc_header_name(balloc_, name); + auto token = http2::lookup_token(name_ref); + add_trailer_token(name_ref, StringRef{}, false, token); +} + void FieldStore::append_last_trailer_key(const char *data, size_t len) { shrpx::append_last_header_key(balloc_, trailer_key_prev_, buffer_size_, trailers_, data, len); diff --git a/src/shrpx_downstream.h b/src/shrpx_downstream.h index d5514ea8..21c253da 100644 --- a/src/shrpx_downstream.h +++ b/src/shrpx_downstream.h @@ -86,6 +86,11 @@ public: void add_header_token(const StringRef &name, const StringRef &value, bool no_index, int32_t token); + // Adds header field name |name|. First, the copy of header field + // name pointed by name.c_str() of length name.size() is made, and + // stored. + void alloc_add_header_name(const StringRef &name); + void append_last_header_key(const char *data, size_t len); void append_last_header_value(const char *data, size_t len); @@ -101,6 +106,11 @@ public: void add_trailer_token(const StringRef &name, const StringRef &value, bool no_index, int32_t token); + // Adds trailer field name |name|. First, the copy of trailer field + // name pointed by name.c_str() of length name.size() is made, and + // stored. + void alloc_add_trailer_name(const StringRef &name); + void append_last_trailer_key(const char *data, size_t len); void append_last_trailer_value(const char *data, size_t len); diff --git a/src/shrpx_downstream_test.cc b/src/shrpx_downstream_test.cc index 786e9046..77895305 100644 --- a/src/shrpx_downstream_test.cc +++ b/src/shrpx_downstream_test.cc @@ -33,26 +33,35 @@ namespace shrpx { void test_downstream_field_store_append_last_header(void) { - BlockAllocator balloc(4096, 4096); + BlockAllocator balloc(16, 16); FieldStore fs(balloc, 0); - fs.add_header_token(StringRef::from_lit("alpha"), StringRef{}, false, -1); + fs.alloc_add_header_name(StringRef::from_lit("alpha")); auto bravo = StringRef::from_lit("BRAVO"); fs.append_last_header_key(bravo.c_str(), bravo.size()); + // Add more characters so that relloc occurs + auto golf = StringRef::from_lit("golF0123456789"); + fs.append_last_header_key(golf.c_str(), golf.size()); + auto charlie = StringRef::from_lit("Charlie"); fs.append_last_header_value(charlie.c_str(), charlie.size()); auto delta = StringRef::from_lit("deltA"); fs.append_last_header_value(delta.c_str(), delta.size()); + // Add more characters so that relloc occurs + auto echo = StringRef::from_lit("echo0123456789"); + fs.append_last_header_value(echo.c_str(), echo.size()); + fs.add_header_token(StringRef::from_lit("echo"), StringRef::from_lit("foxtrot"), false, -1); - auto ans = HeaderRefs{ - {StringRef::from_lit("alphabravo"), StringRef::from_lit("CharliedeltA")}, - {StringRef::from_lit("echo"), StringRef::from_lit("foxtrot")}}; + auto ans = + HeaderRefs{{StringRef::from_lit("alphabravogolf0123456789"), + StringRef::from_lit("CharliedeltAecho0123456789")}, + {StringRef::from_lit("echo"), StringRef::from_lit("foxtrot")}}; CU_ASSERT(ans == fs.headers()); } void test_downstream_field_store_header(void) { - BlockAllocator balloc(4096, 4096); + BlockAllocator balloc(16, 16); FieldStore fs(balloc, 0); fs.add_header_token(StringRef::from_lit("alpha"), StringRef::from_lit("0"), false, -1); diff --git a/src/shrpx_https_upstream.cc b/src/shrpx_https_upstream.cc index 863f8d62..9fce95ce 100644 --- a/src/shrpx_https_upstream.cc +++ b/src/shrpx_https_upstream.cc @@ -123,7 +123,6 @@ int htp_hdr_keycb(http_parser *htp, const char *data, size_t len) { auto downstream = upstream->get_downstream(); auto &req = downstream->request(); auto &httpconf = get_config()->http; - auto &balloc = downstream->get_block_allocator(); if (req.fs.buffer_size() + len > httpconf.request_header_field_buffer) { if (LOG_ENABLED(INFO)) { @@ -148,9 +147,7 @@ int htp_hdr_keycb(http_parser *htp, const char *data, size_t len) { Downstream::HTTP1_REQUEST_HEADER_TOO_LARGE); return -1; } - auto name = http2::copy_lower(balloc, StringRef{data, len}); - auto token = http2::lookup_token(name); - req.fs.add_header_token(name, StringRef{}, false, token); + req.fs.alloc_add_header_name(StringRef{data, len}); } } else { // trailer part @@ -164,9 +161,7 @@ int htp_hdr_keycb(http_parser *htp, const char *data, size_t len) { } return -1; } - auto name = http2::copy_lower(balloc, StringRef{data, len}); - auto token = http2::lookup_token(name); - req.fs.add_trailer_token(name, StringRef{}, false, token); + req.fs.alloc_add_trailer_name(StringRef{data, len}); } } return 0;