This is an automated email from the ASF dual-hosted git repository. amc 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 5ad8eec RecHttp: Convert protocol session pool to use views and MemArena. 5ad8eec is described below commit 5ad8eec303b5f9c38da0de3775e0aadb7186fc38 Author: Alan M. Carroll <a...@apache.org> AuthorDate: Thu Feb 14 15:06:58 2019 -0600 RecHttp: Convert protocol session pool to use views and MemArena. --- lib/records/I_RecHttp.h | 72 ++++++++++++++++++++++++------------------------- lib/records/RecHttp.cc | 58 ++++++++++++++++----------------------- 2 files changed, 58 insertions(+), 72 deletions(-) diff --git a/lib/records/I_RecHttp.h b/lib/records/I_RecHttp.h index 1c95ef7..c3db5a7 100644 --- a/lib/records/I_RecHttp.h +++ b/lib/records/I_RecHttp.h @@ -29,8 +29,9 @@ #include "ts/apidefs.h" #include "tscore/ink_assert.h" #include "tscore/IpMap.h" +#include "tscore/MemArena.h" #include <algorithm> -#include <vector> +#include <array> /// Load default inbound IP addresses from the configuration file. void RecHttpLoadIp(const char *name, ///< Name of value in configuration file. @@ -48,18 +49,17 @@ void RecHttpLoadIpMap(const char *name, ///< Name of value in configuration file */ class SessionProtocolSet { - typedef SessionProtocolSet self; ///< Self reference type. + using self_type = SessionProtocolSet; ///< Self reference type. /// Storage for the set - a bit vector. - uint32_t m_bits; + uint32_t m_bits = 0; public: - // The right way. - // static int const MAX = sizeof(m_bits) * CHAR_BIT; - // The RHEL5/gcc 4.1.2 way - static int const MAX = sizeof(uint32_t) * 8; + static constexpr int MAX = sizeof(m_bits) * CHAR_BIT; + /// Default constructor. /// Constructs and empty set. - SessionProtocolSet() : m_bits(0) {} + SessionProtocolSet() = default; + uint32_t indexToMask(int idx) const { @@ -72,9 +72,10 @@ public: { m_bits |= this->indexToMask(idx); } + /// Mark all the protocols in @a that as present in @a this. void - markIn(self const &that) + markIn(self_type const &that) { m_bits |= that.m_bits; } @@ -86,7 +87,7 @@ public: } /// Mark the protocols in @a that as not in @a this. void - markOut(self const &that) + markOut(self_type const &that) { m_bits &= ~(that.m_bits); } @@ -98,7 +99,7 @@ public: } /// Test if all the protocols in @a that are in @a this protocol set. bool - contains(self const &that) const + contains(self_type const &that) const { return that.m_bits == (that.m_bits & m_bits); } @@ -117,7 +118,7 @@ public: /// Check for intersection. bool - intersects(self const &that) + intersects(self_type const &that) { return 0 != (m_bits & that.m_bits); } @@ -131,7 +132,7 @@ public: /// Equality (identical sets). bool - operator==(self const &that) const + operator==(self_type const &that) const { return m_bits == that.m_bits; } @@ -147,52 +148,51 @@ const char *RecNormalizeProtoTag(const char *tag); /** Registered session protocol names. - We do this to avoid lots of string compares. By normalizing the - string names we can just compare their indices in this table. + We do this to avoid lots of string compares. By normalizing the string names we can just compare + their indices in this table. + + @internal To simplify the implementation we limit the maximum number of strings to 32. That will + be sufficient for the forseeable future. We can come back to this if it ever becomes a problem. - @internal To simplify the implementation we limit the maximum - number of strings to 32. That will be sufficient for the forseeable - future. We can come back to this if it ever becomes a problem. + @internal Because we have so few strings we just use a linear search. If the size gets much + larger we should consider doing something more clever. - @internal Because we have so few strings we just use a linear search. - If the size gets much larger we should consider doing something more - clever. + @internal This supports providing constant strings because those strings are exported to the + C API and this logic @b must return exactly those pointers. */ class SessionProtocolNameRegistry { public: - static int const MAX = SessionProtocolSet::MAX; ///< Maximum # of registered names. - static int const INVALID = -1; ///< Normalized invalid index value. + static int constexpr MAX = SessionProtocolSet::MAX; ///< Maximum # of registered names. + static int constexpr INVALID = -1; ///< Normalized invalid index value. + + using TextView = ts::TextView; /// Default constructor. /// Creates empty registry with no names. - SessionProtocolNameRegistry(); - - /// Destructor. - /// Cleans up strings. - ~SessionProtocolNameRegistry(); + SessionProtocolNameRegistry() = default; /** Get the index for @a name, registering it if needed. The name is copied internally. @return The index for the registered @a name. */ - int toIndex(const char *name); + int toIndex(TextView name); /** Get the index for @a name, registering it if needed. The caller @b guarantees @a name is persistent and immutable. @return The index for the registered @a name. */ - int toIndexConst(const char *name); + int toIndexConst(TextView name); /** Convert a @a name to an index. @return The index for @a name or @c INVALID if it is not registered. */ - int indexFor(const char *name) const; + int indexFor(TextView name) const; /** Convert an @a index to the corresponding name. @return A pointer to the name or @c nullptr if the index isn't registered. */ - const char *nameFor(int index) const; + TextView nameFor(int index) const; /// Mark protocols as present in @a sp_set based on the names in @a value. /// The names can be separated by ;/|,: and space. @@ -201,11 +201,9 @@ public: void markIn(const char *value, SessionProtocolSet &sp_set); protected: - unsigned int m_n; ///< Index of first unused slot. - const char *m_names[MAX]; ///< Pointers to registered names. - uint8_t m_flags[MAX]; ///< Flags for each name. - - static uint8_t const F_ALLOCATED = 0x1; ///< Flag for allocated by this instance. + int m_n = 0; ///< Index of first unused slot. + std::array<TextView, MAX> m_names; + ts::MemArena m_arena; ///< Storage for non-constant strings. }; extern SessionProtocolNameRegistry globalSessionProtocolNameRegistry; diff --git a/lib/records/RecHttp.cc b/lib/records/RecHttp.cc index 64193b8..d20ab3a 100644 --- a/lib/records/RecHttp.cc +++ b/lib/records/RecHttp.cc @@ -500,7 +500,7 @@ SessionProtocolNameRegistry::markIn(const char *value, SessionProtocolSet &sp_se } else if (0 == strcasecmp(elt, TS_ALPN_PROTOCOL_GROUP_HTTP2)) { sp_set.markIn(HTTP2_PROTOCOL_SET); } else { // user defined - register and mark. - int idx = globalSessionProtocolNameRegistry.toIndex(elt); + int idx = globalSessionProtocolNameRegistry.toIndex(TextView{elt, strlen(elt)}); sp_set.markIn(idx); } } @@ -651,7 +651,8 @@ HttpProxyPort::print(char *out, size_t n) bool sep_p = !need_colon_p; for (int k = 0; k < SessionProtocolSet::MAX; ++k) { if (sp_set.contains(k)) { - zret += snprintf(out + zret, n - zret, "%s%s", sep_p ? ";" : "", globalSessionProtocolNameRegistry.nameFor(k)); + auto name{globalSessionProtocolNameRegistry.nameFor(k)}; + zret += snprintf(out + zret, n - zret, "%s%.*s", sep_p ? ";" : "", static_cast<int>(name.size()), name.data()); sep_p = true; } } @@ -678,10 +679,10 @@ void ts_session_protocol_well_known_name_indices_init() { // register all the well known protocols and get the indices set. - TS_ALPN_PROTOCOL_INDEX_HTTP_0_9 = globalSessionProtocolNameRegistry.toIndexConst(TS_ALPN_PROTOCOL_HTTP_0_9); - TS_ALPN_PROTOCOL_INDEX_HTTP_1_0 = globalSessionProtocolNameRegistry.toIndexConst(TS_ALPN_PROTOCOL_HTTP_1_0); - TS_ALPN_PROTOCOL_INDEX_HTTP_1_1 = globalSessionProtocolNameRegistry.toIndexConst(TS_ALPN_PROTOCOL_HTTP_1_1); - TS_ALPN_PROTOCOL_INDEX_HTTP_2_0 = globalSessionProtocolNameRegistry.toIndexConst(TS_ALPN_PROTOCOL_HTTP_2_0); + TS_ALPN_PROTOCOL_INDEX_HTTP_0_9 = globalSessionProtocolNameRegistry.toIndexConst(std::string_view{TS_ALPN_PROTOCOL_HTTP_0_9}); + TS_ALPN_PROTOCOL_INDEX_HTTP_1_0 = globalSessionProtocolNameRegistry.toIndexConst(std::string_view{TS_ALPN_PROTOCOL_HTTP_1_0}); + TS_ALPN_PROTOCOL_INDEX_HTTP_1_1 = globalSessionProtocolNameRegistry.toIndexConst(std::string_view{TS_ALPN_PROTOCOL_HTTP_1_1}); + TS_ALPN_PROTOCOL_INDEX_HTTP_2_0 = globalSessionProtocolNameRegistry.toIndexConst(std::string_view{TS_ALPN_PROTOCOL_HTTP_2_0}); // Now do the predefined protocol sets. HTTP_PROTOCOL_SET.markIn(TS_ALPN_PROTOCOL_INDEX_HTTP_0_9); @@ -713,30 +714,18 @@ RecNormalizeProtoTag(const char *tag) return findResult == TSProtoTags.end() ? nullptr : findResult->data(); } -SessionProtocolNameRegistry::SessionProtocolNameRegistry() : m_n(0) -{ - memset(m_names, 0, sizeof(m_names)); - memset(&m_flags, 0, sizeof(m_flags)); -} - -SessionProtocolNameRegistry::~SessionProtocolNameRegistry() -{ - for (size_t i = 0; i < m_n; ++i) { - if (m_flags[i] & F_ALLOCATED) { - ats_free(const_cast<char *>(m_names[i])); // blech - ats_free won't take a const char* - } - } -} - int -SessionProtocolNameRegistry::toIndex(const char *name) +SessionProtocolNameRegistry::toIndex(ts::TextView name) { int zret = this->indexFor(name); if (INVALID == zret) { - if (m_n < static_cast<size_t>(MAX)) { - m_names[m_n] = ats_strdup(name); - m_flags[m_n] = F_ALLOCATED; - zret = m_n++; + if (m_n < MAX) { + // Localize the name by copying it in to the arena. + auto text = m_arena.alloc(name.size() + 1); + memcpy(text.data(), name.data(), name.size()); + text.end()[-1] = '\0'; + m_names[m_n] = text.view(); + zret = m_n++; } else { ink_release_assert(!"Session protocol name registry overflow"); } @@ -745,11 +734,11 @@ SessionProtocolNameRegistry::toIndex(const char *name) } int -SessionProtocolNameRegistry::toIndexConst(const char *name) +SessionProtocolNameRegistry::toIndexConst(TextView name) { int zret = this->indexFor(name); if (INVALID == zret) { - if (m_n < static_cast<size_t>(MAX)) { + if (m_n < MAX) { m_names[m_n] = name; zret = m_n++; } else { @@ -760,18 +749,17 @@ SessionProtocolNameRegistry::toIndexConst(const char *name) } int -SessionProtocolNameRegistry::indexFor(const char *name) const +SessionProtocolNameRegistry::indexFor(TextView name) const { - for (size_t i = 0; i < m_n; ++i) { - if (0 == strcasecmp(name, m_names[i])) { - return i; - } + auto spot = std::find(m_names.begin(), m_names.begin() + m_n, name); + if (spot != m_names.end()) { + return static_cast<int>(spot - m_names.begin()); } return INVALID; } -const char * +ts::TextView SessionProtocolNameRegistry::nameFor(int idx) const { - return 0 <= idx && idx < static_cast<int>(m_n) ? m_names[idx] : nullptr; + return 0 <= idx && idx < m_n ? m_names[idx] : TextView{}; }