router: optimize router to avoid unnecessary headers/trailers copy#44323
router: optimize router to avoid unnecessary headers/trailers copy#44323wbpcode wants to merge 3 commits intoenvoyproxy:mainfrom
Conversation
Signed-off-by: wbpcode/wangbaiping <wbphub@gmail.com>
| if (shadow_headers_) { | ||
| shadow_trailers_ = Http::createHeaderMap<Http::RequestTrailerMapImpl>(trailers); | ||
| } |
There was a problem hiding this comment.
This is unnecessary because we will always to copy a new trailers map for every shadow stream.
| Http::RequestMessagePtr request(new Http::RequestMessageImpl( | ||
| Http::createHeaderMap<Http::RequestHeaderMapImpl>(*shadow_headers_))); | ||
| applyShadowPolicyHeaders(shadow_policy, request->headers()); | ||
| Http::RequestMessagePtr request(new Http::RequestMessageImpl(std::move(shadow_headers))); |
There was a problem hiding this comment.
to reuse the previously copied one shadow_headers rather then to copy another one here.
| active_shadow_policies_.push_back(std::cref(policy_ref)); | ||
| shadow_headers_ = Http::createHeaderMap<Http::RequestHeaderMapImpl>(*downstream_headers_); | ||
| active_shadow_policies.push_back(std::cref(policy_ref)); |
There was a problem hiding this comment.
It's unnecessary to copy the downstream headers again and again. And actually the shadow_headers_ will not be used actually.
… dev-cleanup-router-shadow-policy
botengyao
left a comment
botengyao
left a comment
There was a problem hiding this comment.
thanks for the optimization, have one concern.
/wait
source/common/router/router.cc
Outdated
| continue; | ||
| } | ||
| auto shadow_headers = Http::createHeaderMap<Http::RequestHeaderMapImpl>(*shadow_headers_); | ||
| auto shadow_headers = Http::createHeaderMap<Http::RequestHeaderMapImpl>(*downstream_headers_); |
There was a problem hiding this comment.
This looks like a behavior change since UpstreamRequest construction injects trace context into *parent_.downstreamHeaders(), and this is after the acceptHeadersFromRouter(), so it can make mirrored requests inherit upstream mutations, but before not?
There was a problem hiding this comment.
good point, I forgot the trace injection. I will update it.
Signed-off-by: wbpcode <wbphub@gmail.com>
|
/retest |
3 participants
Commit Message: router: optimize router to avoid unnecessary headers/trailers copy
Additional Description:
This initially is part of #44305 but I found I forgot to push it and this deserve another PR anyway.
Risk Level: low.
Testing: n/a.
Docs Changes: n/a.
Release Notes: n/a.
Platform Specific Features: n/a.