don't use diagnosers when running the benchmark has failed (#1903)

* add test

* when OverheadJitting succeeds and WorkloadJitting fails, we need to stop the benchmarking

* perform the before parsing the diagnoser stats

* don't try to run the diagnostics if benchmarking has failed

* edge case: the benchmark fails, but not immediately

* don't try to parse to stats if there is nothing to parse

Author: adamsitnik

src/BenchmarkDotNet/Running/BenchmarkRunnerClean.cs

@@ -436,21 +436,32 @@ private static (bool success, List<ExecuteResult> executeResults, GcStats gcStat
                     .Where(r => r.IterationMode != IterationMode.Unknown)
                     .ToArray();
 
-                if (!measurements.Any())
+                if (!measurements.Any(measurement => measurement.IsWorkload()))
                 {
                     // Something went wrong during the benchmark, don't bother doing more runs
                     logger.WriteLineError("No more Benchmark runs will be launched as NO measurements were obtained from the previous run!");
                     success = false;
                     break;
                 }
 
+                if (!success && benchmarkCase.Config.Options.IsSet(ConfigOptions.StopOnFirstError))
+                {
+                    break;
+                }
+
                 if (useDiagnoser)
                 {
                     if (benchmarkCase.Config.HasMemoryDiagnoser())
-                        gcStats = GcStats.Parse(executeResult.Data.Last(line => !string.IsNullOrEmpty(line) && line.StartsWith(GcStats.ResultsLinePrefix)));
+                    {
+                        string resultsLine = executeResult.Data.LastOrDefault(line => !string.IsNullOrEmpty(line) && line.StartsWith(GcStats.ResultsLinePrefix));
+                        gcStats = resultsLine is not null ? GcStats.Parse(resultsLine) : default;
+                    }
 
                     if (benchmarkCase.Config.HasThreadingDiagnoser())
-                        threadingStats = ThreadingStats.Parse(executeResult.Data.Last(line => !string.IsNullOrEmpty(line) && line.StartsWith(ThreadingStats.ResultsLinePrefix)));
+                    {
+                        string resultsLine = executeResult.Data.LastOrDefault(line => !string.IsNullOrEmpty(line) && line.StartsWith(ThreadingStats.ResultsLinePrefix));
+                        threadingStats = resultsLine is not null ? ThreadingStats.Parse(resultsLine) : default;
+                    }
 
                     metrics.AddRange(
                         noOverheadCompositeDiagnoser.ProcessResults(
@@ -465,17 +476,12 @@ private static (bool success, List<ExecuteResult> executeResults, GcStats gcStat
                     double percent = overheadApprox / workloadApprox * 100;
                     launchCount = (int)Math.Round(Math.Max(2, 2 + (percent - 1) / 3)); // an empirical formula
                 }
-
-                if (!success && benchmarkCase.Config.Options.IsSet(ConfigOptions.StopOnFirstError))
-                {
-                    break;
-                }
             }
             logger.WriteLine();
 
             // Do a "Diagnostic" run, but DISCARD the results, so that the overhead of Diagnostics doesn't skew the overall results
             var extraRunCompositeDiagnoser = benchmarkCase.Config.GetCompositeDiagnoser(benchmarkCase, Diagnosers.RunMode.ExtraRun);
-            if (extraRunCompositeDiagnoser != null)
+            if (extraRunCompositeDiagnoser != null && success)
             {
                 logger.WriteLineInfo("// Run, Diagnostic");
 
@@ -499,7 +505,7 @@ private static (bool success, List<ExecuteResult> executeResults, GcStats gcStat
             }
 
             var separateLogicCompositeDiagnoser = benchmarkCase.Config.GetCompositeDiagnoser(benchmarkCase, Diagnosers.RunMode.SeparateLogic);
-            if (separateLogicCompositeDiagnoser != null)
+            if (separateLogicCompositeDiagnoser != null && success)
             {
                 logger.WriteLineInfo("// Run, Diagnostic [SeparateLogic]");
 

tests/BenchmarkDotNet.IntegrationTests/ExceptionHandlingTests.cs

@@ -1,6 +1,8 @@
 using System;
+using System.Linq;
 using BenchmarkDotNet.Attributes;
 using BenchmarkDotNet.Configs;
+using BenchmarkDotNet.Diagnosers;
 using BenchmarkDotNet.Jobs;
 using BenchmarkDotNet.Loggers;
 using BenchmarkDotNet.Toolchains.InProcess.Emit;
@@ -111,5 +113,69 @@ public void Bar1() { }
             [Benchmark]
             public void Bar2() => throw new Exception();
         }
+
+        [Theory]
+        [InlineData(false)]
+        [InlineData(true)]
+        public void StopOnFirstErrorIsRespected(bool value)
+        {
+            var config = ManualConfig.CreateEmpty()
+                .AddJob(Job.Dry)
+                .AddDiagnoser(MemoryDiagnoser.Default) // crucial to repro the bug
+                .WithOption(ConfigOptions.StopOnFirstError, value);
+
+            var summary = CanExecute<MoreThanOneNonThrowingBenchmark>(config, fullValidation: false);
+
+            if (value)
+            {
+                Assert.Equal(1, summary.Reports.Count(report => !report.Success));
+            }
+            else
+            {
+                Assert.Equal(3, summary.Reports.Count(report => report.Success));
+                Assert.Equal(4, summary.Reports.Count(report => !report.Success));
+            }
+        }
+
+        public class MoreThanOneNonThrowingBenchmark
+        {
+            [Benchmark] public void Ok1() { }
+            [Benchmark] public void Throw1() => throw new Exception(BenchmarkExceptionMessage);
+            [Benchmark] public void Ok2() { }
+            [Benchmark] public void Throw2() => throw new Exception(BenchmarkExceptionMessage);
+            [Benchmark] public void Ok3() { }
+            [Benchmark] public void Throw3() => throw new Exception(BenchmarkExceptionMessage);
+            [Benchmark] public void Throw4() => throw new Exception(BenchmarkExceptionMessage);
+        }
+
+        [Fact]
+        public void DiagnosersCantCrashRunnerWhenTheyHaveNothingToParse()
+        {
+            var config = ManualConfig.CreateEmpty()
+                .AddJob(Job.Dry.WithIterationCount(10))
+                .AddDiagnoser(MemoryDiagnoser.Default); // crucial to repro the bug;
+
+            var summary = CanExecute<ThrowButNotImmediately>(config, fullValidation: false);
+
+            var benchmarkReport = summary.Reports.Single();
+            Assert.Equal(default, benchmarkReport.GcStats);
+        }
+
+        public class ThrowButNotImmediately
+        {
+            private int iterationCount;
+
+            [IterationSetup]
+            public void Setup() => ++iterationCount;
+
+            [Benchmark]
+            public void Throwing()
+            {
+                if (iterationCount >= 5)
+                {
+                    throw new Exception();
+                }
+            }
+        }
     }
 }