Skip to content

Commit bd28a2f

Browse files
authored
ext_proc: avoid copies introduced by auto assignment (#44657)
Commit Message: auto is great, not so much when it creates unnecessary copies Additional Description: part of issue #44795 Risk Level: LOW Testing: Docs Changes: Release Notes: Platform Specific Features: [Optional Runtime guard:] [Optional Fixes #Issue] [Optional Fixes commit #PR or SHA] [Optional Deprecated:] [Optional [API Considerations](https://github.com/envoyproxy/envoy/blob/main/api/review_checklist.md):] --------- Signed-off-by: Xin Zhuang <stevenzzz@google.com>
1 parent cd89465 commit bd28a2f

5 files changed

Lines changed: 22 additions & 17 deletions

File tree

source/extensions/filters/http/ext_proc/config.cc

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -88,7 +88,7 @@ ExternalProcessingFilterConfig::createFilterFactoryFromProtoTyped(
8888
PROTOBUF_GET_MS_OR_DEFAULT(proto_config, message_timeout, DefaultMessageTimeoutMs);
8989
const uint32_t max_message_timeout_ms =
9090
PROTOBUF_GET_MS_OR_DEFAULT(proto_config, max_message_timeout, DefaultMaxMessageTimeoutMs);
91-
const auto filter_config = std::make_shared<FilterConfig>(
91+
auto filter_config = std::make_shared<FilterConfig>(
9292
proto_config, std::chrono::milliseconds(message_timeout_ms), max_message_timeout_ms,
9393
dual_info.scope, stats_prefix, dual_info.is_upstream,
9494
Envoy::Extensions::Filters::Common::Expr::getBuilder(context), context);
@@ -140,7 +140,7 @@ ExternalProcessingFilterConfig::createFilterFactoryFromProtoWithServerContextTyp
140140
PROTOBUF_GET_MS_OR_DEFAULT(proto_config, message_timeout, DefaultMessageTimeoutMs);
141141
const uint32_t max_message_timeout_ms =
142142
PROTOBUF_GET_MS_OR_DEFAULT(proto_config, max_message_timeout, DefaultMaxMessageTimeoutMs);
143-
const auto filter_config = std::make_shared<FilterConfig>(
143+
auto filter_config = std::make_shared<FilterConfig>(
144144
proto_config, std::chrono::milliseconds(message_timeout_ms), max_message_timeout_ms,
145145
server_context.scope(), stats_prefix, false,
146146
Envoy::Extensions::Filters::Common::Expr::getBuilder(server_context), server_context);

source/extensions/filters/http/ext_proc/ext_proc.cc

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -718,7 +718,7 @@ void Filter::sendRequest(const ProcessorState& state, ProcessingRequest&& req, b
718718

719719
void Filter::onComplete(ProcessingResponse& response) {
720720
ENVOY_STREAM_LOG(debug, "Received successful response from server", *decoder_callbacks_);
721-
std::unique_ptr<ProcessingResponse> resp_ptr = std::make_unique<ProcessingResponse>(response);
721+
auto resp_ptr = std::make_unique<ProcessingResponse>(response);
722722
onReceiveMessage(std::move(resp_ptr));
723723
}
724724

@@ -1538,7 +1538,7 @@ void Filter::addDynamicMetadata(const ProcessorState& state, ProcessingRequest&
15381538
envoy::config::core::v3::Metadata forwarding_metadata;
15391539

15401540
// Forward cluster metadata if so configured.
1541-
const auto cluster_info = cb->streamInfo().upstreamClusterInfo();
1541+
const auto& cluster_info = cb->streamInfo().upstreamClusterInfo();
15421542
if (cluster_info) {
15431543
const auto& cluster_metadata = cluster_info->metadata().filter_metadata();
15441544
for (const auto& context_key : state.untypedClusterMetadataForwardingNamespaces()) {
@@ -1599,7 +1599,7 @@ void Filter::addDynamicMetadata(const ProcessorState& state, ProcessingRequest&
15991599
}
16001600
}
16011601

1602-
*req.mutable_metadata_context() = forwarding_metadata;
1602+
*req.mutable_metadata_context() = std::move(forwarding_metadata);
16031603
}
16041604

16051605
void Filter::addAttributes(ProcessorState& state, ProcessingRequest& req) {
@@ -1615,7 +1615,7 @@ void Filter::addAttributes(ProcessorState& state, ProcessingRequest& req) {
16151615
auto attributes = state.evaluateAttributes(config_->expressionManager(), *activation_ptr);
16161616

16171617
state.setSentAttributes(true);
1618-
(*req.mutable_attributes())[FilterName] = attributes;
1618+
(*req.mutable_attributes())[FilterName] = std::move(attributes);
16191619
}
16201620

16211621
void Filter::setDynamicMetadata(Http::StreamFilterCallbacks* cb, const ProcessorState& state,
@@ -1630,8 +1630,8 @@ void Filter::setDynamicMetadata(Http::StreamFilterCallbacks* cb, const Processor
16301630
return;
16311631
}
16321632

1633-
auto response_metadata = response.dynamic_metadata().fields();
1634-
auto receiving_namespaces = state.untypedReceivingMetadataNamespaces();
1633+
const auto& response_metadata = response.dynamic_metadata().fields();
1634+
const auto& receiving_namespaces = state.untypedReceivingMetadataNamespaces();
16351635
for (const auto& context_key : response_metadata) {
16361636
bool found_allowed_namespace = false;
16371637
if (auto metadata_it =

source/extensions/filters/http/ext_proc/http_client/http_client_impl.cc

Lines changed: 8 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -46,7 +46,8 @@ void ExtProcHttpClient::sendRequest(envoy::service::ext_proc::v3::ProcessingRequ
4646
// Transcode req message into JSON string.
4747
auto req_in_json = MessageUtil::getJsonStringFromMessage(req);
4848
if (req_in_json.ok()) {
49-
const auto http_uri = config_.http_service().http_service().http_uri();
49+
const envoy::config::core::v3::HttpUri& http_uri =
50+
config_.http_service().http_service().http_uri();
5051
Http::RequestHeaderMapPtr headers = buildHttpRequestHeaders(http_uri.uri(), stream_id);
5152
headers_applicator_->apply(*headers);
5253

@@ -76,12 +77,14 @@ void ExtProcHttpClient::onSuccess(const Http::AsyncClient::Request&,
7677
if (status.has_value()) {
7778
uint64_t status_code = status.value();
7879
if (status_code == Envoy::enumToInt(Envoy::Http::Code::OK)) {
79-
std::string msg_body = response->body().toString();
80-
ENVOY_LOG(debug, "Response status is OK, message body length {}", msg_body.size());
80+
const size_t body_len = response->body().length();
8181
envoy::service::ext_proc::v3::ProcessingResponse response_msg;
82-
if (!msg_body.empty()) {
82+
if (body_len > 0) {
83+
const absl::string_view msg_body(
84+
static_cast<const char*>(response->body().linearize(body_len)), body_len);
8385
bool has_unknown_field;
84-
auto status = MessageUtil::loadFromJsonNoThrow(msg_body, response_msg, has_unknown_field);
86+
const absl::Status& status =
87+
MessageUtil::loadFromJsonNoThrow(msg_body, response_msg, has_unknown_field);
8588
if (!status.ok()) {
8689
ENVOY_LOG(
8790
error,

source/extensions/filters/http/ext_proc/matching_utils.cc

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,8 @@ ExpressionManager::initExpressions(const Protobuf::RepeatedPtrField<std::string>
1919
if (expressions.contains(matcher)) {
2020
continue;
2121
}
22-
auto parse_status = google::api::expr::parser::Parse(matcher);
22+
const absl::StatusOr<cel::expr::ParsedExpr> parse_status =
23+
google::api::expr::parser::Parse(matcher);
2324
if (!parse_status.ok()) {
2425
throw EnvoyException("Unable to parse descriptor expression: " +
2526
parse_status.status().ToString());
@@ -55,7 +56,8 @@ ExpressionManager::evaluateAttributes(const Filters::Common::Expr::Activation& a
5556

5657
for (const auto& hash_entry : expr) {
5758
Protobuf::Arena arena;
58-
const auto result = hash_entry.second.evaluate(activation, &arena);
59+
const absl::StatusOr<google::api::expr::runtime::CelValue>& result =
60+
hash_entry.second.evaluate(activation, &arena);
5961
if (!result.ok()) {
6062
// TODO: Stats?
6163
continue;

source/extensions/filters/http/ext_proc/mutation_utils.cc

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -76,8 +76,8 @@ void MutationUtils::headersToProto(
7676
&disallowed_headers](const Http::HeaderEntry& e) -> Http::HeaderMap::Iterate {
7777
if (headerCanBeForwarded(e.key().getStringView(), allowed_headers, disallowed_headers)) {
7878
auto* new_header = proto_out.add_headers();
79-
new_header->set_key(std::string(e.key().getStringView()));
80-
new_header->set_raw_value(std::string(e.value().getStringView()));
79+
new_header->set_key(e.key().getStringView());
80+
new_header->set_raw_value(e.value().getStringView());
8181
}
8282
return Http::HeaderMap::Iterate::Continue;
8383
});

0 commit comments

Comments
 (0)