Modify S3169: Add benchmarks (#4595)
* Modify S3169: Add benchmarks * Update rules/S3169/csharp/rule.adoc Co-authored-by: Sebastien Marichal <sebastien.marichal@sonarsource.com> * Update benchmark * LAYC * diff-id --------- Co-authored-by: Sebastien Marichal <sebastien.marichal@sonarsource.com>
This commit is contained in:
parent
43247cd487
commit
4e15f3d653
@ -1,14 +1,16 @@
|
||||
== Why is this an issue?
|
||||
|
||||
There's no point in chaining multiple ``++OrderBy++`` calls in a LINQ; only the last one will be reflected in the result because each subsequent call completely reorders the list. Thus, calling ``++OrderBy++`` multiple times is a performance issue as well, because all of the sorting will be executed, but only the result of the last sort will be kept.
|
||||
There's no point in chaining multiple `OrderBy` calls in a LINQ; only the last one will be reflected in the result because each subsequent call completely reorders the list. Thus, calling `OrderBy` multiple times is a performance issue as well, because all of the sorting will be executed, but only the result of the last sort will be kept.
|
||||
|
||||
== How to fix it
|
||||
|
||||
Instead, use ``++ThenBy++`` for each call after the first.
|
||||
Instead, use `ThenBy` for each call after the first.
|
||||
|
||||
=== Code examples
|
||||
|
||||
=== Noncompliant code example
|
||||
==== Noncompliant code example
|
||||
|
||||
[source,csharp]
|
||||
[source,csharp,diff-id=1,diff-type=noncompliant]
|
||||
----
|
||||
var x = personList
|
||||
.OrderBy(person => person.Age)
|
||||
@ -17,9 +19,9 @@ var x = personList
|
||||
----
|
||||
|
||||
|
||||
=== Compliant solution
|
||||
==== Compliant solution
|
||||
|
||||
[source,csharp]
|
||||
[source,csharp,diff-id=1,diff-type=compliant]
|
||||
----
|
||||
var x = personList
|
||||
.OrderBy(person => person.Age)
|
||||
@ -27,6 +29,79 @@ var x = personList
|
||||
.ToList();
|
||||
----
|
||||
|
||||
== Resources
|
||||
|
||||
=== Benchmarks
|
||||
|
||||
[options="header"]
|
||||
|===
|
||||
| Method | Runtime | Mean | StdDev | Allocated
|
||||
| OrderByAge | .NET 9.0 | 12.84 ms | 0.804 ms | 1.53 MB
|
||||
| OrderByAgeOrderBySize | .NET 9.0 | 24.08 ms | 0.267 ms | 3.05 MB
|
||||
| OrderByAgeThenBySize | .NET 9.0 | 18.58 ms | 0.747 ms | 1.91 MB
|
||||
| | | | |
|
||||
| OrderByAge | .NET Framework 4.8.1 | 22.99 ms | 0.228 ms | 1.53 MB
|
||||
| OrderByAgeOrderBySize | .NET Framework 4.8.1 | 44.90 ms | 0.581 ms | 4.3 MB
|
||||
| OrderByAgeThenBySize | .NET Framework 4.8.1 | 31.72 ms | 0.402 ms | 1.91 MB
|
||||
|===
|
||||
|
||||
==== Glossary
|
||||
|
||||
* https://en.wikipedia.org/wiki/Arithmetic_mean[Mean]
|
||||
* https://en.wikipedia.org/wiki/Standard_deviation[Standard Deviation]
|
||||
|
||||
The results were generated by running the following snippet with https://github.com/dotnet/BenchmarkDotNet[BenchmarkDotNet]:
|
||||
|
||||
[source,csharp]
|
||||
----
|
||||
public class Person
|
||||
{
|
||||
public string Name { get; set; }
|
||||
public int Age { get; set; }
|
||||
public int Size { get; set; }
|
||||
}
|
||||
|
||||
private Random random = new Random(1);
|
||||
private Consumer consumer = new Consumer();
|
||||
private Person[] array;
|
||||
|
||||
[Params(100_000)]
|
||||
public int N { get; set; }
|
||||
|
||||
[GlobalSetup]
|
||||
public void GlobalSetup()
|
||||
{
|
||||
array = Enumerable.Range(0, N).Select(x => new Person
|
||||
{
|
||||
Name = Path.GetRandomFileName(),
|
||||
Age = random.Next(0, 100),
|
||||
Size = random.Next(0, 200)
|
||||
}).ToArray();
|
||||
}
|
||||
|
||||
[Benchmark(Baseline = true)]
|
||||
public void OrderByAge() =>
|
||||
array.OrderBy(x => x.Age).Consume(consumer);
|
||||
|
||||
[Benchmark]
|
||||
public void OrderByAgeOrderBySize() =>
|
||||
array.OrderBy(x => x.Age).OrderBy(x => x.Size).Consume(consumer);
|
||||
|
||||
[Benchmark]
|
||||
public void OrderByAgeThenBySize() =>
|
||||
array.OrderBy(x => x.Age).ThenBy(x => x.Size).Consume(consumer);
|
||||
----
|
||||
|
||||
Hardware configuration:
|
||||
|
||||
[source]
|
||||
----
|
||||
BenchmarkDotNet v0.14.0, Windows 10 (10.0.19045.5247/22H2/2022Update)
|
||||
Intel Core Ultra 7 165H, 1 CPU, 22 logical and 16 physical cores
|
||||
[Host] : .NET Framework 4.8.1 (4.8.9282.0), X64 RyuJIT VectorSize=256
|
||||
.NET 9.0 : .NET 9.0.0 (9.0.24.52809), X64 RyuJIT AVX2
|
||||
.NET Framework 4.8.1 : .NET Framework 4.8.1 (4.8.9282.0), X64 RyuJIT VectorSize=256
|
||||
----
|
||||
|
||||
ifdef::env-github,rspecator-view[]
|
||||
|
||||
@ -56,7 +131,7 @@ I shuffled the text some, [~tamas.vajk]
|
||||
\[~ann.campbell.2] Shouldn't this issue have some performance related label as well?
|
||||
|
||||
|
||||
I simplified the message as the ordering might not happen by some property, but by some complex logic, and in this case we can't display the whole expression and ``++Comparer++`` in the message.
|
||||
I simplified the message as the ordering might not happen by some property, but by some complex logic, and in this case we can't display the whole expression and `Comparer` in the message.
|
||||
|
||||
=== on 1 Jul 2015, 11:26:48 Ann Campbell wrote:
|
||||
added [~tamas.vajk]
|
||||
|
@ -7,8 +7,7 @@
|
||||
"constantCost": "5min"
|
||||
},
|
||||
"tags": [
|
||||
"asp.net",
|
||||
"performance"
|
||||
"asp.net"
|
||||
],
|
||||
"defaultSeverity": "Major",
|
||||
"ruleSpecification": "RSPEC-6961",
|
||||
|
Loading…
x
Reference in New Issue
Block a user