Skip to content
Prev Previous commit
Next Next commit
Address further PR comments
  • Loading branch information
caaavik-msft committed Sep 21, 2023
commit e9d3deb790fe12e1105e3cac2cece56aeac29979
3 changes: 1 addition & 2 deletions src/BenchmarkDotNet/Configs/ConfigExtensions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -111,7 +111,7 @@ public static class ConfigExtensions
[Obsolete("This method will soon be removed, please start using .AddLogicalGroupRules() instead.")]
[EditorBrowsable(EditorBrowsableState.Never)] public static IConfig With(this IConfig config, params BenchmarkLogicalGroupRule[] rules) => config.AddLogicalGroupRules(rules);
[PublicAPI] public static ManualConfig AddLogicalGroupRules(this IConfig config, params BenchmarkLogicalGroupRule[] rules) => config.With(c => c.AddLogicalGroupRules(rules));
[PublicAPI] public static ManualConfig AddEventProcessor(this IConfig config, EventProcessorBase eventProcessor) => config.With(c => c.AddEventProcessor(eventProcessor));
[PublicAPI] public static ManualConfig AddEventProcessor(this IConfig config, EventProcessor eventProcessor) => config.With(c => c.AddEventProcessor(eventProcessor));

[PublicAPI] public static ManualConfig HideColumns(this IConfig config, params string[] columnNames) => config.With(c => c.HideColumns(columnNames));
[PublicAPI] public static ManualConfig HideColumns(this IConfig config, params IColumn[] columns) => config.With(c => c.HideColumns(columns));
Expand All @@ -134,5 +134,4 @@ private static ManualConfig With(this IConfig config, Action<ManualConfig> addAc
addAction(manualConfig);
return manualConfig;
}
}
}
2 changes: 1 addition & 1 deletion src/BenchmarkDotNet/Configs/DebugConfig.cs
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ public abstract class DebugConfig : IConfig
public IEnumerable<IDiagnoser> GetDiagnosers() => Array.Empty<IDiagnoser>();
public IEnumerable<IAnalyser> GetAnalysers() => Array.Empty<IAnalyser>();
public IEnumerable<HardwareCounter> GetHardwareCounters() => Array.Empty<HardwareCounter>();
public IEnumerable<EventProcessorBase> GetEventProcessors() => Array.Empty<EventProcessorBase>();
public IEnumerable<EventProcessor> GetEventProcessors() => Array.Empty<EventProcessor>();
public IEnumerable<IFilter> GetFilters() => Array.Empty<IFilter>();
public IEnumerable<IColumnHidingRule> GetColumnHidingRules() => Array.Empty<IColumnHidingRule>();

Expand Down
2 changes: 1 addition & 1 deletion src/BenchmarkDotNet/Configs/DefaultConfig.cs
Original file line number Diff line number Diff line change
Expand Up @@ -109,7 +109,7 @@ public string ArtifactsPath

public IEnumerable<IFilter> GetFilters() => Array.Empty<IFilter>();

public IEnumerable<EventProcessorBase> GetEventProcessors() => Array.Empty<EventProcessorBase>();
public IEnumerable<EventProcessor> GetEventProcessors() => Array.Empty<EventProcessor>();

public IEnumerable<IColumnHidingRule> GetColumnHidingRules() => Array.Empty<IColumnHidingRule>();
}
Expand Down
2 changes: 1 addition & 1 deletion src/BenchmarkDotNet/Configs/IConfig.cs
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ public interface IConfig
IEnumerable<HardwareCounter> GetHardwareCounters();
IEnumerable<IFilter> GetFilters();
IEnumerable<BenchmarkLogicalGroupRule> GetLogicalGroupRules();
IEnumerable<EventProcessorBase> GetEventProcessors();
IEnumerable<EventProcessor> GetEventProcessors();
IEnumerable<IColumnHidingRule> GetColumnHidingRules();

IOrderer? Orderer { get; }
Expand Down
6 changes: 3 additions & 3 deletions src/BenchmarkDotNet/Configs/ImmutableConfig.cs
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ public sealed class ImmutableConfig : IConfig
private readonly ImmutableHashSet<HardwareCounter> hardwareCounters;
private readonly ImmutableHashSet<IFilter> filters;
private readonly ImmutableArray<BenchmarkLogicalGroupRule> rules;
private readonly ImmutableHashSet<EventProcessorBase> eventProcessors;
private readonly ImmutableHashSet<EventProcessor> eventProcessors;
private readonly ImmutableArray<IColumnHidingRule> columnHidingRules;

internal ImmutableConfig(
Expand All @@ -47,7 +47,7 @@ internal ImmutableConfig(
ImmutableArray<BenchmarkLogicalGroupRule> uniqueRules,
ImmutableArray<IColumnHidingRule> uniqueColumnHidingRules,
ImmutableHashSet<Job> uniqueRunnableJobs,
ImmutableHashSet<EventProcessorBase> uniqueEventProcessors,
ImmutableHashSet<EventProcessor> uniqueEventProcessors,
ConfigUnionRule unionRule,
string artifactsPath,
CultureInfo cultureInfo,
Expand Down Expand Up @@ -100,7 +100,7 @@ internal ImmutableConfig(
public IEnumerable<HardwareCounter> GetHardwareCounters() => hardwareCounters;
public IEnumerable<IFilter> GetFilters() => filters;
public IEnumerable<BenchmarkLogicalGroupRule> GetLogicalGroupRules() => rules;
public IEnumerable<EventProcessorBase> GetEventProcessors() => eventProcessors;
public IEnumerable<EventProcessor> GetEventProcessors() => eventProcessors;
public IEnumerable<IColumnHidingRule> GetColumnHidingRules() => columnHidingRules;

public ILogger GetCompositeLogger() => new CompositeLogger(loggers);
Expand Down
6 changes: 3 additions & 3 deletions src/BenchmarkDotNet/Configs/ManualConfig.cs
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ public class ManualConfig : IConfig
private readonly HashSet<HardwareCounter> hardwareCounters = new HashSet<HardwareCounter>();
private readonly List<IFilter> filters = new List<IFilter>();
private readonly List<BenchmarkLogicalGroupRule> logicalGroupRules = new List<BenchmarkLogicalGroupRule>();
private readonly List<EventProcessorBase> eventProcessors = new List<EventProcessorBase>();
private readonly List<EventProcessor> eventProcessors = new List<EventProcessor>();
private readonly List<IColumnHidingRule> columnHidingRules = new List<IColumnHidingRule>();

public IEnumerable<IColumnProvider> GetColumnProviders() => columnProviders;
Expand All @@ -47,7 +47,7 @@ public class ManualConfig : IConfig
public IEnumerable<HardwareCounter> GetHardwareCounters() => hardwareCounters;
public IEnumerable<IFilter> GetFilters() => filters;
public IEnumerable<BenchmarkLogicalGroupRule> GetLogicalGroupRules() => logicalGroupRules;
public IEnumerable<EventProcessorBase> GetEventProcessors() => eventProcessors;
public IEnumerable<EventProcessor> GetEventProcessors() => eventProcessors;
public IEnumerable<IColumnHidingRule> GetColumnHidingRules() => columnHidingRules;

[PublicAPI] public ConfigOptions Options { get; set; }
Expand Down Expand Up @@ -224,7 +224,7 @@ public ManualConfig AddLogicalGroupRules(params BenchmarkLogicalGroupRule[] rule
return this;
}

public ManualConfig AddEventProcessor(EventProcessorBase eventProcessor)
public ManualConfig AddEventProcessor(EventProcessor eventProcessor)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@adamsitnik, @caaavik-msft

nit: it's unlikely that the users will need more than one instance

While this may be true, this makes it the only one of the Add... methods that doesn't accept a params array. Personally, I would prefer the API consistency.

{
this.eventProcessors.Add(eventProcessor);
return this;
Expand Down
11 changes: 3 additions & 8 deletions src/BenchmarkDotNet/EventProcessors/CompositeEventProcessor.cs
Original file line number Diff line number Diff line change
Expand Up @@ -8,25 +8,20 @@

namespace BenchmarkDotNet.EventProcessors
{
internal sealed class CompositeEventProcessor : EventProcessorBase
internal sealed class CompositeEventProcessor : EventProcessor
{
private readonly IReadOnlyCollection<EventProcessorBase> eventProcessors;
private readonly HashSet<EventProcessor> eventProcessors;

public CompositeEventProcessor(BenchmarkRunInfo[] benchmarkRunInfos)
{
var eventProcessors = new HashSet<EventProcessorBase>();
var eventProcessors = new HashSet<EventProcessor>();

foreach (var info in benchmarkRunInfos)
eventProcessors.AddRange(info.Config.GetEventProcessors());

this.eventProcessors = eventProcessors;
}

public CompositeEventProcessor(IReadOnlyCollection<EventProcessorBase> eventProcessors)
{
this.eventProcessors = eventProcessors;
}

public override void OnStartValidationStage()
{
foreach (var eventProcessor in eventProcessors)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,13 +7,13 @@

namespace BenchmarkDotNet.EventProcessors
{
public abstract class EventProcessorBase
public abstract class EventProcessor
{
public virtual void OnStartValidationStage() { }
public virtual void OnValidationError(ValidationError validationError) { }
public virtual void OnEndValidationStage() { }
public virtual void OnStartBuildStage(IReadOnlyList<BuildPartition> partitions) { }
public virtual void OnBuildComplete(BuildPartition benchmarkCase, BuildResult buildResult) { }
public virtual void OnBuildComplete(BuildPartition partition, BuildResult buildResult) { }
public virtual void OnEndBuildStage() { }
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what is the difference between OnBuildComplete and OnEndBuildStage? The first is for one partition and the latter is for all of them?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, that is the difference between them.

public virtual void OnStartRunStage() { }
public virtual void OnStartRunBenchmarksInType(Type type, IReadOnlyList<BenchmarkCase> benchmarks) { }
Expand Down
4 changes: 2 additions & 2 deletions src/BenchmarkDotNet/Running/BenchmarkRunnerClean.cs
Original file line number Diff line number Diff line change
Expand Up @@ -175,7 +175,7 @@ private static Summary Run(BenchmarkRunInfo benchmarkRunInfo,
Dictionary<BenchmarkCase, (BenchmarkId benchmarkId, BuildResult buildResult)> buildResults,
IResolver resolver,
ILogger logger,
EventProcessorBase eventProcessor,
EventProcessor eventProcessor,
List<string> artifactsToCleanup,
string resultsFolderPath,
string logFilePath,
Expand Down Expand Up @@ -370,7 +370,7 @@ private static ImmutableArray<ValidationError> Validate(params BenchmarkRunInfo[
return validationErrors.ToImmutableArray();
}

private static Dictionary<BuildPartition, BuildResult> BuildInParallel(ILogger logger, string rootArtifactsFolderPath, BuildPartition[] buildPartitions, in StartedClock globalChronometer, EventProcessorBase eventProcessor)
private static Dictionary<BuildPartition, BuildResult> BuildInParallel(ILogger logger, string rootArtifactsFolderPath, BuildPartition[] buildPartitions, in StartedClock globalChronometer, EventProcessor eventProcessor)
{
logger.WriteLineHeader($"// ***** Building {buildPartitions.Length} exe(s) in Parallel: Start *****");

Expand Down
Loading