fixup! Refine the jsonResponse bench a bit more

Author: jasnell

src/workerd/api/headers.c++

@@ -74,22 +74,11 @@ static constexpr size_t MAX_COMMON_HEADER_ID =
 // and must be kept in sync with the ordinal values defined in http-over-capnp.capnp). Since
 // it is extremely unlikely that those will change often, we hardcode them here for runtime
 // efficiency.
-#define V(Name) Name,
-static constexpr const char* COMMON_HEADER_NAMES[] = {nullptr,  // 0: invalid
+#define V(Name) Name##_kj,
+static constexpr kj::StringPtr COMMON_HEADER_NAMES[] = {nullptr,  // 0: invalid
   COMMON_HEADERS(V)};
 #undef V
 
-constexpr size_t constexprStrlen(const char* str) {
-  return *str ? 1 + constexprStrlen(str + 1) : 0;
-}
-
-// Helper to avoid recalculating lengths of common headers at runtime repeatedly
-static constexpr size_t COMMON_HEADER_NAME_LENGTHS[] = {0,  // 0: invalid (nullptr)
-#define V(n) constexprStrlen(n),
-  COMMON_HEADERS(V)
-#undef V
-};
-
 inline constexpr kj::StringPtr getCommonHeaderName(uint id) {
   KJ_ASSERT(id > 0 && id <= MAX_COMMON_HEADER_ID, "Invalid common header ID");
   kj::StringPtr name = COMMON_HEADER_NAMES[id];
@@ -107,8 +96,8 @@ constexpr kj::Maybe<uint> getCommonHeaderId(kj::StringPtr name) {
   for (uint i = 1; i <= MAX_COMMON_HEADER_ID; ++i) {
     KJ_DASSERT(COMMON_HEADER_NAMES[i] != nullptr);
     // If the lengths don't match or the first character doesn't match, skip full comparison
-    if (len != COMMON_HEADER_NAME_LENGTHS[i]) continue;
-    if (strncasecmp(name.begin(), COMMON_HEADER_NAMES[i], len) == 0) {
+    if (len != COMMON_HEADER_NAMES[i].size()) continue;
+    if (strncasecmp(name.begin(), COMMON_HEADER_NAMES[i].begin(), len) == 0) {
       return i;
     }
   }
@@ -240,40 +229,22 @@ inline void requireValidHeaderValue(kj::StringPtr value) {
 }
 }  // namespace
 
-Headers::UncommonHeaderKey::UncommonHeaderKey(kj::String name)
-    : name(kj::mv(name)),
-      hash(kj::hashCode(this->name)) {}
-
-Headers::UncommonHeaderKey::UncommonHeaderKey(kj::StringPtr name)
-    : name(kj::str(name)),
-      hash(kj::hashCode(this->name)) {}
-
-bool Headers::UncommonHeaderKey::operator==(const UncommonHeaderKey& other) const {
-  // The same hash code is a necessary but not sufficient condition for equality.
-  return hash == other.hash && name == other.name;
-}
-
-bool Headers::UncommonHeaderKey::operator==(kj::StringPtr otherName) const {
-  if (name.size() != otherName.size()) return false;
-  return strncasecmp(name.begin(), otherName.begin(), name.size()) == 0;
-}
-
 Headers::HeaderKey Headers::getHeaderKeyFor(kj::StringPtr name) {
   KJ_IF_SOME(commonId, getCommonHeaderId(name)) {
     return commonId;
   }
 
   // Not a common header, so allocate lowercase copy for uncommon header
-  return UncommonHeaderKey(toLower(name));
+  return toLower(name);
 }
 
 Headers::HeaderKey Headers::cloneHeaderKey(const HeaderKey& key) {
   KJ_SWITCH_ONEOF(key) {
     KJ_CASE_ONEOF(commonId, uint) {
       return commonId;
     }
-    KJ_CASE_ONEOF(uncommonKey, UncommonHeaderKey) {
-      return uncommonKey.clone();
+    KJ_CASE_ONEOF(uncommonKey, kj::String) {
+      return kj::str(uncommonKey);
     }
   }
   KJ_UNREACHABLE;
@@ -284,7 +255,7 @@ bool Headers::isSetCookie(const HeaderKey& key) {
     KJ_CASE_ONEOF(commonId, uint) {
       return commonId == static_cast<uint>(capnp::CommonHeaderName::SET_COOKIE);
     }
-    KJ_CASE_ONEOF(uncommonKey, UncommonHeaderKey) {
+    KJ_CASE_ONEOF(uncommonKey, kj::String) {
       // This case really shouldn't happen since "set-cookie" is a common header,
       // but just in case...
       return uncommonKey == "set-cookie";
@@ -294,21 +265,7 @@ bool Headers::isSetCookie(const HeaderKey& key) {
 }
 
 bool Headers::headerKeyEquals(const HeaderKey& a, const HeaderKey& b) {
-  KJ_SWITCH_ONEOF(a) {
-    KJ_CASE_ONEOF(aCommonId, uint) {
-      KJ_IF_SOME(bCommonId, b.tryGet<uint>()) {
-        return aCommonId == bCommonId;
-      }
-      return false;
-    }
-    KJ_CASE_ONEOF(aUncommonKey, UncommonHeaderKey) {
-      KJ_IF_SOME(bUncommonKey, b.tryGet<UncommonHeaderKey>()) {
-        return aUncommonKey == bUncommonKey;
-      }
-      return false;
-    }
-  }
-  KJ_UNREACHABLE;
+  return a == b;
 }
 
 Headers::Header::Header(jsg::ByteString name, kj::Vector<jsg::ByteString> values)
@@ -354,8 +311,8 @@ kj::StringPtr Headers::Header::Header::getKeyName() const {
     KJ_CASE_ONEOF(commonId, uint) {
       return COMMON_HEADER_NAMES[commonId];
     }
-    KJ_CASE_ONEOF(uncommonKey, UncommonHeaderKey) {
-      return uncommonKey.getName();
+    KJ_CASE_ONEOF(uncommonKey, kj::String) {
+      return uncommonKey;
     }
   }
   KJ_UNREACHABLE;
@@ -394,15 +351,15 @@ kj::uint Headers::HeaderCallbacks::hashCode(const HeaderKey& key) {
     KJ_CASE_ONEOF(commonId, uint) {
       return kj::hashCode(commonId);
     }
-    KJ_CASE_ONEOF(uncommonKey, UncommonHeaderKey) {
-      return uncommonKey.hashCode();
+    KJ_CASE_ONEOF(uncommonKey, kj::String) {
+      return kj::hashCode(uncommonKey);
     }
   }
   KJ_UNREACHABLE;
 }
 
 kj::uint Headers::HeaderCallbacks::hashCode(capnp::CommonHeaderName commondId) {
-  return kj::hashCode(static_cast<uint>(commondId));
+  return kj::hashCode(commondId);
 }
 
 Headers::Headers(jsg::Lock& js, jsg::Dict<jsg::ByteString, jsg::ByteString> dict)
@@ -776,7 +733,7 @@ void Headers::serialize(jsg::Lock& js, jsg::Serializer& serializer) {
         KJ_CASE_ONEOF(commonId, uint) {
           serializer.writeRawUint32(commonId);
         }
-        KJ_CASE_ONEOF(uncommonKey, UncommonHeaderKey) {
+        KJ_CASE_ONEOF(_, kj::String) {
           serializer.writeRawUint32(0);
           serializer.writeLengthDelimited(header.getHeaderName());
         }

src/workerd/api/headers.h

@@ -164,50 +164,10 @@ class Headers final: public jsg::Object {
   void visitForMemoryInfo(jsg::MemoryTracker& tracker) const;
 
 private:
-  // The header key can be stored either as a common header ID (uint) or an an
-  // UncommonHeaderKey. This class encapsulates the lower-cased header name and
-  // precomputed hash code for efficient comparisons. It is used for any header
-  // name that is not in the common header list.
-  class UncommonHeaderKey final {
-   public:
-    UncommonHeaderKey(UncommonHeaderKey&&) = default;
-    UncommonHeaderKey& operator=(UncommonHeaderKey&&) = default;
-    KJ_DISALLOW_COPY(UncommonHeaderKey);
-
-    inline bool operator==(const UncommonHeaderKey& other) const;
-    inline bool operator==(kj::StringPtr otherName) const;
-
-    inline uint hashCode() const { return hash; }
-    inline kj::StringPtr getName() const { return name; }
-
-    JSG_MEMORY_INFO(UncommonHeaderKey) {
-      tracker.trackField("name", name);
-    }
-
-    inline UncommonHeaderKey clone() const {
-      return UncommonHeaderKey(kj::str(name), hash);
-    }
-
-   private:
-    // The name is expected to be stored in lower-case form, but we do not
-    // enforce that actually within the struct. It is the responsibility of
-    // the caller to ensure it is appropriately lower-cased.
-    kj::String name;
-    kj::uint hash;
-
-    // Critically, because of hash collisions, we must still compare the full string
-    // to determine equality but we can use the hash code to avoid unnecessary string
-    // comparisons.
-    inline UncommonHeaderKey(kj::String name);
-    inline UncommonHeaderKey(kj::StringPtr name);
-    inline UncommonHeaderKey(kj::String name, kj::uint hash): name(kj::mv(name)), hash(hash) {}
-    friend class Headers;
-  };
-
   // A header is identified by either a common header ID or an uncommon header name.
   // The header key name is always identifed in lower-case form, while the original
   // casing is preserved in the actual Header struct to support case-preserving display.
-  using HeaderKey = kj::OneOf<uint, UncommonHeaderKey>;
+  using HeaderKey = kj::OneOf<uint, kj::String>;
 
   static HeaderKey getHeaderKeyFor(kj::StringPtr name);
   static bool headerKeyEquals(const HeaderKey& a, const HeaderKey& b);
@@ -249,7 +209,7 @@ class Headers final: public jsg::Object {
     Header clone() const;
 
     JSG_MEMORY_INFO(Header) {
-      tracker.trackField("key", key.tryGet<UncommonHeaderKey>());
+      tracker.trackField("key", key.tryGet<kj::String>());
       tracker.trackField("name", name);
       for (const auto& value : values) {
         tracker.trackField(nullptr, value);