aspnet / Routing

[Archived] Middleware for routing requests to application logic. Project moved to https://github.com/aspnet/AspNetCore
Apache License 2.0
272 stars 122 forks source link

Use Array.Clone() to copy arrays, and avoid copy where applicable #855

Closed drieseng closed 6 years ago

drieseng commented 6 years ago

Use Array.Clone()to copy arrays, and avoid copy on overloads that do not take an array as argument (but would previously result in a IEnumerable<TSource>.ToArray() on an empty array).


BenchmarkDotNet=v0.10.13, OS=Windows 10.0.17134
Intel Core i7-6700K CPU 4.00GHz (Skylake), 1 CPU, 8 logical cores and 4 physical cores
Frequency=3914069 Hz, Resolution=255.4886 ns, Timer=TSC
.NET Core SDK=2.1.400
  [Host]     : .NET Core 2.2.0-preview3-26927-02 (CoreCLR 4.6.26927.02, CoreFX 4.6.26927.02), 64bit RyuJIT
  Job-GXNCHM : .NET Core 2.2.0-preview3-26927-02 (CoreCLR 4.6.26927.02, CoreFX 4.6.26927.02), 64bit RyuJIT

Runtime=Core  Server=True  Toolchain=.NET Core 2.2  
RunStrategy=Throughput  

Before:

Method Mean Error StdDev Op/s Gen 0 Allocated
Segment_IEnumerable 61.65 ns 0.3671 ns 0.3434 ns 16,220,567.5 0.0010 88 B
Segment_Array 88.65 ns 0.9344 ns 0.8283 ns 11,279,681.3 0.0010 88 B
ParameterPart_Name 85.28 ns 0.3587 ns 0.2996 ns 11,725,692.8 0.0006 56 B
ParameterPart_NameAndDefault 89.95 ns 0.8693 ns 0.8131 ns 11,116,857.3 0.0008 80 B
ParameterPart_NameAndDefaultAndKind 94.33 ns 0.3389 ns 0.3170 ns 10,600,945.5 0.0008 80 B
ParameterPart_NameAndDefaultAndKindPolicies_IEnumerable 91.43 ns 1.3934 ns 1.3034 ns 10,937,076.2 0.0015 144 B
ParameterPart_NameAndDefaultAndKindPolicies_Array 110.10 ns 0.4493 ns 0.3983 ns 9,082,426.6 0.0015 144 B
Pattern_Segments_Array 1,213.76 ns 3.5435 ns 2.9590 ns 823,882.8 0.0019 232 B
Pattern_Segments_IEnumerable 1,199.66 ns 9.2345 ns 8.6379 ns 833,570.6 0.0019 232 B

After:

Method Mean Error StdDev Op/s Gen 0 Allocated
Segment_IEnumerable 64.51 ns 1.3240 ns 1.5761 ns 15,500,607.3 0.0010 88 B
Segment_Array 54.87 ns 0.3814 ns 0.3381 ns 18,223,443.5 0.0010 88 B
ParameterPart_Name 27.38 ns 0.0366 ns 0.0305 ns 36,523,385.3 0.0007 56 B
ParameterPart_NameAndDefault 31.40 ns 0.4519 ns 0.4227 ns 31,845,325.5 0.0009 80 B
ParameterPart_NameAndDefaultAndKind 31.11 ns 0.0830 ns 0.0648 ns 32,147,564.8 0.0009 80 B
ParameterPart_NameAndDefaultAndKindPolicies_IEnumerable 91.52 ns 0.5890 ns 0.5509 ns 10,926,313.6 0.0015 144 B
ParameterPart_NameAndDefaultAndKindPolicies_Array 81.79 ns 0.4686 ns 0.4154 ns 12,226,700.0 0.0015 144 B
Pattern_Segments_Array 1,207.43 ns 1.5503 ns 1.3743 ns 828,205.4 0.0019 232 B
Pattern_Segments_IEnumerable 1,188.09 ns 6.9480 ns 6.4991 ns 841,688.0 0.0019 232 B
drieseng commented 6 years ago

If this approach is ok, I'll speed up the Pattern(…) overloads (that take an array) as well.

JamesNK commented 6 years ago

Where is the performance benefit coming from? Are there specific cases that are no longer copying, or is it from using Array.Clone?

I wonder why ToArray doesn't use Array.Clone internally if it is faster.

drieseng commented 6 years ago

Where is the performance benefit coming from? Are there specific cases that are no longer copying, or is it from using Array.Clone? I wonder why ToArray doesn't use Array.Clone internally if it is faster.

The speedup comes from the fact that previously an array was passed to - for example - ParameterPartCore(…) as an IEnumerable\ on which subsequently the ToArray<T>() extension method was invoked. This one treats an array of T as an ICollection\ to create a new array. It doesn't special case T[] so it can't just clone the array.

This was even done when we knew the array was empty, and as such no copy was necessary. Now the copy (clone) is done on the caller size, and the resulting array is then passed to ParameterPartCore(…) as an array. There's no Linq involved in the array case, and no virtual dispatch.

In case of factory method that does not take an array or IEnumerable, we avoid the copy altogether. This explains why the (micro)benefit is bigger for - for example - the ParameterPart_Name benchmark.

drieseng commented 6 years ago

Unrelated, but why doesn't the template for MatcherAzureBenchmarkBase combine multiple HTTP methods for a given endpoint in a single RouteEndpoint with a HttpMethodMetadata containing all HTTP methods for that endpoint? Wouldn't this be more representative?

Currently we have this:

            ...
            Endpoints[138] = CreateEndpoint("/portalsettings/signin", "PUT");
            Endpoints[139] = CreateEndpoint("/portalsettings/signin", "PATCH");
            Endpoints[140] = CreateEndpoint("/portalsettings/signin", "GET");
            ...
rynowak commented 6 years ago

Wouldn't this be more representative?

No not really. Typically these would be separate action methods.

drieseng commented 6 years ago

All PR feedback should have been addressed now.

For reference, the benchmark is now available here.