Skip to content

Commit c654069

Browse files
authored
Don't allocate extra array and a ResponseClassifier during pipeline building (Azure#25441)
1 parent 9c9ec82 commit c654069

15 files changed

+77
-54
lines changed

sdk/core/Azure.Core.Experimental/tests/LowLevelClient/PetStoreClient.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -59,7 +59,7 @@ public PetStoreClient(Uri endpoint, TokenCredential credential, PetStoreClientOp
5959
var authPolicy = new BearerTokenAuthenticationPolicy(_tokenCredential, AuthorizationScopes);
6060

6161
// TODO: When we move the IsError functionality into Core, we'll move the addition of ResponsePropertiesPolicy to the pipeline into HttpPipelineBuilder.
62-
Pipeline = HttpPipelineBuilder.Build(options, new HttpPipelinePolicy[] { new LowLevelCallbackPolicy() }, new HttpPipelinePolicy[] { authPolicy, new ResponsePropertiesPolicy(options) }, new ResponseClassifier());
62+
Pipeline = HttpPipelineBuilder.Build(options, new HttpPipelinePolicy[] { new LowLevelCallbackPolicy() }, new HttpPipelinePolicy[] { authPolicy, new ResponsePropertiesPolicy(options) }, null);
6363
this.endpoint = endpoint;
6464
apiVersion = options.Version;
6565
}

sdk/core/Azure.Core/api/Azure.Core.net461.cs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -775,8 +775,8 @@ public void Send(Azure.Core.HttpMessage message, System.Threading.CancellationTo
775775
public static partial class HttpPipelineBuilder
776776
{
777777
public static Azure.Core.Pipeline.HttpPipeline Build(Azure.Core.ClientOptions options, params Azure.Core.Pipeline.HttpPipelinePolicy[] perRetryPolicies) { throw null; }
778-
public static Azure.Core.Pipeline.HttpPipeline Build(Azure.Core.ClientOptions options, Azure.Core.Pipeline.HttpPipelinePolicy[] perCallPolicies, Azure.Core.Pipeline.HttpPipelinePolicy[] perRetryPolicies, Azure.Core.ResponseClassifier responseClassifier) { throw null; }
779-
public static Azure.Core.Pipeline.HttpPipeline Build(Azure.Core.ClientOptions options, Azure.Core.Pipeline.HttpPipelinePolicy[] perCallPolicies, Azure.Core.Pipeline.HttpPipelinePolicy[] perRetryPolicies, Azure.Core.ResponseClassifier responseClassifier, Azure.Core.Pipeline.HttpPipelineTransportOptions? defaultTransportOptions) { throw null; }
778+
public static Azure.Core.Pipeline.HttpPipeline Build(Azure.Core.ClientOptions options, Azure.Core.Pipeline.HttpPipelinePolicy[] perCallPolicies, Azure.Core.Pipeline.HttpPipelinePolicy[] perRetryPolicies, Azure.Core.ResponseClassifier? responseClassifier) { throw null; }
779+
public static Azure.Core.Pipeline.HttpPipeline Build(Azure.Core.ClientOptions options, Azure.Core.Pipeline.HttpPipelinePolicy[] perCallPolicies, Azure.Core.Pipeline.HttpPipelinePolicy[] perRetryPolicies, Azure.Core.ResponseClassifier? responseClassifier, Azure.Core.Pipeline.HttpPipelineTransportOptions? defaultTransportOptions) { throw null; }
780780
}
781781
public abstract partial class HttpPipelinePolicy
782782
{

sdk/core/Azure.Core/api/Azure.Core.net5.0.cs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -775,8 +775,8 @@ public void Send(Azure.Core.HttpMessage message, System.Threading.CancellationTo
775775
public static partial class HttpPipelineBuilder
776776
{
777777
public static Azure.Core.Pipeline.HttpPipeline Build(Azure.Core.ClientOptions options, params Azure.Core.Pipeline.HttpPipelinePolicy[] perRetryPolicies) { throw null; }
778-
public static Azure.Core.Pipeline.HttpPipeline Build(Azure.Core.ClientOptions options, Azure.Core.Pipeline.HttpPipelinePolicy[] perCallPolicies, Azure.Core.Pipeline.HttpPipelinePolicy[] perRetryPolicies, Azure.Core.ResponseClassifier responseClassifier) { throw null; }
779-
public static Azure.Core.Pipeline.HttpPipeline Build(Azure.Core.ClientOptions options, Azure.Core.Pipeline.HttpPipelinePolicy[] perCallPolicies, Azure.Core.Pipeline.HttpPipelinePolicy[] perRetryPolicies, Azure.Core.ResponseClassifier responseClassifier, Azure.Core.Pipeline.HttpPipelineTransportOptions? defaultTransportOptions) { throw null; }
778+
public static Azure.Core.Pipeline.HttpPipeline Build(Azure.Core.ClientOptions options, Azure.Core.Pipeline.HttpPipelinePolicy[] perCallPolicies, Azure.Core.Pipeline.HttpPipelinePolicy[] perRetryPolicies, Azure.Core.ResponseClassifier? responseClassifier) { throw null; }
779+
public static Azure.Core.Pipeline.HttpPipeline Build(Azure.Core.ClientOptions options, Azure.Core.Pipeline.HttpPipelinePolicy[] perCallPolicies, Azure.Core.Pipeline.HttpPipelinePolicy[] perRetryPolicies, Azure.Core.ResponseClassifier? responseClassifier, Azure.Core.Pipeline.HttpPipelineTransportOptions? defaultTransportOptions) { throw null; }
780780
}
781781
public abstract partial class HttpPipelinePolicy
782782
{

sdk/core/Azure.Core/api/Azure.Core.netcoreapp2.1.cs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -775,8 +775,8 @@ public void Send(Azure.Core.HttpMessage message, System.Threading.CancellationTo
775775
public static partial class HttpPipelineBuilder
776776
{
777777
public static Azure.Core.Pipeline.HttpPipeline Build(Azure.Core.ClientOptions options, params Azure.Core.Pipeline.HttpPipelinePolicy[] perRetryPolicies) { throw null; }
778-
public static Azure.Core.Pipeline.HttpPipeline Build(Azure.Core.ClientOptions options, Azure.Core.Pipeline.HttpPipelinePolicy[] perCallPolicies, Azure.Core.Pipeline.HttpPipelinePolicy[] perRetryPolicies, Azure.Core.ResponseClassifier responseClassifier) { throw null; }
779-
public static Azure.Core.Pipeline.HttpPipeline Build(Azure.Core.ClientOptions options, Azure.Core.Pipeline.HttpPipelinePolicy[] perCallPolicies, Azure.Core.Pipeline.HttpPipelinePolicy[] perRetryPolicies, Azure.Core.ResponseClassifier responseClassifier, Azure.Core.Pipeline.HttpPipelineTransportOptions? defaultTransportOptions) { throw null; }
778+
public static Azure.Core.Pipeline.HttpPipeline Build(Azure.Core.ClientOptions options, Azure.Core.Pipeline.HttpPipelinePolicy[] perCallPolicies, Azure.Core.Pipeline.HttpPipelinePolicy[] perRetryPolicies, Azure.Core.ResponseClassifier? responseClassifier) { throw null; }
779+
public static Azure.Core.Pipeline.HttpPipeline Build(Azure.Core.ClientOptions options, Azure.Core.Pipeline.HttpPipelinePolicy[] perCallPolicies, Azure.Core.Pipeline.HttpPipelinePolicy[] perRetryPolicies, Azure.Core.ResponseClassifier? responseClassifier, Azure.Core.Pipeline.HttpPipelineTransportOptions? defaultTransportOptions) { throw null; }
780780
}
781781
public abstract partial class HttpPipelinePolicy
782782
{

sdk/core/Azure.Core/api/Azure.Core.netstandard2.0.cs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -775,8 +775,8 @@ public void Send(Azure.Core.HttpMessage message, System.Threading.CancellationTo
775775
public static partial class HttpPipelineBuilder
776776
{
777777
public static Azure.Core.Pipeline.HttpPipeline Build(Azure.Core.ClientOptions options, params Azure.Core.Pipeline.HttpPipelinePolicy[] perRetryPolicies) { throw null; }
778-
public static Azure.Core.Pipeline.HttpPipeline Build(Azure.Core.ClientOptions options, Azure.Core.Pipeline.HttpPipelinePolicy[] perCallPolicies, Azure.Core.Pipeline.HttpPipelinePolicy[] perRetryPolicies, Azure.Core.ResponseClassifier responseClassifier) { throw null; }
779-
public static Azure.Core.Pipeline.HttpPipeline Build(Azure.Core.ClientOptions options, Azure.Core.Pipeline.HttpPipelinePolicy[] perCallPolicies, Azure.Core.Pipeline.HttpPipelinePolicy[] perRetryPolicies, Azure.Core.ResponseClassifier responseClassifier, Azure.Core.Pipeline.HttpPipelineTransportOptions? defaultTransportOptions) { throw null; }
778+
public static Azure.Core.Pipeline.HttpPipeline Build(Azure.Core.ClientOptions options, Azure.Core.Pipeline.HttpPipelinePolicy[] perCallPolicies, Azure.Core.Pipeline.HttpPipelinePolicy[] perRetryPolicies, Azure.Core.ResponseClassifier? responseClassifier) { throw null; }
779+
public static Azure.Core.Pipeline.HttpPipeline Build(Azure.Core.ClientOptions options, Azure.Core.Pipeline.HttpPipelinePolicy[] perCallPolicies, Azure.Core.Pipeline.HttpPipelinePolicy[] perRetryPolicies, Azure.Core.ResponseClassifier? responseClassifier, Azure.Core.Pipeline.HttpPipelineTransportOptions? defaultTransportOptions) { throw null; }
780780
}
781781
public abstract partial class HttpPipelinePolicy
782782
{

sdk/core/Azure.Core/src/Pipeline/DisposableHttpPipeline.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@ public sealed class DisposableHttpPipeline : HttpPipeline, IDisposable
1818
/// <param name="perRetryIndex"></param>
1919
/// <param name="policies">Policies to be invoked as part of the pipeline in order.</param>
2020
/// <param name="responseClassifier">The response classifier to be used in invocations.</param>
21-
internal DisposableHttpPipeline(HttpPipelineTransport transport, int perCallIndex, int perRetryIndex, HttpPipelinePolicy[]? policies = null, ResponseClassifier? responseClassifier = null)
21+
internal DisposableHttpPipeline(HttpPipelineTransport transport, int perCallIndex, int perRetryIndex, HttpPipelinePolicy[] policies, ResponseClassifier responseClassifier)
2222
: base(transport, perCallIndex, perRetryIndex, policies, responseClassifier)
2323
{
2424
}

sdk/core/Azure.Core/src/Pipeline/HttpPipeline.cs

Lines changed: 16 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44
using System;
55
using System.Buffers;
66
using System.Collections.Generic;
7+
using System.Diagnostics;
78
using System.Threading;
89
using System.Threading.Tasks;
910

@@ -48,9 +49,9 @@ public class HttpPipeline
4849
public HttpPipeline(HttpPipelineTransport transport, HttpPipelinePolicy[]? policies = null, ResponseClassifier? responseClassifier = null)
4950
{
5051
_transport = transport ?? throw new ArgumentNullException(nameof(transport));
51-
ResponseClassifier = responseClassifier ?? new ResponseClassifier();
52+
ResponseClassifier = responseClassifier ?? ResponseClassifier.Shared;
5253

53-
policies = policies ?? Array.Empty<HttpPipelinePolicy>();
54+
policies ??= Array.Empty<HttpPipelinePolicy>();
5455

5556
var all = new HttpPipelinePolicy[policies.Length + 1];
5657
all[policies.Length] = new HttpPipelineTransportPolicy(_transport);
@@ -59,8 +60,20 @@ public HttpPipeline(HttpPipelineTransport transport, HttpPipelinePolicy[]? polic
5960
_pipeline = all;
6061
}
6162

62-
internal HttpPipeline(HttpPipelineTransport transport, int perCallIndex, int perRetryIndex, HttpPipelinePolicy[]? policies = null, ResponseClassifier? responseClassifier = null) : this(transport, policies, responseClassifier)
63+
internal HttpPipeline(
64+
HttpPipelineTransport transport,
65+
int perCallIndex,
66+
int perRetryIndex,
67+
HttpPipelinePolicy[] pipeline,
68+
ResponseClassifier responseClassifier)
6369
{
70+
ResponseClassifier = responseClassifier ?? throw new ArgumentNullException(nameof(responseClassifier));
71+
72+
_transport = transport ?? throw new ArgumentNullException(nameof(transport));
73+
_pipeline = pipeline ?? throw new ArgumentNullException(nameof(pipeline));
74+
75+
Debug.Assert(pipeline[pipeline.Length - 1] is HttpPipelineTransportPolicy);
76+
6477
_perCallIndex = perCallIndex;
6578
_perRetryIndex = perRetryIndex;
6679
_internallyConstructed = true;

sdk/core/Azure.Core/src/Pipeline/HttpPipelineBuilder.cs

Lines changed: 20 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,7 @@ public static class HttpPipelineBuilder
2222
/// <returns>A new instance of <see cref="HttpPipeline"/></returns>
2323
public static HttpPipeline Build(ClientOptions options, params HttpPipelinePolicy[] perRetryPolicies)
2424
{
25-
return Build(options, Array.Empty<HttpPipelinePolicy>(), perRetryPolicies, new ResponseClassifier());
25+
return Build(options, Array.Empty<HttpPipelinePolicy>(), perRetryPolicies, ResponseClassifier.Shared);
2626
}
2727

2828
/// <summary>
@@ -37,7 +37,7 @@ public static HttpPipeline Build(
3737
ClientOptions options,
3838
HttpPipelinePolicy[] perCallPolicies,
3939
HttpPipelinePolicy[] perRetryPolicies,
40-
ResponseClassifier responseClassifier)
40+
ResponseClassifier? responseClassifier)
4141
{
4242
return Build(options, perCallPolicies, perRetryPolicies, responseClassifier, null);
4343
}
@@ -51,10 +51,8 @@ public static HttpPipeline Build(
5151
/// <param name="responseClassifier">The client provided response classifier.</param>
5252
/// <param name="defaultTransportOptions">The customer provided transport options which will be applied to the default transport.</param>
5353
/// <returns>A new instance of <see cref="HttpPipeline"/></returns>
54-
public static HttpPipeline Build(ClientOptions options, HttpPipelinePolicy[] perCallPolicies, HttpPipelinePolicy[] perRetryPolicies, ResponseClassifier responseClassifier, HttpPipelineTransportOptions? defaultTransportOptions)
54+
public static HttpPipeline Build(ClientOptions options, HttpPipelinePolicy[] perCallPolicies, HttpPipelinePolicy[] perRetryPolicies, ResponseClassifier? responseClassifier, HttpPipelineTransportOptions? defaultTransportOptions)
5555
{
56-
int perCallIndex;
57-
int perRetryIndex;
5856
if (perCallPolicies == null)
5957
{
6058
throw new ArgumentNullException(nameof(perCallPolicies));
@@ -97,7 +95,7 @@ void AddCustomerPolicies(HttpPipelinePosition position)
9795
AddCustomerPolicies(HttpPipelinePosition.PerCall);
9896

9997
policies.RemoveAll(static policy => policy == null);
100-
perCallIndex = policies.Count;
98+
var perCallIndex = policies.Count;
10199

102100
policies.Add(ClientRequestIdPolicy.Shared);
103101

@@ -116,7 +114,8 @@ void AddCustomerPolicies(HttpPipelinePosition position)
116114
AddCustomerPolicies(HttpPipelinePosition.PerRetry);
117115

118116
policies.RemoveAll(static policy => policy == null);
119-
perRetryIndex = policies.Count;
117+
118+
var perRetryIndex = policies.Count;
120119

121120
if (diagnostics.IsLoggingEnabled)
122121
{
@@ -134,6 +133,7 @@ void AddCustomerPolicies(HttpPipelinePosition position)
134133

135134
// Override the provided Transport with the provided transport options if the transport has not been set after default construction and options are not null.
136135
HttpPipelineTransport transport = options.Transport;
136+
bool disposablePipeline = false;
137137
if (defaultTransportOptions != null)
138138
{
139139
if (options.IsCustomTransportSet)
@@ -147,14 +147,22 @@ void AddCustomerPolicies(HttpPipelinePosition position)
147147
else
148148
{
149149
transport = HttpPipelineTransport.Create(defaultTransportOptions);
150-
return new DisposableHttpPipeline(transport,
151-
perCallIndex,
152-
perRetryIndex,
153-
policies.ToArray(),
154-
responseClassifier);
155150
}
156151
}
157152

153+
policies.Add(new HttpPipelineTransportPolicy(transport));
154+
155+
responseClassifier ??= ResponseClassifier.Shared;
156+
157+
if (disposablePipeline)
158+
{
159+
return new DisposableHttpPipeline(transport,
160+
perCallIndex,
161+
perRetryIndex,
162+
policies.ToArray(),
163+
responseClassifier);
164+
}
165+
158166
return new HttpPipeline(transport,
159167
perCallIndex,
160168
perRetryIndex,

sdk/core/Azure.Core/src/ResponseClassifier.cs

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,8 @@ namespace Azure.Core
1111
/// </summary>
1212
public class ResponseClassifier
1313
{
14+
internal static ResponseClassifier Shared { get; } = new();
15+
1416
/// <summary>
1517
/// Specifies if the request contained in the <paramref name="message"/> should be retried.
1618
/// </summary>

sdk/core/Azure.Core/tests/HttpPipelineBuilderTest.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -216,7 +216,7 @@ public void SetTransportOptions([Values(true, false)] bool isCustomTransportSet)
216216
options,
217217
Array.Empty<HttpPipelinePolicy>(),
218218
Array.Empty<HttpPipelinePolicy>(),
219-
new ResponseClassifier(),
219+
ResponseClassifier.Shared,
220220
new HttpPipelineTransportOptions());
221221

222222
HttpPipelineTransport transportField = pipeline.GetType().GetField("_transport", BindingFlags.Instance | BindingFlags.NonPublic | BindingFlags.GetField).GetValue(pipeline) as HttpPipelineTransport;

0 commit comments

Comments
 (0)