Repository navigation
Conversation
- HttpClientResponseData: copy each response header once from the enumerated values instead of re-looking every key up with TryGetValues; build GetHeaderNames lazily from the dictionary and answer IsHeaderPresent from the dictionary instead of a second HashSet. - User-Agent: reuse the last generated string per HttpHandler when the ordered components and feature ids are unchanged; merge request-level UA details without the intermediate join/enumerator; avoid the UTF-8 byte[] in the 1 KB truncation check. - HttpHandler: reuse the previously parsed request Uri when the composed URL string (and the canonicalization setting) is unchanged (non-BCL); skip attribute building/boxing in RecordHttpTelemetryData when the meter/tracer providers are the built-in no-op ones; skip RequestSize boxing when RequestMetrics is disabled. - HttpWebRequestMessage: use the shared HttpMethod instances for the standard methods; enumerate concrete Dictionary headers without boxing. HttpAdapterTests expects the runtime's own exception type for HttpMethod(null): netcoreapp3.1 throws ArgumentException where net472 and .NET 5+ throw ArgumentNullException, and GetHttpMethod passes null through unchanged.
jnunn-aws
force-pushed
the
jonatnun/http-adapter-trims
branch
from
October 22, 2026 19:08
b79c2c1 to
e55ee3e
Compare
8 of 16 tasks
…only header collisions
jnunn-aws
marked this pull request as ready for review
October 28, 2026 07:03
jnunn-aws
requested review from
dscpinheiro and
sichanyoo
and removed request for
a team
October 28, 2026 07:03
Merge near-duplicate tests into data rows, assert literal User-Agent values, and use the shared RecordingTelemetry.cs helpers. The response-header test still compares against the previous copy algorithm.
Only tests passed a cached entry; HttpHandler.GetUserAgent already does the matching through UserAgentDetails.Matches, so the method no longer takes one.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
enumerated values instead of re-looking every key up with TryGetValues;
build GetHeaderNames lazily from the dictionary and answer
IsHeaderPresent from the dictionary instead of a second HashSet.
ordered components and feature ids match exactly (a client alternating
sync and async calls, or operations with different feature ids, keeps one
entry per shape); merge request-level
UA details without the intermediate join/enumerator; avoid the UTF-8
byte[] in the 1 KB truncation check.
URL string (and the canonicalization setting) is unchanged (non-BCL);
skip attribute building and boxing in RecordHttpTelemetryData when the
trace span and meter provider are the SDK's no-op ones; skip RequestSize boxing when RequestMetrics
is disabled.
standard methods; enumerate concrete Dictionary headers without boxing.
HttpAdapterTests expects the runtime's own exception type for
HttpMethod(null): netcoreapp3.1 throws ArgumentException where net472 and
.NET 5+ throw ArgumentNullException, and GetHttpMethod passes null through
unchanged.
Motivation and Context
Measurement: Numbers come from the serde ops/CPU-sec runner (
sdk/test/Performance/SerdeBenchmarks,--cpu-time, stub transport), geomean of 71 cases, 3 interleaved runs. Instances: m7i.metal-24xl (x86) and m7g.metal (ARM64). Settings: net8.0, 20,000-iteration warmup, measured ondevelopment8956121, which has the same code as this branch's base for these files. Each change is measured alone against that base. Both metal runs measured an earlier revision of this PR that kept one User-Agent string per client instead of four. Onfeature/4.1(337f357, net10.0, 2,000-iteration warmup, 3 interleaved rounds, same instances) this change gained +5.8% on x86 and +5.1% on ARM. The final run at the harness defaults (net10.0, 100,000 warmup, stub and https) is tracked separately.Clients that alternate request shapes. A cache miss costs no more than today's code. In a development-host run (one client, alternating
GetObjectkeys and/or User-Agent components, stub transport, 9 interleaved reps), every alternating pattern was 6.6–7.8% cheaper per call thandevelopment, against 7.4% when nothing alternates. The −3.4% onawsQuery_GetMetricDataResponse_Sin thedevelopmentrun didn't reproduce onfeature/4.1(+4.9% x86, +3.9% ARM), where the worst of the 71 cases was −0.9% on x86, within the noise.Testing
HttpAdapterTests(29 on net8.0): response headers match a reference copy; User-Agent reuse regenerates when inputs differ and matches the generated header across requests;ComposeRequestUrlmatchesComposeUrlfor paths, queries and sub-resources and honors canonicalization changes, using the canonicalization setting read for the cache key;RequestSizerecorded only when request metrics are enabled (sync, and the async path on net472); standardHttpMethodreuse; HTTP metrics and span attributes with custom and no-op providers; a case-only name collision between response and content headers keeps the response header; User-Agent reuse with alternating shapes and with more shapes than cache entries.feature/4.17d795cb.Breaking Changes Assessment
None to the public API.
HttpClientResponseData(Amazon.Runtime.Internal.Transform) no longer throws when a response header and a content header differ only in case (the response header's value is kept), andIsHeaderPresentreturns false instead of throwingNullReferenceExceptionon event-stream message data.Credit
@dscpinheiro independently arrived at the same response-side change in 9b07f37: copying each response header once and answering
IsHeaderPresent/GetHeaderNamesfrom the header dictionary.Dry-runs
71ac0493-f653-41b9-83d1-de0212b1ff30f9ba88a1-149a-4962-9861-e7055ae8d82cChecklist
License