minor perf: update filter state method to reduce repeated hash search…

…ing (envoyproxy#19609)

Signed-off-by: wbpcode <[email protected]>

envoy/stream_info/filter_state.h

@@ -87,52 +87,42 @@ class FilterState {
 
   /**
    * @param data_name the name of the data being looked up (mutable/readonly).
-   * @return a const reference to the stored data.
-   * An exception will be thrown if the data does not exist. This function
-   * will fail if the data stored under |data_name| cannot be dynamically
-   * cast to the type specified.
+   * @return a typed pointer to the stored data or nullptr if the data does not exist or the data
+   * type does not match the expected type.
    */
-  template <typename T> const T& getDataReadOnly(absl::string_view data_name) const {
-    const T* result = dynamic_cast<const T*>(getDataReadOnlyGeneric(data_name));
-    if (!result) {
-      ExceptionUtil::throwEnvoyException(
-          fmt::format("Data stored under {} cannot be coerced to specified type", data_name));
-    }
-    return *result;
+  template <typename T> const T* getDataReadOnly(absl::string_view data_name) const {
+    return dynamic_cast<const T*>(getDataReadOnlyGeneric(data_name));
   }
 
   /**
    * @param data_name the name of the data being looked up (mutable/readonly).
-   * @return a const reference to the stored data.
-   * An exception will be thrown if the data does not exist.
+   * @return a const pointer to the stored data or nullptr if the data does not exist.
    */
   virtual const Object* getDataReadOnlyGeneric(absl::string_view data_name) const PURE;
 
   /**
-   * @param data_name the name of the data being looked up (mutable only).
-   * @return a non-const reference to the stored data if and only if the
-   * underlying data is mutable.
-   * An exception will be thrown if the data does not exist or if it is
-   * immutable. This function will fail if the data stored under
-   * |data_name| cannot be dynamically cast to the type specified.
+   * @param data_name the name of the data being looked up (mutable/readonly).
+   * @return a typed pointer to the stored data or nullptr if the data does not exist or the data
+   * type does not match the expected type.
    */
-  template <typename T> T& getDataMutable(absl::string_view data_name) {
-    T* result = dynamic_cast<T*>(getDataMutableGeneric(data_name));
-    if (!result) {
-      ExceptionUtil::throwEnvoyException(
-          fmt::format("Data stored under {} cannot be coerced to specified type", data_name));
-    }
-    return *result;
+  template <typename T> T* getDataMutable(absl::string_view data_name) {
+    return dynamic_cast<T*>(getDataMutableGeneric(data_name));
   }
 
+  /**
+   * @param data_name the name of the data being looked up (mutable/readonly).
+   * @return a pointer to the stored data or nullptr if the data does not exist.
+   * An exception will be thrown if the data is not mutable.
+   */
+  virtual Object* getDataMutableGeneric(absl::string_view data_name) PURE;
+
   /**
    * @param data_name the name of the data being probed.
    * @return Whether data of the type and name specified exists in the
    * data store.
    */
   template <typename T> bool hasData(absl::string_view data_name) const {
-    return (hasDataWithName(data_name) &&
-            (dynamic_cast<const T*>(getDataReadOnlyGeneric(data_name)) != nullptr));
+    return getDataReadOnly<T>(data_name) != nullptr;
   }
 
   /**
@@ -159,9 +149,6 @@ class FilterState {
    * either the top LifeSpan or the parent is not yet created.
    */
   virtual FilterStateSharedPtr parent() const PURE;
-
-protected:
-  virtual Object* getDataMutableGeneric(absl::string_view data_name) PURE;
 };
 
 } // namespace StreamInfo

source/common/formatter/substitution_formatter.cc

@@ -1490,10 +1490,7 @@ FilterStateFormatter::FilterStateFormatter(const std::string& key,
 const Envoy::StreamInfo::FilterState::Object*
 FilterStateFormatter::filterState(const StreamInfo::StreamInfo& stream_info) const {
   const StreamInfo::FilterState& filter_state = stream_info.filterState();
-  if (!filter_state.hasDataWithName(key_)) {
-    return nullptr;
-  }
-  return &filter_state.getDataReadOnly<StreamInfo::FilterState::Object>(key_);
+  return filter_state.getDataReadOnly<StreamInfo::FilterState::Object>(key_);
 }
 
 absl::optional<std::string> FilterStateFormatter::format(const Http::RequestHeaderMap&,

source/common/http/hash_policy.cc

@@ -163,8 +163,8 @@ class FilterStateHashMethod : public HashMethodImplBase {
   evaluate(const Network::Address::Instance*, const RequestHeaderMap&,
            const HashPolicy::AddCookieCallback,
            const StreamInfo::FilterStateSharedPtr filter_state) const override {
-    if (filter_state->hasData<Hashable>(key_)) {
-      return filter_state->getDataReadOnly<Hashable>(key_).hash();
+    if (auto typed_state = filter_state->getDataReadOnly<Hashable>(key_); typed_state != nullptr) {
+      return typed_state->hash();
     }
     return absl::nullopt;
   }

source/common/network/hash_policy.cc

@@ -28,8 +28,8 @@ class FilterStateHashMethod : public HashPolicyImpl::HashMethod {
 
   absl::optional<uint64_t> evaluate(const Network::Connection& connection) const override {
     const auto& filter_state = connection.streamInfo().filterState();
-    if (filter_state.hasData<Hashable>(key_)) {
-      return filter_state.getDataReadOnly<Hashable>(key_).hash();
+    if (auto typed_state = filter_state.getDataReadOnly<Hashable>(key_); typed_state != nullptr) {
+      return typed_state->hash();
     }
     return absl::nullopt;
   }

source/common/network/transport_socket_options_impl.cc

@@ -67,31 +67,30 @@ TransportSocketOptionsUtility::fromFilterState(const StreamInfo::FilterState& fi
   absl::optional<Network::ProxyProtocolData> proxy_protocol_options;
 
   bool needs_transport_socket_options = false;
-  if (filter_state.hasData<UpstreamServerName>(UpstreamServerName::key())) {
-    const auto& upstream_server_name =
-        filter_state.getDataReadOnly<UpstreamServerName>(UpstreamServerName::key());
-    server_name = upstream_server_name.value();
+  if (auto typed_data = filter_state.getDataReadOnly<UpstreamServerName>(UpstreamServerName::key());
+      typed_data != nullptr) {
+    server_name = typed_data->value();
     needs_transport_socket_options = true;
   }
 
-  if (filter_state.hasData<Network::ApplicationProtocols>(Network::ApplicationProtocols::key())) {
-    const auto& alpn = filter_state.getDataReadOnly<Network::ApplicationProtocols>(
-        Network::ApplicationProtocols::key());
-    application_protocols = alpn.value();
+  if (auto typed_data = filter_state.getDataReadOnly<Network::ApplicationProtocols>(
+          Network::ApplicationProtocols::key());
+      typed_data != nullptr) {
+    application_protocols = typed_data->value();
     needs_transport_socket_options = true;
   }
 
-  if (filter_state.hasData<UpstreamSubjectAltNames>(UpstreamSubjectAltNames::key())) {
-    const auto& upstream_subject_alt_names =
-        filter_state.getDataReadOnly<UpstreamSubjectAltNames>(UpstreamSubjectAltNames::key());
-    subject_alt_names = upstream_subject_alt_names.value();
+  if (auto typed_data =
+          filter_state.getDataReadOnly<UpstreamSubjectAltNames>(UpstreamSubjectAltNames::key());
+      typed_data != nullptr) {
+    subject_alt_names = typed_data->value();
     needs_transport_socket_options = true;
   }
 
-  if (filter_state.hasData<ProxyProtocolFilterState>(ProxyProtocolFilterState::key())) {
-    const auto& proxy_protocol_filter_state =
-        filter_state.getDataReadOnly<ProxyProtocolFilterState>(ProxyProtocolFilterState::key());
-    proxy_protocol_options.emplace(proxy_protocol_filter_state.value());
+  if (auto typed_data =
+          filter_state.getDataReadOnly<ProxyProtocolFilterState>(ProxyProtocolFilterState::key());
+      typed_data != nullptr) {
+    proxy_protocol_options.emplace(typed_data->value());
     needs_transport_socket_options = true;
   }
 

source/common/router/config_impl.cc

@@ -676,11 +676,12 @@ void RouteEntryImplBase::finalizeRequestHeaders(Http::RequestHeaderMap& headers,
   // Restore the port if this was a CONNECT request.
   // Note this will restore the port for HTTP/2 CONNECT-upgrades as well as as HTTP/1.1 style
   // CONNECT requests.
-  if (stream_info.filterState().hasData<OriginalConnectPort>(OriginalConnectPort::key()) &&
-      Http::HeaderUtility::getPortStart(headers.getHostValue()) == absl::string_view::npos) {
-    const OriginalConnectPort& original_port =
-        stream_info.filterState().getDataReadOnly<OriginalConnectPort>(OriginalConnectPort::key());
-    headers.setHost(absl::StrCat(headers.getHostValue(), ":", original_port.value()));
+  if (Http::HeaderUtility::getPortStart(headers.getHostValue()) == absl::string_view::npos) {
+    if (auto typed_state = stream_info.filterState().getDataReadOnly<OriginalConnectPort>(
+            OriginalConnectPort::key());
+        typed_state != nullptr) {
+      headers.setHost(absl::StrCat(headers.getHostValue(), ":", typed_state->value()));
+    }
   }
 
   if (!host_rewrite_.empty()) {

source/common/router/header_formatter.cc

@@ -179,21 +179,18 @@ parsePerRequestStateField(absl::string_view param_str) {
   return [param](const Envoy::StreamInfo::StreamInfo& stream_info) -> std::string {
     const Envoy::StreamInfo::FilterState& filter_state = stream_info.filterState();
 
-    // No such value means don't output anything.
-    if (!filter_state.hasDataWithName(param)) {
-      return std::string();
-    }
+    auto typed_state = filter_state.getDataReadOnly<StringAccessor>(param);
 
     // Value exists but isn't string accessible is a contract violation; throw an error.
-    if (!filter_state.hasData<StringAccessor>(param)) {
+    if (typed_state == nullptr) {
       ENVOY_LOG_MISC(debug,
                      "Invalid header information: PER_REQUEST_STATE value \"{}\" "
                      "exists but is not string accessible",
                      param);
       return std::string();
     }
 
-    return std::string(filter_state.getDataReadOnly<StringAccessor>(param).asString());
+    return std::string(typed_state->asString());
   };
 }
 

source/common/router/router.cc

@@ -385,10 +385,7 @@ Http::FilterHeadersStatus Filter::decodeHeaders(Http::RequestHeaderMap& headers,
   // we should append cluster and host headers to the response, and whether to forward the request
   // upstream.
   const StreamInfo::FilterStateSharedPtr& filter_state = callbacks_->streamInfo().filterState();
-  const DebugConfig* debug_config =
-      filter_state->hasData<DebugConfig>(DebugConfig::key())
-          ? &(filter_state->getDataReadOnly<DebugConfig>(DebugConfig::key()))
-          : nullptr;
+  const DebugConfig* debug_config = filter_state->getDataReadOnly<DebugConfig>(DebugConfig::key());
 
   // TODO: Maybe add a filter API for this.
   grpc_request_ = Grpc::Common::isGrpcRequestHeaders(headers);
@@ -567,24 +564,18 @@ Http::FilterHeadersStatus Filter::decodeHeaders(Http::RequestHeaderMap& headers,
   transport_socket_options_ = Network::TransportSocketOptionsUtility::fromFilterState(
       *callbacks_->streamInfo().filterState());
 
-  auto has_options_from_downstream =
-      downstreamConnection() && downstreamConnection()
-                                    ->streamInfo()
-                                    .filterState()
-                                    .hasData<Network::UpstreamSocketOptionsFilterState>(
-                                        Network::UpstreamSocketOptionsFilterState::key());
-
-  if (has_options_from_downstream) {
-    auto downstream_options = downstreamConnection()
-                                  ->streamInfo()
-                                  .filterState()
-                                  .getDataReadOnly<Network::UpstreamSocketOptionsFilterState>(
-                                      Network::UpstreamSocketOptionsFilterState::key())
-                                  .value();
-    if (!upstream_options_) {
-      upstream_options_ = std::make_shared<Network::Socket::Options>();
+  if (auto downstream_connection = downstreamConnection(); downstream_connection != nullptr) {
+    if (auto typed_state = downstream_connection->streamInfo()
+                               .filterState()
+                               .getDataReadOnly<Network::UpstreamSocketOptionsFilterState>(
+                                   Network::UpstreamSocketOptionsFilterState::key());
+        typed_state != nullptr) {
+      auto downstream_options = typed_state->value();
+      if (!upstream_options_) {
+        upstream_options_ = std::make_shared<Network::Socket::Options>();
+      }
+      Network::Socket::appendOptions(upstream_options_, downstream_options);
     }
-    Network::Socket::appendOptions(upstream_options_, downstream_options);
   }
 
   if (upstream_options_ && callbacks_->getUpstreamSocketOptions()) {
@@ -1609,15 +1600,20 @@ bool Filter::convertRequestHeadersForInternalRedirect(Http::RequestHeaderMap& do
   const StreamInfo::FilterStateSharedPtr& filter_state = callbacks_->streamInfo().filterState();
   // Make sure that performing the redirect won't result in exceeding the configured number of
   // redirects allowed for this route.
-  if (!filter_state->hasData<StreamInfo::UInt32Accessor>(NumInternalRedirectsFilterStateName)) {
-    filter_state->setData(
-        NumInternalRedirectsFilterStateName, std::make_shared<StreamInfo::UInt32AccessorImpl>(0),
-        StreamInfo::FilterState::StateType::Mutable, StreamInfo::FilterState::LifeSpan::Request);
+  StreamInfo::UInt32Accessor* num_internal_redirect{};
+
+  if (num_internal_redirect = filter_state->getDataMutable<StreamInfo::UInt32Accessor>(
+          NumInternalRedirectsFilterStateName);
+      num_internal_redirect == nullptr) {
+    auto state = std::make_shared<StreamInfo::UInt32AccessorImpl>(0);
+    num_internal_redirect = state.get();
+
+    filter_state->setData(NumInternalRedirectsFilterStateName, std::move(state),
+                          StreamInfo::FilterState::StateType::Mutable,
+                          StreamInfo::FilterState::LifeSpan::Request);
   }
-  StreamInfo::UInt32Accessor& num_internal_redirect =
-      filter_state->getDataMutable<StreamInfo::UInt32Accessor>(NumInternalRedirectsFilterStateName);
 
-  if (num_internal_redirect.value() >= policy.maxInternalRedirects()) {
+  if (num_internal_redirect->value() >= policy.maxInternalRedirects()) {
     ENVOY_STREAM_LOG(trace, "Internal redirect failed: redirect limits exceeded.", *callbacks_);
     config_.stats_.passthrough_internal_redirect_too_many_redirects_.inc();
     return false;
@@ -1681,7 +1677,7 @@ bool Filter::convertRequestHeadersForInternalRedirect(Http::RequestHeaderMap& do
     callbacks_->modifyDecodingBuffer([](Buffer::Instance& data) { data.drain(data.length()); });
   }
 
-  num_internal_redirect.increment();
+  num_internal_redirect->increment();
   restore_original_headers.cancel();
   // Preserve the original request URL for the second pass.
   downstream_headers.setEnvoyOriginalUrl(absl::StrCat(scheme_is_http

source/common/stream_info/filter_state_impl.cc

@@ -47,13 +47,13 @@ bool FilterStateImpl::hasDataWithName(absl::string_view data_name) const {
 
 const FilterState::Object*
 FilterStateImpl::getDataReadOnlyGeneric(absl::string_view data_name) const {
-  const auto& it = data_storage_.find(data_name);
+  const auto it = data_storage_.find(data_name);
 
   if (it == data_storage_.end()) {
     if (parent_) {
-      return &(parent_->getDataReadOnly<FilterState::Object>(data_name));
+      return parent_->getDataReadOnlyGeneric(data_name);
     }
-    throw EnvoyException("FilterState::getDataReadOnly<T> called for unknown data name.");
+    return nullptr;
   }
 
   const FilterStateImpl::FilterObject* current = it->second.get();
@@ -65,9 +65,9 @@ FilterState::Object* FilterStateImpl::getDataMutableGeneric(absl::string_view da
 
   if (it == data_storage_.end()) {
     if (parent_) {
-      return &(parent_->getDataMutable<FilterState::Object>(data_name));
+      return parent_->getDataMutableGeneric(data_name);
     }
-    throw EnvoyException("FilterState::getDataMutable<T> called for unknown data name.");
+    return nullptr;
   }
 
   FilterStateImpl::FilterObject* current = it->second.get();

source/common/tcp_proxy/tcp_proxy.cc

@@ -132,13 +132,11 @@ Config::Config(const envoy::extensions::filters::network::tcp_proxy::v3::TcpProx
 
 RouteConstSharedPtr Config::getRegularRouteFromEntries(Network::Connection& connection) {
   // First check if the per-connection state to see if we need to route to a pre-selected cluster
-  if (connection.streamInfo().filterState()->hasData<PerConnectionCluster>(
-          PerConnectionCluster::key())) {
-    const PerConnectionCluster& per_connection_cluster =
-        connection.streamInfo().filterState()->getDataReadOnly<PerConnectionCluster>(
-            PerConnectionCluster::key());
-
-    return std::make_shared<const SimpleRouteImpl>(*this, per_connection_cluster.value());
+  if (const auto* per_connection_cluster =
+          connection.streamInfo().filterState()->getDataReadOnly<PerConnectionCluster>(
+              PerConnectionCluster::key());
+      per_connection_cluster != nullptr) {
+    return std::make_shared<const SimpleRouteImpl>(*this, per_connection_cluster->value());
   }
 
   if (default_route_ != nullptr) {
@@ -363,37 +361,32 @@ Network::FilterStatus Filter::initializeUpstreamConnection() {
     return Network::FilterStatus::StopIteration;
   }
 
-  if (downstreamConnection()) {
+  if (auto downstream_connection = downstreamConnection(); downstream_connection != nullptr) {
     if (!read_callbacks_->connection()
              .streamInfo()
              .filterState()
              ->hasData<Network::ProxyProtocolFilterState>(
                  Network::ProxyProtocolFilterState::key())) {
       read_callbacks_->connection().streamInfo().filterState()->setData(
           Network::ProxyProtocolFilterState::key(),
-          std::make_unique<Network::ProxyProtocolFilterState>(Network::ProxyProtocolData{
-              downstreamConnection()->connectionInfoProvider().remoteAddress(),
-              downstreamConnection()->connectionInfoProvider().localAddress()}),
+          std::make_shared<Network::ProxyProtocolFilterState>(Network::ProxyProtocolData{
+              downstream_connection->connectionInfoProvider().remoteAddress(),
+              downstream_connection->connectionInfoProvider().localAddress()}),
           StreamInfo::FilterState::StateType::ReadOnly,
           StreamInfo::FilterState::LifeSpan::Connection);
     }
     transport_socket_options_ = Network::TransportSocketOptionsUtility::fromFilterState(
-        downstreamConnection()->streamInfo().filterState());
-
-    auto has_options_from_downstream =
-        downstreamConnection() && downstreamConnection()
-                                      ->streamInfo()
-                                      .filterState()
-                                      .hasData<Network::UpstreamSocketOptionsFilterState>(
-                                          Network::UpstreamSocketOptionsFilterState::key());
-    if (has_options_from_downstream) {
-      auto downstream_options = downstreamConnection()
-                                    ->streamInfo()
-                                    .filterState()
-                                    .getDataReadOnly<Network::UpstreamSocketOptionsFilterState>(
-                                        Network::UpstreamSocketOptionsFilterState::key())
-                                    .value();
-      upstream_options_ = std::make_shared<Network::Socket::Options>();
+        downstream_connection->streamInfo().filterState());
+
+    if (auto typed_state = downstream_connection->streamInfo()
+                               .filterState()
+                               .getDataReadOnly<Network::UpstreamSocketOptionsFilterState>(
+                                   Network::UpstreamSocketOptionsFilterState::key());
+        typed_state != nullptr) {
+      auto downstream_options = typed_state->value();
+      if (!upstream_options_) {
+        upstream_options_ = std::make_shared<Network::Socket::Options>();
+      }
       Network::Socket::appendOptions(upstream_options_, downstream_options);
     }
   }