diff --git a/src/core/lib/experiments/experiments.cc b/src/core/lib/experiments/experiments.cc index f3e670dcbdb..e745b668acf 100644 --- a/src/core/lib/experiments/experiments.cc +++ b/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, diff --git a/src/core/lib/experiments/experiments.h b/src/core/lib/experiments/experiments.h index 8d95faf922f..b36f340a7ba 100644 --- a/src/core/lib/experiments/experiments.h +++ b/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); diff --git a/src/core/lib/experiments/experiments.yaml b/src/core/lib/experiments/experiments.yaml index 59cdd8952a4..23220f4498f 100644 --- a/src/core/lib/experiments/experiments.yaml +++ b/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. diff --git a/src/core/lib/experiments/rollouts.yaml b/src/core/lib/experiments/rollouts.yaml index fb7020ebfe3..bdbb7ed35f3 100644 --- a/src/core/lib/experiments/rollouts.yaml +++ b/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 diff --git a/src/core/lib/transport/parsed_metadata.h b/src/core/lib/transport/parsed_metadata.h index 3510825c26d..6da6a862785 100644 --- a/src/core/lib/transport/parsed_metadata.h +++ b/src/core/lib/transport/parsed_metadata.h @@ -33,7 +33,6 @@ #include -#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::KeyValueVTable(absl::string_view key) { MetadataParseErrorFn, ParsedMetadata* result) { auto* p = new KV{ static_cast(result->value_.pointer)->first.Ref(), - will_keep_past_request_lifetime && IsUniqueMetadataStringsEnabled() - ? value->TakeUniquelyOwned() - : std::move(*value), + will_keep_past_request_lifetime ? value->TakeUniquelyOwned() + : std::move(*value), }; result->value_.pointer = p; }; diff --git a/src/core/lib/transport/simple_slice_based_metadata.h b/src/core/lib/transport/simple_slice_based_metadata.h index 0aaf980cf9b..41ede91e97e 100644 --- a/src/core/lib/transport/simple_slice_based_metadata.h +++ b/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();