Restore a shortcut in hot loop in _mime_hdr_field_list_search_by_string

[masaori335]

@c-taylor noticed the 10.2.x branch has regression of throughput performance. It looks like several changes made it but the first big drop is introduced by 3b0f0b760387af0c4f6fdd40c4bc98f14dbf0c4d. The shortcut in the hot loop seems important.

Below is comparison of 1 URL against 1 ET_NET thread performance.

commit	req/sec	note
0868475	14683.37	(parent of 3b0f0b7)
3b0f0b7	11024.33
3b0f0b7 + this patch	13535.39

[Copilot]

Pull request overview

This PR restores a fast-path check in the MIME header field linear search hot loop to reduce unnecessary case-insensitive string comparisons and recover throughput regression seen on the 10.2.x branch.

Changes:

• Add a name-length equality check before calling strcasecmp() in _mime_hdr_field_list_search_by_string().

[bryancall]

What strcasecmp() is it calling and look at inlining the function?

[masaori335]

This strcasecmp in the loop calls our strcasecmp(const std::string_view &lhs, const std::string_view &rhs) in the libswoc, which is inlined.

The problem is that this strcasecmp almost always call libc's ::strncasecmp, because the size comparison inside the function is only done to determine the size_t n passed as the 3rd argument. ( I thought the size comparison does early return in the beginning, but it doesn't )

trafficserver/lib/swoc/src/string_view_util.cc

Lines 29 to 47 in a26a2d8

	int
	strcasecmp(const std::string_view &lhs, const std::string_view &rhs) {
	int zret = 0;
	size_t n = rhs.size();
	// Seems a bit ugly but size comparisons must be done anyway to get the @c strncasecmp args.
	if (lhs.size() < rhs.size()) {
	zret = 1;
	n = lhs.size();
	} else if (lhs.size() > rhs.size()) {
	zret = -1;
	} else if (lhs.data() == rhs.data()) { // the same memory, obviously equal.
	return 0;
	}
	int r = ::strncasecmp(lhs.data(), rhs.data(), n);
	return r ? r : zret;
	}

[masaori335]

I introduced a new helper function, bool iequals(const std::string_view &lhs, const std::string_view &rhs), simple equality check case-insensitivly.

I found 3b0f0b7 did the same change in three places, so this PR restores the length check with ts::iequals.

git show 3b0f0b760387af0c4f6fdd40c4bc98f14dbf0c4d | grep -B 10 strcasecmp
 {
   MIMEFieldBlockImpl *fblock;
   MIMEField          *field, *too_far_field;
@@ -1277,8 +1117,9 @@ _mime_hdr_field_list_search_by_string(MIMEHdrImpl *mh, const char *field_name_st
 
     too_far_field = &(fblock->m_field_slots[fblock->m_freetop]);
     while (field < too_far_field) {
-      if (field->is_live() && (field_name_len == field->m_len_name) &&
-          (strncasecmp(field->m_ptr_name, field_name_str, field_name_len) == 0)) {
+      if (field->is_live() &&
+          strcasecmp(std::string_view{field->m_ptr_name, static_cast<std::string_view::size_type>(field->m_len_name)},
--
--- a/src/proxy/http/HttpSM.cc
+++ b/src/proxy/http/HttpSM.cc
@@ -735,9 +735,9 @@ HttpSM::state_read_client_request_header(int event, void *data)
     if (t_state.hdr_info.client_request.version_get() == HTTP_1_1 &&
         (t_state.hdr_info.client_request.method_get_wksidx() == HTTP_WKSIDX_POST ||
          t_state.hdr_info.client_request.method_get_wksidx() == HTTP_WKSIDX_PUT)) {
-      int         len    = 0;
-      const char *expect = t_state.hdr_info.client_request.value_get(MIME_FIELD_EXPECT, MIME_LEN_EXPECT, &len);
-      if ((len == HTTP_LEN_100_CONTINUE) && (strncasecmp(expect, HTTP_VALUE_100_CONTINUE, HTTP_LEN_100_CONTINUE) == 0)) {
+      auto expect{t_state.hdr_info.client_request.value_get(static_cast<std::string_view>(MIME_FIELD_EXPECT))};
+      if (strcasecmp(expect, std::string_view{HTTP_VALUE_100_CONTINUE,
--
@@ -882,11 +888,11 @@ HttpTransactHeaders::remove_conditional_headers(HTTPHdr *outgoing)
 void
 HttpTransactHeaders::remove_100_continue_headers(HttpTransact::State *s, HTTPHdr *outgoing)
 {
-  int         len    = 0;
-  const char *expect = s->hdr_info.client_request.value_get(MIME_FIELD_EXPECT, MIME_LEN_EXPECT, &len);
+  auto expect{s->hdr_info.client_request.value_get(static_cast<std::string_view>(MIME_FIELD_EXPECT))};
 
-  if ((len == HTTP_LEN_100_CONTINUE) && (strncasecmp(expect, HTTP_VALUE_100_CONTINUE, HTTP_LEN_100_CONTINUE) == 0)) {
-    outgoing->field_delete(MIME_FIELD_EXPECT, MIME_LEN_EXPECT);
+  if (strcasecmp(expect,

There're so many other libswoc strcasecmp usages we can replace, but it's out of scope from this PR.

[Copilot]

Pull request overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.

[masaori335]

Standard library doesn't have this, but boost has one. I followed the naming.
https://www.boost.org/doc/libs/latest/libs/beast/doc/html/beast/ref/boost__beast__iequals.html