Remove use_fast_protobuf_hash runtime flag (#36645)

Commit Message: Remove use_fast_protobuf_hash runtime flag
Additional Description: Fixes #36613 
Risk Level: Should be a no-op for anyone who didn't override the flag.
Testing: Same as before but without the branches that no longer exist.
Docs Changes: n/a
Release Notes: n/a
Platform Specific Features: n/a

---------

Signed-off-by: Raven Black <ravenblack@dropbox.com>

source/common/protobuf/utility.cc

@@ -214,18 +214,7 @@ void ProtoExceptionUtil::throwProtoValidationException(const std::string& valida
 
 size_t MessageUtil::hash(const Protobuf::Message& message) {
 #if defined(ENVOY_ENABLE_FULL_PROTOS)
-  if (Runtime::runtimeFeatureEnabled("envoy.restart_features.use_fast_protobuf_hash")) {
-    return DeterministicProtoHash::hash(message);
-  } else {
-    std::string text_format;
-    Protobuf::TextFormat::Printer printer;
-    printer.SetExpandAny(true);
-    printer.SetUseFieldNumber(true);
-    printer.SetSingleLineMode(true);
-    printer.SetHideUnknownFields(true);
-    printer.PrintToString(message, &text_format);
-    return HashUtil::xxHash64(text_format);
-  }
+  return DeterministicProtoHash::hash(message);
 #else
   return HashUtil::xxHash64(message.SerializeAsString());
 #endif

source/common/runtime/runtime_features.cc

@@ -114,7 +114,6 @@ RUNTIME_GUARD(envoy_restart_features_allow_slot_destroy_on_worker_threads);
 RUNTIME_GUARD(envoy_restart_features_fix_dispatcher_approximate_now);
 RUNTIME_GUARD(envoy_restart_features_quic_handle_certs_with_shared_tls_code);
 RUNTIME_GUARD(envoy_restart_features_use_eds_cache_for_ads);
-RUNTIME_GUARD(envoy_restart_features_use_fast_protobuf_hash);
 
 // Begin false flags. Most of them should come with a TODO to flip true.
 

source/extensions/filters/http/cache/http_cache.cc

@@ -57,13 +57,7 @@ LookupRequest::LookupRequest(const Http::RequestHeaderMap& request_headers, Syst
 // Unless this API is still alpha, calls to stableHashKey() must always return
 // the same result, or a way must be provided to deal with a complete cache
 // flush.
-size_t stableHashKey(const Key& key) {
-  if (Runtime::runtimeFeatureEnabled("envoy.restart_features.use_fast_protobuf_hash")) {
-    return DeterministicProtoHash::hash(key);
-  } else {
-    return MessageUtil::hash(key);
-  }
-}
+size_t stableHashKey(const Key& key) { return DeterministicProtoHash::hash(key); }
 
 void LookupRequest::initializeRequestCacheControl(const Http::RequestHeaderMap& request_headers) {
   const absl::string_view cache_control =

test/common/protobuf/utility_speed_test.cc

@@ -4,7 +4,6 @@
 #include "source/common/protobuf/utility.h"
 
 #include "test/common/protobuf/deterministic_hash_test.pb.h"
-#include "test/test_common/test_runtime.h"
 
 #include "benchmark/benchmark.h"
 
@@ -53,21 +52,8 @@ static std::unique_ptr<Protobuf::Message> testProtoWithRepeatedFields() {
   return msg;
 }
 
-static void bmHashByTextFormat(benchmark::State& state, std::unique_ptr<Protobuf::Message> msg) {
-  TestScopedRuntime runtime;
-  runtime.mergeValues({{"envoy.restart_features.use_fast_protobuf_hash", "false"}});
-  uint64_t hash = 0;
-  for (auto _ : state) {
-    UNREFERENCED_PARAMETER(_);
-    hash += MessageUtil::hash(*msg);
-  }
-  benchmark::DoNotOptimize(hash);
-}
-
 static void bmHashByDeterministicHash(benchmark::State& state,
                                       std::unique_ptr<Protobuf::Message> msg) {
-  TestScopedRuntime runtime;
-  runtime.mergeValues({{"envoy.restart_features.use_fast_protobuf_hash", "true"}});
   uint64_t hash = 0;
   for (auto _ : state) {
     UNREFERENCED_PARAMETER(_);
@@ -76,10 +62,7 @@ static void bmHashByDeterministicHash(benchmark::State& state,
   benchmark::DoNotOptimize(hash);
 }
 BENCHMARK_CAPTURE(bmHashByDeterministicHash, map, testProtoWithMaps());
-BENCHMARK_CAPTURE(bmHashByTextFormat, map, testProtoWithMaps());
 BENCHMARK_CAPTURE(bmHashByDeterministicHash, recursion, testProtoWithRecursion());
-BENCHMARK_CAPTURE(bmHashByTextFormat, recursion, testProtoWithRecursion());
 BENCHMARK_CAPTURE(bmHashByDeterministicHash, repeatedFields, testProtoWithRepeatedFields());
-BENCHMARK_CAPTURE(bmHashByTextFormat, repeatedFields, testProtoWithRepeatedFields());
 
 } // namespace Envoy

test/common/protobuf/utility_test.cc

@@ -194,22 +194,16 @@ TEST_F(ProtobufUtilityTest, MessageUtilHash) {
   a4.PackFrom(s2);
   a5.PackFrom(s3);
 
-  TestScopedRuntime runtime_;
-  for (std::string runtime_value : {"true", "false"}) {
-    // TODO(ravenblack): when the runtime flag is removed, keep the expects
-    // but remove the loop around them and the extra output.
-    runtime_.mergeValues({{"envoy.restart_features.use_fast_protobuf_hash", runtime_value}});
-    EXPECT_EQ(MessageUtil::hash(a1), MessageUtil::hash(a2)) << runtime_value;
-    EXPECT_EQ(MessageUtil::hash(a2), MessageUtil::hash(a3)) << runtime_value;
-    EXPECT_NE(0, MessageUtil::hash(a1)) << runtime_value;
-    // Same keys and values but with the values in a different order should not have
-    // the same hash.
-    EXPECT_NE(MessageUtil::hash(a1), MessageUtil::hash(a4)) << runtime_value;
-    // Different keys with the values in the same order should not have the same hash.
-    EXPECT_NE(MessageUtil::hash(a1), MessageUtil::hash(a5)) << runtime_value;
-    // Struct without 'any' around it should not hash the same as struct inside 'any'.
-    EXPECT_NE(MessageUtil::hash(s), MessageUtil::hash(a1)) << runtime_value;
-  }
+  EXPECT_EQ(MessageUtil::hash(a1), MessageUtil::hash(a2));
+  EXPECT_EQ(MessageUtil::hash(a2), MessageUtil::hash(a3));
+  EXPECT_NE(0, MessageUtil::hash(a1));
+  // Same keys and values but with the values in a different order should not have
+  // the same hash.
+  EXPECT_NE(MessageUtil::hash(a1), MessageUtil::hash(a4));
+  // Different keys with the values in the same order should not have the same hash.
+  EXPECT_NE(MessageUtil::hash(a1), MessageUtil::hash(a5));
+  // Struct without 'any' around it should not hash the same as struct inside 'any'.
+  EXPECT_NE(MessageUtil::hash(s), MessageUtil::hash(a1));
 }
 
 TEST_F(ProtobufUtilityTest, RepeatedPtrUtilDebugString) {

test/extensions/filters/http/cache/http_cache_test.cc

@@ -7,7 +7,6 @@
 #include "test/mocks/http/mocks.h"
 #include "test/mocks/server/server_factory_context.h"
 #include "test/test_common/simulated_time_system.h"
-#include "test/test_common/test_runtime.h"
 #include "test/test_common/utility.h"
 
 #include "gtest/gtest.h"
@@ -336,22 +335,11 @@ TEST_F(LookupRequestTest, PragmaNoFallback) {
 }
 
 TEST(HttpCacheTest, StableHashKey) {
-  TestScopedRuntime runtime;
-  runtime.mergeValues({{"envoy.restart_features.use_fast_protobuf_hash", "true"}});
   Key key;
   key.set_host("example.com");
   ASSERT_EQ(stableHashKey(key), 6153940628716543519u);
 }
 
-TEST(HttpCacheTest, StableHashKeyWithSlowHash) {
-  // TODO(ravenblack): This test should be removed when the runtime guard is removed.
-  TestScopedRuntime runtime;
-  runtime.mergeValues({{"envoy.restart_features.use_fast_protobuf_hash", "false"}});
-  Key key;
-  key.set_host("example.com");
-  ASSERT_EQ(stableHashKey(key), 9582653837550152292u);
-}
-
 TEST_P(LookupRequestTest, ResultWithBodyAndTrailersMatchesExpectation) {
   request_headers_.setReferenceKey(Http::CustomHeaders::get().CacheControl,
                                    GetParam().request_cache_control);