Respect LogicalGroup order in DefaultOrderer (see #1864) (#1866)

Author: AndreyAkinshin

samples/BenchmarkDotNet.Samples/IntroOrderManual.cs

@@ -21,7 +21,8 @@ private class Config : ManualConfig
 
             private class FastestToSlowestOrderer : IOrderer
             {
-                public IEnumerable<BenchmarkCase> GetExecutionOrder(ImmutableArray<BenchmarkCase> benchmarksCase) =>
+                public IEnumerable<BenchmarkCase> GetExecutionOrder(ImmutableArray<BenchmarkCase> benchmarksCase,
+                    IEnumerable<BenchmarkLogicalGroupRule> order = null) =>
                     from benchmark in benchmarksCase
                     orderby benchmark.Parameters["X"] descending,
                         benchmark.Descriptor.WorkloadMethodDisplayInfo
@@ -37,7 +38,8 @@ orderby summary[benchmark].ResultStatistics.Mean
                 public string GetLogicalGroupKey(ImmutableArray<BenchmarkCase> allBenchmarksCases, BenchmarkCase benchmarkCase) =>
                     benchmarkCase.Job.DisplayInfo + "_" + benchmarkCase.Parameters.DisplayInfo;
 
-                public IEnumerable<IGrouping<string, BenchmarkCase>> GetLogicalGroupOrder(IEnumerable<IGrouping<string, BenchmarkCase>> logicalGroups) =>
+                public IEnumerable<IGrouping<string, BenchmarkCase>> GetLogicalGroupOrder(IEnumerable<IGrouping<string, BenchmarkCase>> logicalGroups,
+                    IEnumerable<BenchmarkLogicalGroupRule> order = null) =>
                     logicalGroups.OrderBy(it => it.Key);
 
                 public bool SeparateLogicalGroups => true;

src/BenchmarkDotNet/Configs/ImmutableConfig.cs

@@ -30,7 +30,7 @@ public sealed class ImmutableConfig : IConfig
         private readonly ImmutableHashSet<Job> jobs;
         private readonly ImmutableHashSet<HardwareCounter> hardwareCounters;
         private readonly ImmutableHashSet<IFilter> filters;
-        private readonly ImmutableHashSet<BenchmarkLogicalGroupRule> rules;
+        private readonly ImmutableArray<BenchmarkLogicalGroupRule> rules;
 
         internal ImmutableConfig(
             ImmutableArray<IColumnProvider> uniqueColumnProviders,
@@ -41,7 +41,7 @@ internal ImmutableConfig(
             ImmutableHashSet<IAnalyser> uniqueAnalyzers,
             ImmutableHashSet<IValidator> uniqueValidators,
             ImmutableHashSet<IFilter> uniqueFilters,
-            ImmutableHashSet<BenchmarkLogicalGroupRule> uniqueRules,
+            ImmutableArray<BenchmarkLogicalGroupRule> uniqueRules,
             ImmutableHashSet<Job> uniqueRunnableJobs,
             ConfigUnionRule unionRule,
             string artifactsPath,

src/BenchmarkDotNet/Configs/ImmutableConfigBuilder.cs

@@ -45,7 +45,7 @@ public static ImmutableConfig Create(IConfig source)
             var uniqueValidators = GetValidators(source.GetValidators(), MandatoryValidators, source.Options);
 
             var uniqueFilters = source.GetFilters().ToImmutableHashSet();
-            var uniqueRules = source.GetLogicalGroupRules().ToImmutableHashSet();
+            var uniqueRules = source.GetLogicalGroupRules().ToImmutableArray();
 
             var uniqueRunnableJobs = GetRunnableJobs(source.GetJobs()).ToImmutableHashSet();
 

src/BenchmarkDotNet/Configs/ManualConfig.cs

@@ -29,7 +29,7 @@ public class ManualConfig : IConfig
         private readonly List<Job> jobs = new List<Job>();
         private readonly HashSet<HardwareCounter> hardwareCounters = new HashSet<HardwareCounter>();
         private readonly List<IFilter> filters = new List<IFilter>();
-        private readonly HashSet<BenchmarkLogicalGroupRule> logicalGroupRules = new HashSet<BenchmarkLogicalGroupRule>();
+        private readonly List<BenchmarkLogicalGroupRule> logicalGroupRules = new List<BenchmarkLogicalGroupRule>();
 
         public IEnumerable<IColumnProvider> GetColumnProviders() => columnProviders;
         public IEnumerable<IExporter> GetExporters() => exporters;
@@ -191,7 +191,12 @@ public ManualConfig AddFilter(params IFilter[] newFilters)
 
         public ManualConfig AddLogicalGroupRules(params BenchmarkLogicalGroupRule[] rules)
         {
-            logicalGroupRules.AddRange(rules);
+            foreach (var rule in rules)
+            {
+                if (logicalGroupRules.Contains(rule))
+                    logicalGroupRules.Remove(rule);
+                logicalGroupRules.Add(rule);
+            }
             return this;
         }
 

src/BenchmarkDotNet/Order/DefaultOrderer.cs

@@ -1,3 +1,4 @@
+using System;
 using System.Collections.Generic;
 using System.Collections.Immutable;
 using System.Diagnostics.CodeAnalysis;
@@ -19,8 +20,7 @@ public class DefaultOrderer : IOrderer
         private readonly IComparer<string[]> categoryComparer = CategoryComparer.Instance;
         private readonly IComparer<ParameterInstances> paramsComparer = ParameterComparer.Instance;
         private readonly IComparer<Job> jobComparer = JobComparer.Instance;
-        private readonly IComparer<BenchmarkCase> benchmarkComparer;
-        private readonly IComparer<IGrouping<string, BenchmarkCase>> logicalGroupComparer;
+        private readonly IComparer<Descriptor> targetComparer;
 
         public SummaryOrderPolicy SummaryOrderPolicy { get; }
         public MethodOrderPolicy MethodOrderPolicy { get; }
@@ -31,14 +31,15 @@ public DefaultOrderer(
         {
             SummaryOrderPolicy = summaryOrderPolicy;
             MethodOrderPolicy = methodOrderPolicy;
-            IComparer<Descriptor> targetComparer = new DescriptorComparer(methodOrderPolicy);
-            benchmarkComparer = new BenchmarkComparer(categoryComparer, paramsComparer, jobComparer, targetComparer);
-            logicalGroupComparer = new LogicalGroupComparer(benchmarkComparer);
+            targetComparer = new DescriptorComparer(methodOrderPolicy);
         }
 
         [PublicAPI]
-        public virtual IEnumerable<BenchmarkCase> GetExecutionOrder(ImmutableArray<BenchmarkCase> benchmarkCases)
+        public virtual IEnumerable<BenchmarkCase> GetExecutionOrder(
+            ImmutableArray<BenchmarkCase> benchmarkCases,
+            IEnumerable<BenchmarkLogicalGroupRule> order = null)
         {
+            var benchmarkComparer = new BenchmarkComparer(categoryComparer, paramsComparer, jobComparer, targetComparer, order);
             var list = benchmarkCases.ToList();
             list.Sort(benchmarkComparer);
             return list;
@@ -47,7 +48,7 @@ public virtual IEnumerable<BenchmarkCase> GetExecutionOrder(ImmutableArray<Bench
         public virtual IEnumerable<BenchmarkCase> GetSummaryOrder(ImmutableArray<BenchmarkCase> benchmarksCases, Summary summary)
         {
             var benchmarkLogicalGroups = benchmarksCases.GroupBy(b => GetLogicalGroupKey(benchmarksCases, b));
-            foreach (var logicalGroup in GetLogicalGroupOrder(benchmarkLogicalGroups))
+            foreach (var logicalGroup in GetLogicalGroupOrder(benchmarkLogicalGroups, benchmarksCases.FirstOrDefault()?.Config.GetLogicalGroupRules()))
             foreach (var benchmark in GetSummaryOrderForGroup(logicalGroup.ToImmutableArray(), summary))
                 yield return benchmark;
         }
@@ -65,7 +66,7 @@ protected virtual IEnumerable<BenchmarkCase> GetSummaryOrderForGroup(ImmutableAr
                 case SummaryOrderPolicy.Declared:
                     return benchmarksCase;
                 default:
-                    return GetExecutionOrder(benchmarksCase);
+                    return GetExecutionOrder(benchmarksCase, benchmarksCase.FirstOrDefault()?.Config.GetLogicalGroupRules());
             }
         }
 
@@ -84,41 +85,62 @@ public string GetHighlightGroupKey(BenchmarkCase benchmarkCase)
 
         public string GetLogicalGroupKey(ImmutableArray<BenchmarkCase> allBenchmarksCases, BenchmarkCase benchmarkCase)
         {
-            var rules = new HashSet<BenchmarkLogicalGroupRule>(benchmarkCase.Config.GetLogicalGroupRules());
+            var explicitRules = benchmarkCase.Config.GetLogicalGroupRules().ToList();
+            var implicitRules = new List<BenchmarkLogicalGroupRule>();
             bool hasJobBaselines = allBenchmarksCases.Any(b => b.Job.Meta.Baseline);
             bool hasDescriptorBaselines = allBenchmarksCases.Any(b => b.Descriptor.Baseline);
             if (hasJobBaselines)
             {
-                rules.Add(BenchmarkLogicalGroupRule.ByMethod);
-                rules.Add(BenchmarkLogicalGroupRule.ByParams);
+                implicitRules.Add(BenchmarkLogicalGroupRule.ByParams);
+                implicitRules.Add(BenchmarkLogicalGroupRule.ByMethod);
             }
             if (hasDescriptorBaselines)
             {
-                rules.Add(BenchmarkLogicalGroupRule.ByJob);
-                rules.Add(BenchmarkLogicalGroupRule.ByParams);
+                implicitRules.Add(BenchmarkLogicalGroupRule.ByParams);
+                implicitRules.Add(BenchmarkLogicalGroupRule.ByJob);
             }
             if (hasJobBaselines && hasDescriptorBaselines)
             {
-                rules.Remove(BenchmarkLogicalGroupRule.ByMethod);
-                rules.Remove(BenchmarkLogicalGroupRule.ByJob);
+                implicitRules.Remove(BenchmarkLogicalGroupRule.ByMethod);
+                implicitRules.Remove(BenchmarkLogicalGroupRule.ByJob);
             }
 
+            var rules = new List<BenchmarkLogicalGroupRule>(explicitRules);
+            foreach (var rule in implicitRules.Where(rule => !rules.Contains(rule)))
+                rules.Add(rule);
+
             var keys = new List<string>();
-            if (rules.Contains(BenchmarkLogicalGroupRule.ByCategory))
-                keys.Add(string.Join(",", benchmarkCase.Descriptor.Categories));
-            if (rules.Contains(BenchmarkLogicalGroupRule.ByMethod))
-                keys.Add(benchmarkCase.Descriptor.DisplayInfo);
-            if (rules.Contains(BenchmarkLogicalGroupRule.ByJob))
-                keys.Add(benchmarkCase.Job.DisplayInfo);
-            if (rules.Contains(BenchmarkLogicalGroupRule.ByParams))
-                keys.Add(benchmarkCase.Parameters.ValueInfo);
+            foreach (var rule in rules)
+            {
+                switch (rule)
+                {
+                    case BenchmarkLogicalGroupRule.ByMethod:
+                        keys.Add(benchmarkCase.Descriptor.DisplayInfo);
+                        break;
+                    case BenchmarkLogicalGroupRule.ByJob:
+                        keys.Add(benchmarkCase.Job.DisplayInfo);
+                        break;
+                    case BenchmarkLogicalGroupRule.ByParams:
+                        keys.Add(benchmarkCase.Parameters.ValueInfo);
+                        break;
+                    case BenchmarkLogicalGroupRule.ByCategory:
+                        keys.Add(string.Join(",", benchmarkCase.Descriptor.Categories));
+                        break;
+                    default:
+                        throw new ArgumentOutOfRangeException(nameof(rule), rule, $"Not supported {nameof(BenchmarkLogicalGroupRule)}");
+                }
+            }
 
             string logicalGroupKey = string.Join("-", keys.Where(key => key != string.Empty));
             return logicalGroupKey == string.Empty ? "*" : logicalGroupKey;
         }
 
-        public virtual IEnumerable<IGrouping<string, BenchmarkCase>> GetLogicalGroupOrder(IEnumerable<IGrouping<string, BenchmarkCase>> logicalGroups)
+        public virtual IEnumerable<IGrouping<string, BenchmarkCase>> GetLogicalGroupOrder(
+            IEnumerable<IGrouping<string, BenchmarkCase>> logicalGroups,
+            IEnumerable<BenchmarkLogicalGroupRule> order = null)
         {
+            var benchmarkComparer = new BenchmarkComparer(categoryComparer, paramsComparer, jobComparer, targetComparer, order);
+            var logicalGroupComparer = new LogicalGroupComparer(benchmarkComparer);
             var list = logicalGroups.ToList();
             list.Sort(logicalGroupComparer);
             return list;
@@ -128,36 +150,58 @@ public virtual IEnumerable<IGrouping<string, BenchmarkCase>> GetLogicalGroupOrde
 
         private class BenchmarkComparer : IComparer<BenchmarkCase>
         {
+            private static readonly BenchmarkLogicalGroupRule[] DefaultOrder =
+            {
+                BenchmarkLogicalGroupRule.ByCategory,
+                BenchmarkLogicalGroupRule.ByParams,
+                BenchmarkLogicalGroupRule.ByJob,
+                BenchmarkLogicalGroupRule.ByMethod
+            };
+
             private readonly IComparer<string[]> categoryComparer;
             private readonly IComparer<ParameterInstances> paramsComparer;
             private readonly IComparer<Job> jobComparer;
             private readonly IComparer<Descriptor> targetComparer;
+            private readonly List<BenchmarkLogicalGroupRule> order;
 
             public BenchmarkComparer(
                 IComparer<string[]> categoryComparer,
                 IComparer<ParameterInstances> paramsComparer,
                 IComparer<Job> jobComparer,
-                IComparer<Descriptor> targetComparer)
+                IComparer<Descriptor> targetComparer,
+                IEnumerable<BenchmarkLogicalGroupRule> order)
             {
                 this.categoryComparer = categoryComparer;
                 this.targetComparer = targetComparer;
                 this.jobComparer = jobComparer;
                 this.paramsComparer = paramsComparer;
+
+                this.order = new List<BenchmarkLogicalGroupRule>();
+                foreach (var rule in (order ?? ImmutableArray<BenchmarkLogicalGroupRule>.Empty).Concat(DefaultOrder))
+                    if (!this.order.Contains(rule))
+                        this.order.Add(rule);
             }
 
             public int Compare(BenchmarkCase x, BenchmarkCase y)
             {
                 if (x == null && y == null) return 0;
                 if (x != null && y == null) return 1;
                 if (x == null) return -1;
-                return new[]
+
+                foreach (var rule in order)
                 {
-                    categoryComparer?.Compare(x.Descriptor.Categories, y.Descriptor.Categories) ?? 0,
-                    paramsComparer?.Compare(x.Parameters, y.Parameters) ?? 0,
-                    jobComparer?.Compare(x.Job, y.Job) ?? 0,
-                    targetComparer?.Compare(x.Descriptor, y.Descriptor) ?? 0,
-                    string.CompareOrdinal(x.DisplayInfo, y.DisplayInfo)
-                }.FirstOrDefault(c => c != 0);
+                    int compare = rule switch
+                    {
+                        BenchmarkLogicalGroupRule.ByMethod => targetComparer?.Compare(x.Descriptor, y.Descriptor) ?? 0,
+                        BenchmarkLogicalGroupRule.ByJob => jobComparer?.Compare(x.Job, y.Job) ?? 0,
+                        BenchmarkLogicalGroupRule.ByParams => paramsComparer?.Compare(x.Parameters, y.Parameters) ?? 0,
+                        BenchmarkLogicalGroupRule.ByCategory => categoryComparer?.Compare(x.Descriptor.Categories, y.Descriptor.Categories) ?? 0,
+                        _ => throw new ArgumentOutOfRangeException()
+                    };
+                    if (compare != 0)
+                        return compare;
+                }
+                return string.CompareOrdinal(x.DisplayInfo, y.DisplayInfo);
             }
         }
 
@@ -176,4 +220,4 @@ public int Compare(IGrouping<string, BenchmarkCase> x, IGrouping<string, Benchma
             }
         }
     }
-}
+}

src/BenchmarkDotNet/Order/IOrderer.cs

@@ -1,6 +1,7 @@
 using System.Collections.Generic;
 using System.Collections.Immutable;
 using System.Linq;
+using BenchmarkDotNet.Configs;
 using BenchmarkDotNet.Reports;
 using BenchmarkDotNet.Running;
 using JetBrains.Annotations;
@@ -10,7 +11,7 @@ namespace BenchmarkDotNet.Order
     public interface IOrderer
     {
         [PublicAPI, NotNull]
-        IEnumerable<BenchmarkCase> GetExecutionOrder(ImmutableArray<BenchmarkCase> benchmarksCase);
+        IEnumerable<BenchmarkCase> GetExecutionOrder(ImmutableArray<BenchmarkCase> benchmarksCase, IEnumerable<BenchmarkLogicalGroupRule> order = null);
 
         [PublicAPI, NotNull]
         IEnumerable<BenchmarkCase> GetSummaryOrder(ImmutableArray<BenchmarkCase> benchmarksCases, [NotNull] Summary summary);
@@ -22,7 +23,8 @@ public interface IOrderer
         string GetLogicalGroupKey(ImmutableArray<BenchmarkCase> allBenchmarksCases, [NotNull] BenchmarkCase benchmarkCase);
 
         [PublicAPI, NotNull]
-        IEnumerable<IGrouping<string, BenchmarkCase>> GetLogicalGroupOrder(IEnumerable<IGrouping<string, BenchmarkCase>> logicalGroups);
+        IEnumerable<IGrouping<string, BenchmarkCase>> GetLogicalGroupOrder(IEnumerable<IGrouping<string, BenchmarkCase>> logicalGroups,
+            IEnumerable<BenchmarkLogicalGroupRule> order = null);
 
         [PublicAPI]
         bool SeparateLogicalGroups { get; }