From ade0c98e87274fb6eb0b6bdc7a94496dbf93c696 Mon Sep 17 00:00:00 2001 From: Yash Tibrewal Date: Thu, 16 Jan 2020 17:45:26 -0800 Subject: [PATCH] Fix grpc_core::Optional --- .../client_channel/lb_policy/xds/xds.cc | 2 +- .../client_channel/resolver_result_parsing.cc | 12 ++-- .../ext/filters/client_channel/xds/xds_api.cc | 2 +- .../filters/client_channel/xds/xds_client.cc | 2 +- src/core/lib/gprpp/optional.h | 10 +-- src/core/lib/iomgr/buffer_list.cc | 71 ++++++++++--------- test/core/gprpp/optional_test.cc | 2 +- 7 files changed, 49 insertions(+), 52 deletions(-) diff --git a/src/core/ext/filters/client_channel/lb_policy/xds/xds.cc b/src/core/ext/filters/client_channel/lb_policy/xds/xds.cc index 59f59fad4c2..28c5f745009 100644 --- a/src/core/ext/filters/client_channel/lb_policy/xds/xds.cc +++ b/src/core/ext/filters/client_channel/lb_policy/xds/xds.cc @@ -1886,7 +1886,7 @@ class XdsFactory : public LoadBalancingPolicyFactory { if (error_list.empty()) { Optional optional_lrs_load_reporting_server_name; if (lrs_load_reporting_server_name != nullptr) { - optional_lrs_load_reporting_server_name.set( + optional_lrs_load_reporting_server_name.emplace( std::string(lrs_load_reporting_server_name)); } return MakeRefCounted( diff --git a/src/core/ext/filters/client_channel/resolver_result_parsing.cc b/src/core/ext/filters/client_channel/resolver_result_parsing.cc index 55742afe396..df201fd310a 100644 --- a/src/core/ext/filters/client_channel/resolver_result_parsing.cc +++ b/src/core/ext/filters/client_channel/resolver_result_parsing.cc @@ -348,8 +348,8 @@ ClientChannelServiceConfigParser::ParseGlobalParams(const grpc_json* json, "field:retryThrottling field:maxTokens error:Type should be " "number")); } else { - max_milli_tokens.set(gpr_parse_nonnegative_int(sub_field->value) * - 1000); + max_milli_tokens.emplace( + gpr_parse_nonnegative_int(sub_field->value) * 1000); if (max_milli_tokens.value() <= 0) { error_list.push_back(GRPC_ERROR_CREATE_FROM_STATIC_STRING( "field:retryThrottling field:maxTokens error:should be " @@ -398,7 +398,7 @@ ClientChannelServiceConfigParser::ParseGlobalParams(const grpc_json* json, "parsing")); continue; } - milli_token_ratio.set( + milli_token_ratio.emplace( static_cast((whole_value * multiplier) + decimal_value)); if (milli_token_ratio.value() <= 0) { error_list.push_back(GRPC_ERROR_CREATE_FROM_STATIC_STRING( @@ -421,7 +421,7 @@ ClientChannelServiceConfigParser::ParseGlobalParams(const grpc_json* json, } else { data.milli_token_ratio = milli_token_ratio.value(); } - retry_throttling.set(data); + retry_throttling.emplace(data); } if (strcmp(field->key, "healthCheckConfig") == 0) { if (health_check_service_name != nullptr) { @@ -461,9 +461,9 @@ ClientChannelServiceConfigParser::ParsePerMethodParams(const grpc_json* json, "field:waitForReady error:Duplicate entry")); } // Duplicate, continue parsing. if (field->type == GRPC_JSON_TRUE) { - wait_for_ready.set(true); + wait_for_ready.emplace(true); } else if (field->type == GRPC_JSON_FALSE) { - wait_for_ready.set(false); + wait_for_ready.emplace(false); } else { error_list.push_back(GRPC_ERROR_CREATE_FROM_STATIC_STRING( "field:waitForReady error:Type should be true/false")); diff --git a/src/core/ext/filters/client_channel/xds/xds_api.cc b/src/core/ext/filters/client_channel/xds/xds_api.cc index 43e9e6957dc..f2cce90ae27 100644 --- a/src/core/ext/filters/client_channel/xds/xds_api.cc +++ b/src/core/ext/filters/client_channel/xds/xds_api.cc @@ -397,7 +397,7 @@ grpc_error* CdsResponseParse(const envoy_api_v2_DiscoveryResponse* response, return GRPC_ERROR_CREATE_FROM_STATIC_STRING( "ConfigSource is not self."); } - cds_update.lrs_load_reporting_server_name.set(""); + cds_update.lrs_load_reporting_server_name.emplace(""); } upb_strview cluster_name = envoy_api_v2_Cluster_name(cluster); cds_update_map->emplace(std::string(cluster_name.data, cluster_name.size), diff --git a/src/core/ext/filters/client_channel/xds/xds_client.cc b/src/core/ext/filters/client_channel/xds/xds_client.cc index ccdad9ee93d..c9f07690814 100644 --- a/src/core/ext/filters/client_channel/xds/xds_client.cc +++ b/src/core/ext/filters/client_channel/xds/xds_client.cc @@ -762,7 +762,7 @@ void XdsClient::ChannelState::AdsCallState::AcceptCdsUpdate( continue; } // Update the cluster state. - cluster_state.update.set(std::move(cds_update)); + cluster_state.update.emplace(std::move(cds_update)); // Notify all watchers. for (const auto& p : cluster_state.watchers) { p.first->OnClusterChanged(cluster_state.update.value()); diff --git a/src/core/lib/gprpp/optional.h b/src/core/lib/gprpp/optional.h index a1b54666c6a..e6228c1498a 100644 --- a/src/core/lib/gprpp/optional.h +++ b/src/core/lib/gprpp/optional.h @@ -46,13 +46,9 @@ class Optional { public: Optional() : value_() {} - void set(const T& val) { - value_ = val; - set_ = true; - } - - void set(T&& val) { - value_ = std::move(val); + template + void emplace(Args&&... args) { + value_ = T(std::forward(args)...); set_ = true; } diff --git a/src/core/lib/iomgr/buffer_list.cc b/src/core/lib/iomgr/buffer_list.cc index 9123cd3aa6d..e1b87da4d0c 100644 --- a/src/core/lib/iomgr/buffer_list.cc +++ b/src/core/lib/iomgr/buffer_list.cc @@ -66,27 +66,27 @@ void extract_opt_stats_from_tcp_info(ConnectionMetrics* metrics, return; } if (info->length > offsetof(grpc_core::tcp_info, tcpi_sndbuf_limited)) { - metrics->recurring_retrans.set(info->tcpi_retransmits); - metrics->is_delivery_rate_app_limited.set( + metrics->recurring_retrans.emplace(info->tcpi_retransmits); + metrics->is_delivery_rate_app_limited.emplace( info->tcpi_delivery_rate_app_limited); - metrics->congestion_window.set(info->tcpi_snd_cwnd); - metrics->reordering.set(info->tcpi_reordering); - metrics->packet_retx.set(info->tcpi_total_retrans); - metrics->pacing_rate.set(info->tcpi_pacing_rate); - metrics->data_notsent.set(info->tcpi_notsent_bytes); + metrics->congestion_window.emplace(info->tcpi_snd_cwnd); + metrics->reordering.emplace(info->tcpi_reordering); + metrics->packet_retx.emplace(info->tcpi_total_retrans); + metrics->pacing_rate.emplace(info->tcpi_pacing_rate); + metrics->data_notsent.emplace(info->tcpi_notsent_bytes); if (info->tcpi_min_rtt != UINT32_MAX) { - metrics->min_rtt.set(info->tcpi_min_rtt); + metrics->min_rtt.emplace(info->tcpi_min_rtt); } - metrics->packet_sent.set(info->tcpi_data_segs_out); - metrics->delivery_rate.set(info->tcpi_delivery_rate); - metrics->busy_usec.set(info->tcpi_busy_time); - metrics->rwnd_limited_usec.set(info->tcpi_rwnd_limited); - metrics->sndbuf_limited_usec.set(info->tcpi_sndbuf_limited); + metrics->packet_sent.emplace(info->tcpi_data_segs_out); + metrics->delivery_rate.emplace(info->tcpi_delivery_rate); + metrics->busy_usec.emplace(info->tcpi_busy_time); + metrics->rwnd_limited_usec.emplace(info->tcpi_rwnd_limited); + metrics->sndbuf_limited_usec.emplace(info->tcpi_sndbuf_limited); } if (info->length > offsetof(grpc_core::tcp_info, tcpi_dsack_dups)) { - metrics->data_sent.set(info->tcpi_bytes_sent); - metrics->data_retx.set(info->tcpi_bytes_retrans); - metrics->packet_spurious_retx.set(info->tcpi_dsack_dups); + metrics->data_sent.emplace(info->tcpi_bytes_sent); + metrics->data_retx.emplace(info->tcpi_bytes_retrans); + metrics->packet_spurious_retx.emplace(info->tcpi_dsack_dups); } } @@ -107,79 +107,80 @@ void extract_opt_stats_from_cmsg(ConnectionMetrics* metrics, const void* val = data + offset + NLA_HDRLEN; switch (attr->nla_type) { case TCP_NLA_BUSY: { - metrics->busy_usec.set(read_unaligned(val)); + metrics->busy_usec.emplace(read_unaligned(val)); break; } case TCP_NLA_RWND_LIMITED: { - metrics->rwnd_limited_usec.set(read_unaligned(val)); + metrics->rwnd_limited_usec.emplace(read_unaligned(val)); break; } case TCP_NLA_SNDBUF_LIMITED: { - metrics->sndbuf_limited_usec.set(read_unaligned(val)); + metrics->sndbuf_limited_usec.emplace(read_unaligned(val)); break; } case TCP_NLA_PACING_RATE: { - metrics->pacing_rate.set(read_unaligned(val)); + metrics->pacing_rate.emplace(read_unaligned(val)); break; } case TCP_NLA_DELIVERY_RATE: { - metrics->delivery_rate.set(read_unaligned(val)); + metrics->delivery_rate.emplace(read_unaligned(val)); break; } case TCP_NLA_DELIVERY_RATE_APP_LMT: { - metrics->is_delivery_rate_app_limited.set(read_unaligned(val)); + metrics->is_delivery_rate_app_limited.emplace( + read_unaligned(val)); break; } case TCP_NLA_SND_CWND: { - metrics->congestion_window.set(read_unaligned(val)); + metrics->congestion_window.emplace(read_unaligned(val)); break; } case TCP_NLA_MIN_RTT: { - metrics->min_rtt.set(read_unaligned(val)); + metrics->min_rtt.emplace(read_unaligned(val)); break; } case TCP_NLA_SRTT: { - metrics->srtt.set(read_unaligned(val)); + metrics->srtt.emplace(read_unaligned(val)); break; } case TCP_NLA_RECUR_RETRANS: { - metrics->recurring_retrans.set(read_unaligned(val)); + metrics->recurring_retrans.emplace(read_unaligned(val)); break; } case TCP_NLA_BYTES_SENT: { - metrics->data_sent.set(read_unaligned(val)); + metrics->data_sent.emplace(read_unaligned(val)); break; } case TCP_NLA_DATA_SEGS_OUT: { - metrics->packet_sent.set(read_unaligned(val)); + metrics->packet_sent.emplace(read_unaligned(val)); break; } case TCP_NLA_TOTAL_RETRANS: { - metrics->packet_retx.set(read_unaligned(val)); + metrics->packet_retx.emplace(read_unaligned(val)); break; } case TCP_NLA_DELIVERED: { - metrics->packet_delivered.set(read_unaligned(val)); + metrics->packet_delivered.emplace(read_unaligned(val)); break; } case TCP_NLA_DELIVERED_CE: { - metrics->packet_delivered_ce.set(read_unaligned(val)); + metrics->packet_delivered_ce.emplace(read_unaligned(val)); break; } case TCP_NLA_BYTES_RETRANS: { - metrics->data_retx.set(read_unaligned(val)); + metrics->data_retx.emplace(read_unaligned(val)); break; } case TCP_NLA_DSACK_DUPS: { - metrics->packet_spurious_retx.set(read_unaligned(val)); + metrics->packet_spurious_retx.emplace(read_unaligned(val)); break; } case TCP_NLA_REORDERING: { - metrics->reordering.set(read_unaligned(val)); + metrics->reordering.emplace(read_unaligned(val)); break; } case TCP_NLA_SND_SSTHRESH: { - metrics->snd_ssthresh.set(read_unaligned(val)); + metrics->snd_ssthresh.emplace(read_unaligned(val)); break; } } diff --git a/test/core/gprpp/optional_test.cc b/test/core/gprpp/optional_test.cc index ce6f8692fd5..67c7fad6c32 100644 --- a/test/core/gprpp/optional_test.cc +++ b/test/core/gprpp/optional_test.cc @@ -31,7 +31,7 @@ TEST(OptionalTest, BasicTest) { EXPECT_FALSE(opt_val.has_value()); const int kTestVal = 123; - opt_val.set(kTestVal); + opt_val.emplace(kTestVal); EXPECT_TRUE(opt_val.has_value()); EXPECT_EQ(opt_val.value(), kTestVal);