[GitHub] [trafficserver] bneradt commented on a diff in pull request #9297: Optimize HPACK string comparison

[GitBox <gi...@apache.org>]

bneradt commented on code in PR #9297:
URL: https://github.com/apache/trafficserver/pull/9297#discussion_r1072938445


##########
proxy/http2/HPACK.cc:
##########
@@ -189,6 +189,44 @@ hpack_field_is_literal(HpackField ftype)
   return ftype == HpackField::INDEXED_LITERAL || ftype == HpackField::NOINDEX_LITERAL || ftype == HpackField::NEVERINDEX_LITERAL;
 }
 
+// Try not to use memcmp(sv, sv) and strncasecmp(sv, sv) because we don't care which value comes first on a dictionary.
+// Return immediately if the lengths of given strings don't match.
+static inline bool
+match(const char *s1, int s1_len, const char *s2, int s2_len)

Review Comment:
   It seems like the caller of these functions would be better served with these functions taking std::string_view parameters.



##########
proxy/http2/HPACK.cc:
##########
@@ -228,12 +266,12 @@ namespace HpackStaticTable
     HpackLookupResult result;
 
     for (unsigned int index = 1; index < TS_HPACK_STATIC_TABLE_ENTRY_NUM; ++index) {
-      std::string_view name  = STATIC_TABLE[index].name;
-      std::string_view value = STATIC_TABLE[index].value;
+      const std::string_view &name  = STATIC_TABLE[index].name;
+      const std::string_view &value = STATIC_TABLE[index].value;

Review Comment:
   Is the const reference cheaper than copying the pointer and length values of original `name` and `value` types, as the original did? Each time these are accessed, it will now require a pointer dereference.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: github-unsubscribe@trafficserver.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org