[experiments] Remove unique_metadata_strings experiment (#34303)

Submit after: 2023/09/18

Remove this experiment since it's sufficiently rolled out.

src/core/lib/experiments/experiments.cc

@@ -83,11 +83,6 @@ const char* const description_canary_client_privacy =
 const char* const additional_constraints_canary_client_privacy = "{}";
 const char* const description_server_privacy = "If set, server privacy";
 const char* const additional_constraints_server_privacy = "{}";
-const char* const description_unique_metadata_strings =
-    "Ensure a unique copy of strings from parsed metadata are taken. The "
-    "hypothesis here is that ref counting these are causing read buffer "
-    "lifetimes to be extended leading to memory bloat.";
-const char* const additional_constraints_unique_metadata_strings = "{}";
 const char* const description_keepalive_fix =
     "Allows overriding keepalive_permit_without_calls. Refer "
     "https://github.com/grpc/grpc/pull/33428 for more information.";
@@ -169,8 +164,6 @@ const ExperimentMetadata g_experiment_metadata[] = {
      additional_constraints_canary_client_privacy, false, false},
     {"server_privacy", description_server_privacy,
      additional_constraints_server_privacy, false, false},
-    {"unique_metadata_strings", description_unique_metadata_strings,
-     additional_constraints_unique_metadata_strings, true, true},
     {"keepalive_fix", description_keepalive_fix,
      additional_constraints_keepalive_fix, false, false},
     {"keepalive_server_fix", description_keepalive_server_fix,
@@ -257,11 +250,6 @@ const char* const description_canary_client_privacy =
 const char* const additional_constraints_canary_client_privacy = "{}";
 const char* const description_server_privacy = "If set, server privacy";
 const char* const additional_constraints_server_privacy = "{}";
-const char* const description_unique_metadata_strings =
-    "Ensure a unique copy of strings from parsed metadata are taken. The "
-    "hypothesis here is that ref counting these are causing read buffer "
-    "lifetimes to be extended leading to memory bloat.";
-const char* const additional_constraints_unique_metadata_strings = "{}";
 const char* const description_keepalive_fix =
     "Allows overriding keepalive_permit_without_calls. Refer "
     "https://github.com/grpc/grpc/pull/33428 for more information.";
@@ -343,8 +331,6 @@ const ExperimentMetadata g_experiment_metadata[] = {
      additional_constraints_canary_client_privacy, false, false},
     {"server_privacy", description_server_privacy,
      additional_constraints_server_privacy, false, false},
-    {"unique_metadata_strings", description_unique_metadata_strings,
-     additional_constraints_unique_metadata_strings, true, true},
     {"keepalive_fix", description_keepalive_fix,
      additional_constraints_keepalive_fix, false, false},
     {"keepalive_server_fix", description_keepalive_server_fix,
@@ -431,11 +417,6 @@ const char* const description_canary_client_privacy =
 const char* const additional_constraints_canary_client_privacy = "{}";
 const char* const description_server_privacy = "If set, server privacy";
 const char* const additional_constraints_server_privacy = "{}";
-const char* const description_unique_metadata_strings =
-    "Ensure a unique copy of strings from parsed metadata are taken. The "
-    "hypothesis here is that ref counting these are causing read buffer "
-    "lifetimes to be extended leading to memory bloat.";
-const char* const additional_constraints_unique_metadata_strings = "{}";
 const char* const description_keepalive_fix =
     "Allows overriding keepalive_permit_without_calls. Refer "
     "https://github.com/grpc/grpc/pull/33428 for more information.";
@@ -517,8 +498,6 @@ const ExperimentMetadata g_experiment_metadata[] = {
      additional_constraints_canary_client_privacy, false, false},
     {"server_privacy", description_server_privacy,
      additional_constraints_server_privacy, false, false},
-    {"unique_metadata_strings", description_unique_metadata_strings,
-     additional_constraints_unique_metadata_strings, true, true},
     {"keepalive_fix", description_keepalive_fix,
      additional_constraints_keepalive_fix, false, false},
     {"keepalive_server_fix", description_keepalive_server_fix,

src/core/lib/experiments/experiments.h

@@ -77,8 +77,6 @@ inline bool IsWorkStealingEnabled() { return true; }
 inline bool IsClientPrivacyEnabled() { return false; }
 inline bool IsCanaryClientPrivacyEnabled() { return false; }
 inline bool IsServerPrivacyEnabled() { return false; }
-#define GRPC_EXPERIMENT_IS_INCLUDED_UNIQUE_METADATA_STRINGS
-inline bool IsUniqueMetadataStringsEnabled() { return true; }
 inline bool IsKeepaliveFixEnabled() { return false; }
 inline bool IsKeepaliveServerFixEnabled() { return false; }
 inline bool IsWorkSerializerDispatchEnabled() { return false; }
@@ -116,8 +114,6 @@ inline bool IsWorkStealingEnabled() { return true; }
 inline bool IsClientPrivacyEnabled() { return false; }
 inline bool IsCanaryClientPrivacyEnabled() { return false; }
 inline bool IsServerPrivacyEnabled() { return false; }
-#define GRPC_EXPERIMENT_IS_INCLUDED_UNIQUE_METADATA_STRINGS
-inline bool IsUniqueMetadataStringsEnabled() { return true; }
 inline bool IsKeepaliveFixEnabled() { return false; }
 inline bool IsKeepaliveServerFixEnabled() { return false; }
 inline bool IsWorkSerializerDispatchEnabled() { return false; }
@@ -155,8 +151,6 @@ inline bool IsWorkStealingEnabled() { return true; }
 inline bool IsClientPrivacyEnabled() { return false; }
 inline bool IsCanaryClientPrivacyEnabled() { return false; }
 inline bool IsServerPrivacyEnabled() { return false; }
-#define GRPC_EXPERIMENT_IS_INCLUDED_UNIQUE_METADATA_STRINGS
-inline bool IsUniqueMetadataStringsEnabled() { return true; }
 inline bool IsKeepaliveFixEnabled() { return false; }
 inline bool IsKeepaliveServerFixEnabled() { return false; }
 inline bool IsWorkSerializerDispatchEnabled() { return false; }
@@ -194,7 +188,6 @@ enum ExperimentIds {
   kExperimentIdClientPrivacy,
   kExperimentIdCanaryClientPrivacy,
   kExperimentIdServerPrivacy,
-  kExperimentIdUniqueMetadataStrings,
   kExperimentIdKeepaliveFix,
   kExperimentIdKeepaliveServerFix,
   kExperimentIdWorkSerializerDispatch,
@@ -277,10 +270,6 @@ inline bool IsCanaryClientPrivacyEnabled() {
 inline bool IsServerPrivacyEnabled() {
   return IsExperimentEnabled(kExperimentIdServerPrivacy);
 }
-#define GRPC_EXPERIMENT_IS_INCLUDED_UNIQUE_METADATA_STRINGS
-inline bool IsUniqueMetadataStringsEnabled() {
-  return IsExperimentEnabled(kExperimentIdUniqueMetadataStrings);
-}
 #define GRPC_EXPERIMENT_IS_INCLUDED_KEEPALIVE_FIX
 inline bool IsKeepaliveFixEnabled() {
   return IsExperimentEnabled(kExperimentIdKeepaliveFix);

src/core/lib/experiments/experiments.yaml

@@ -146,15 +146,6 @@
   owner: alishananda@google.com
   test_tags: []
   allow_in_fuzzing_config: false
-- name: unique_metadata_strings
-  description:
-    Ensure a unique copy of strings from parsed metadata are taken.
-    The hypothesis here is that ref counting these are causing read buffer
-    lifetimes to be extended leading to memory bloat.
-  expiry: 2023/11/01
-  owner: ctiller@google.com
-  test_tags: []
-  allow_in_fuzzing_config: true
 - name: keepalive_fix
   description:
     Allows overriding keepalive_permit_without_calls.

src/core/lib/experiments/rollouts.yaml

@@ -86,8 +86,6 @@
   default: false
 - name: server_privacy
   default: false
-- name: unique_metadata_strings
-  default: true
 - name: keepalive_fix
   default: false
 - name: keepalive_server_fix

src/core/lib/transport/parsed_metadata.h

@@ -33,7 +33,6 @@
 
 #include <grpc/slice.h>
 
-#include "src/core/lib/experiments/experiments.h"
 #include "src/core/lib/gprpp/time.h"
 #include "src/core/lib/slice/slice.h"
 
@@ -403,9 +402,8 @@ ParsedMetadata<MetadataContainer>::KeyValueVTable(absl::string_view key) {
          MetadataParseErrorFn, ParsedMetadata* result) {
         auto* p = new KV{
             static_cast<KV*>(result->value_.pointer)->first.Ref(),
-            will_keep_past_request_lifetime && IsUniqueMetadataStringsEnabled()
+            will_keep_past_request_lifetime ? value->TakeUniquelyOwned()
-                ? value->TakeUniquelyOwned()
+                                            : std::move(*value),
-                : std::move(*value),
         };
         result->value_.pointer = p;
       };

src/core/lib/transport/simple_slice_based_metadata.h

@@ -19,7 +19,6 @@
 
 #include "absl/strings/string_view.h"
 
-#include "src/core/lib/experiments/experiments.h"
 #include "src/core/lib/slice/slice.h"
 #include "src/core/lib/transport/parsed_metadata.h"
 
@@ -34,7 +33,7 @@ struct SimpleSliceBasedMetadata {
   static MementoType ParseMemento(Slice value,
                                   bool will_keep_past_request_lifetime,
                                   MetadataParseErrorFn) {
-    if (will_keep_past_request_lifetime && IsUniqueMetadataStringsEnabled()) {
+    if (will_keep_past_request_lifetime) {
       return value.TakeUniquelyOwned();
     } else {
       return value.TakeOwned();