diff --git a/rules/S3169/csharp/rule.adoc b/rules/S3169/csharp/rule.adoc index b2e9860eb1d..df9d2c517f5 100644 --- a/rules/S3169/csharp/rule.adoc +++ b/rules/S3169/csharp/rule.adoc @@ -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] diff --git a/rules/S6961/csharp/metadata.json b/rules/S6961/csharp/metadata.json index 7116988462a..f3d4d9be3d7 100644 --- a/rules/S6961/csharp/metadata.json +++ b/rules/S6961/csharp/metadata.json @@ -7,8 +7,7 @@ "constantCost": "5min" }, "tags": [ - "asp.net", - "performance" + "asp.net" ], "defaultSeverity": "Major", "ruleSpecification": "RSPEC-6961",