[hpack] Reduce parse table size in the rare case of a parse error (#33637)

Most of the time parsing succeeds, and only rarely do we see an error.

This change reduces the parse memento size from 120 bytes to 56 bytes.

---------

Co-authored-by: ctiller <ctiller@users.noreply.github.com>

src/core/ext/transport/chttp2/transport/hpack_parse_result.h

@@ -19,6 +19,7 @@
 
 #include <stdint.h>
 
+#include <memory>
 #include <string>
 #include <utility>
 
@@ -132,9 +133,9 @@ class HpackParseResult {
   bool connection_error() const { return IsConnectionError(status_.get()); }
   bool ephemeral() const { return IsEphemeralError(status_.get()); }
 
-  HpackParseResult PersistentStreamErrorOrOk() const {
-    if (connection_error() || ephemeral()) return HpackParseResult();
-    return *this;
+  std::unique_ptr<HpackParseResult> PersistentStreamErrorOrNullptr() const {
+    if (ok() || connection_error() || ephemeral()) return nullptr;
+    return std::make_unique<HpackParseResult>(*this);
   }
 
   static HpackParseResult FromStatus(HpackParseStatus status) {

src/core/ext/transport/chttp2/transport/hpack_parser.cc

@@ -24,6 +24,7 @@
 #include <stdlib.h>
 
 #include <algorithm>
+#include <memory>
 #include <string>
 #include <utility>
 
@@ -696,19 +697,19 @@ class HPackParser::Parser {
     gpr_log(
         GPR_DEBUG, "HTTP:%d:%s:%s: %s%s", log_info_.stream_id, type,
         log_info_.is_client ? "CLI" : "SVR", memento.md.DebugString().c_str(),
-        memento.parse_status.ok()
+        memento.parse_status == nullptr
             ? ""
             : absl::StrCat(" (parse error: ",
-                           memento.parse_status.Materialize().ToString(), ")")
+                           memento.parse_status->Materialize().ToString(), ")")
                   .c_str());
   }
 
   void EmitHeader(const HPackTable::Memento& md) {
     // Pass up to the transport
     state_.frame_length += md.md.transport_size();
-    if (!md.parse_status.ok()) {
+    if (md.parse_status != nullptr) {
       // Reject any requests with invalid metadata.
-      input_->SetErrorAndContinueParsing(md.parse_status);
+      input_->SetErrorAndContinueParsing(*md.parse_status);
     }
     if (GPR_LIKELY(metadata_buffer_ != nullptr)) {
       metadata_buffer_->Set(md.md);
@@ -957,8 +958,8 @@ class HPackParser::Parser {
     } else {
       const auto* memento = absl::get<const HPackTable::Memento*>(state_.key);
       key_string = memento->md.key();
-      if (status.ok() && !memento->parse_status.ok()) {
-        input_->SetErrorAndContinueParsing(memento->parse_status);
+      if (status.ok() && memento->parse_status != nullptr) {
+        input_->SetErrorAndContinueParsing(*memento->parse_status);
       }
     }
     switch (value.status) {
@@ -993,7 +994,7 @@ class HPackParser::Parser {
                   std::string(message).c_str());
         });
     HPackTable::Memento memento{std::move(md),
-                                status.PersistentStreamErrorOrOk()};
+                                status.PersistentStreamErrorOrNullptr()};
     input_->UpdateFrontier();
     state_.parse_state = ParseState::kTop;
     if (state_.add_to_table) {

src/core/ext/transport/chttp2/transport/hpack_parser_table.cc

@@ -161,10 +161,10 @@ void HPackTable::AddLargerThanCurrentTableSize() {
 std::string HPackTable::TestOnlyDynamicTableAsString() const {
   std::string out;
   entries_.ForEach([&out](uint32_t i, const Memento& m) {
-    if (m.parse_status.ok()) {
+    if (m.parse_status == nullptr) {
       absl::StrAppend(&out, i, ": ", m.md.DebugString(), "\n");
     } else {
-      absl::StrAppend(&out, i, ": ", m.parse_status.Materialize().ToString(),
+      absl::StrAppend(&out, i, ": ", m.parse_status->Materialize().ToString(),
                       "\n");
     }
   });
@@ -250,7 +250,7 @@ HPackTable::Memento MakeMemento(size_t i) {
           [](absl::string_view, const Slice&) {
             abort();  // not expecting to see this
           }),
-      HpackParseResult()};
+      nullptr};
 }
 
 }  // namespace

src/core/ext/transport/chttp2/transport/hpack_parser_table.h

@@ -23,6 +23,7 @@
 
 #include <stdint.h>
 
+#include <memory>
 #include <string>
 #include <vector>
 
@@ -51,7 +52,7 @@ class HPackTable {
 
   struct Memento {
     ParsedMetadata<grpc_metadata_batch> md;
-    HpackParseResult parse_status;
+    std::unique_ptr<HpackParseResult> parse_status;
   };
 
   // Lookup, but don't ref.

test/core/transport/chttp2/hpack_parser_table_test.cc

@@ -123,7 +123,7 @@ TEST(HpackParserTableTest, ManyAdditions) {
             ParsedMetadata<grpc_metadata_batch>::FromSlicePair{},
             std::move(key_slice), std::move(value_slice),
             key.length() + value.length() + 32),
-        HpackParseResult()};
+        nullptr};
     ASSERT_TRUE(tbl.Add(std::move(memento)));
     AssertIndex(&tbl, 1 + hpack_constants::kLastStaticEntry, key.c_str(),
                 value.c_str());