Revert "[chttp2] Fix bug in timeout encoding" (grpc#34811)

Reverts grpc#34751

fuzztest/core/transport/chttp2/BUILD

@@ -24,17 +24,3 @@ grpc_fuzz_test(
     ],
     deps = ["//src/core:write_size_policy"],
 )
-
-grpc_fuzz_test(
-    name = "hpack_encoder_timeout_test",
-    srcs = ["hpack_encoder_timeout_test.cc"],
-    external_deps = [
-        "fuzztest",
-        "fuzztest_main",
-        "gtest",
-    ],
-    deps = [
-        "//:hpack_encoder",
-        "//:hpack_parser",
-    ],
-)

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

@@ -475,25 +475,31 @@ void Encoder::EncodeRepeatingSliceValue(const absl::string_view& key,
 
 void TimeoutCompressorImpl::EncodeWith(absl::string_view key,
                                        Timestamp deadline, Encoder* encoder) {
-  const Timeout timeout = Timeout::FromDuration(deadline - Timestamp::Now());
+  Timeout timeout = Timeout::FromDuration(deadline - Timestamp::Now());
   auto& table = encoder->hpack_table();
-  for (size_t i = 0; i < kNumPreviousValues; i++) {
-    const auto& previous = previous_timeouts_[i];
-    if (!table.ConvertableToDynamicIndex(previous.index)) continue;
-    const double ratio = timeout.RatioVersus(previous.timeout);
+  for (auto it = previous_timeouts_.begin(); it != previous_timeouts_.end();
+       ++it) {
+    double ratio = timeout.RatioVersus(it->timeout);
     // If the timeout we're sending is shorter than a previous timeout, but
     // within 3% of it, we'll consider sending it.
-    if (ratio > -3 && ratio <= 0) {
-      encoder->EmitIndexed(table.DynamicIndex(previous.index));
+    if (ratio > -3 && ratio <= 0 &&
+        table.ConvertableToDynamicIndex(it->index)) {
+      encoder->EmitIndexed(table.DynamicIndex(it->index));
+      // Put this timeout to the front of the queue - forces common timeouts to
+      // be considered earlier.
+      std::swap(*it, *previous_timeouts_.begin());
       return;
     }
   }
+  // Clean out some expired timeouts.
+  while (!previous_timeouts_.empty() &&
+         !table.ConvertableToDynamicIndex(previous_timeouts_.back().index)) {
+    previous_timeouts_.pop_back();
+  }
   Slice encoded = timeout.Encode();
   uint32_t index = encoder->EmitLitHdrWithNonBinaryStringKeyIncIdx(
       Slice::FromStaticString(key), std::move(encoded));
-  uint32_t i = next_previous_value_;
-  ++next_previous_value_;
-  previous_timeouts_[i % kNumPreviousValues] = PreviousTimeout{timeout, index};
+  previous_timeouts_.push_back(PreviousTimeout{timeout, index});
 }
 
 Encoder::Encoder(HPackCompressor* compressor, bool use_true_binary_metadata,

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

@@ -276,20 +276,16 @@ class Compressor<MetadataTrait, SmallSetOfValuesCompressor> {
 };
 
 struct PreviousTimeout {
-  Timeout timeout = Timeout::FromDuration(Duration::Zero());
-  // Dynamic table index of a previously sent timeout
-  // 0 is guaranteed not in the dynamic table so is a safe initializer
-  uint32_t index = 0;
+  Timeout timeout;
+  uint32_t index;
 };
 
 class TimeoutCompressorImpl {
  public:
   void EncodeWith(absl::string_view key, Timestamp deadline, Encoder* encoder);
 
  private:
-  static constexpr const size_t kNumPreviousValues = 5;
-  PreviousTimeout previous_timeouts_[kNumPreviousValues];
-  uint32_t next_previous_value_ = 0;
+  std::vector<PreviousTimeout> previous_timeouts_;
 };
 
 template <typename MetadataTrait>

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

@@ -59,7 +59,6 @@ class HPackEncoderTable {
            table_elems_ - index;
   }
   // Check if an element index is convertable to a dynamic index
-  // Note that 0 is always not convertable
   bool ConvertableToDynamicIndex(uint32_t index) const {
     return index > tail_remote_index_;
   }

src/core/lib/transport/timeout_encoding.h

@@ -61,8 +61,8 @@ class Timeout {
   static Timeout FromMinutes(int64_t minutes);
   static Timeout FromHours(int64_t hours);
 
-  uint16_t value_ = 0;
-  Unit unit_ = Unit::kNanoseconds;
+  uint16_t value_;
+  Unit unit_;
 };
 
 absl::optional<Duration> ParseTimeout(const Slice& text);