Don't use ArtifactsPath for wasm and monoaotllvm.

MonoPublisher inherits from DotNetCliBuilderBase.
Don't repeat sequential builds.

Author: timcassell

src/BenchmarkDotNet/Running/BenchmarkRunnerClean.cs

@@ -22,7 +22,6 @@
 using BenchmarkDotNet.Portability;
 using BenchmarkDotNet.Reports;
 using BenchmarkDotNet.Toolchains;
-using BenchmarkDotNet.Toolchains.DotNetCli;
 using BenchmarkDotNet.Toolchains.Parameters;
 using BenchmarkDotNet.Toolchains.Results;
 using BenchmarkDotNet.Validators;
@@ -362,22 +361,17 @@ private static ImmutableArray<ValidationError> Validate(params BenchmarkRunInfo[
 
         private static Dictionary<BuildPartition, BuildResult> BuildInParallel(ILogger logger, string rootArtifactsFolderPath, BuildPartition[] buildPartitions, in StartedClock globalChronometer, EventProcessor eventProcessor)
         {
-            var parallelPartitions = new List<BuildPartition[]>();
-            var sequentialPartitions = new List<BuildPartition>();
+            var parallelPartitions = new List<(BuildPartition buildPartition, IToolchain toolchain, bool isSequential)[]>();
+            var sequentialPartitions = new List<(BuildPartition, IToolchain, bool)>();
             foreach (var partition in buildPartitions)
             {
-                // If dotnet sdk does not support ArtifactsPath, it's unsafe to build the same project in parallel.
-                // We cannot rely on falling back to sequential after failure, because the builds can be corrupted.
-                // Therefore we have to build them sequentially. #2425
-                if (partition.RepresentativeBenchmarkCase.GetToolchain().Builder is DotNetCliBuilderBase dotnetBuilder
-                    && !DotNetCliCommandExecutor.DotNetSdkSupportsArtifactsPath(dotnetBuilder.CustomDotNetCliPath))
-                {
-                    sequentialPartitions.Add(partition);
-                }
+                // It's guaranteed that all the benchmarks in single partition have same toolchain.
+                var toolchain = partition.RepresentativeBenchmarkCase.GetToolchain();
+                // #2425
+                if (toolchain.IsSafeToBuildInParallel())
+                    parallelPartitions.Add([(partition, toolchain, false)]);
                 else
-                {
-                    parallelPartitions.Add([partition]);
-                }
+                    sequentialPartitions.Add((partition, toolchain, true));
             }
             if (sequentialPartitions.Count > 0)
             {
@@ -392,37 +386,43 @@ private static Dictionary<BuildPartition, BuildResult> BuildInParallel(ILogger l
 
             var beforeParallelBuild = globalChronometer.GetElapsed();
 
-            var buildResults = parallelPartitions
+            var parallelBuildResults = parallelPartitions
                 .AsParallel()
                 .SelectMany(partitions => partitions
-                    .Select(buildPartition => (Partition: buildPartition, Result: Build(buildPartition, rootArtifactsFolderPath, buildLogger))))
+                    .Select(build => (Partition: build.buildPartition,
+                        Result: Build(build.buildPartition, build.toolchain, rootArtifactsFolderPath, buildLogger),
+                        WasSequential: build.isSequential)
+                    )
+                )
                 .AsSequential() // Ensure that build completion events are processed sequentially
                 .Select(build =>
                 {
                     // If the generation was successful, but the build was not, we will try building sequentially
                     // so don't send the OnBuildComplete event yet.
-                    if (buildPartitions.Length <= 1 || !build.Result.IsGenerateSuccess || build.Result.IsBuildSuccess)
+                    if (build.WasSequential || buildPartitions.Length <= 1 || !build.Result.IsGenerateSuccess || build.Result.IsBuildSuccess)
                         eventProcessor.OnBuildComplete(build.Partition, build.Result);
 
                     return build;
                 })
-                .ToDictionary(build => build.Partition, build => build.Result);
+                .ToArray();
 
             var afterParallelBuild = globalChronometer.GetElapsed();
 
             logger.WriteLineHeader($"// ***** Done, took {GetFormattedDifference(beforeParallelBuild, afterParallelBuild)}   *****");
 
-            if (buildPartitions.Length <= 1 || !buildResults.Values.Any(result => result.IsGenerateSuccess && !result.IsBuildSuccess))
+            var buildResults = parallelBuildResults.ToDictionary(build => build.Partition, build => build.Result);
+
+            if (parallelPartitions.Count <= 1 || !parallelBuildResults.Any(build => !build.WasSequential && build.Result.IsGenerateSuccess && !build.Result.IsBuildSuccess))
                 return buildResults;
 
             logger.WriteLineHeader("// ***** Failed to build in Parallel, switching to sequential build   *****");
 
-            foreach (var buildPartition in buildPartitions)
+            foreach (var (buildPartition, toolchain, _) in parallelPartitions.Skip(1).Select(arr => arr[0]))
             {
                 if (buildResults[buildPartition].IsGenerateSuccess && !buildResults[buildPartition].IsBuildSuccess)
                 {
                     if (!buildResults[buildPartition].TryToExplainFailureReason(out string _))
-                        buildResults[buildPartition] = Build(buildPartition, rootArtifactsFolderPath, buildLogger);
+                        buildResults[buildPartition] = Build(buildPartition, toolchain, rootArtifactsFolderPath, buildLogger);
 
                     eventProcessor.OnBuildComplete(buildPartition, buildResults[buildPartition]);
                 }
@@ -438,10 +438,8 @@ static string GetFormattedDifference(ClockSpan before, ClockSpan after)
                 => (after.GetTimeSpan() - before.GetTimeSpan()).ToFormattedTotalTime(DefaultCultureInfo.Instance);
         }
 
-        private static BuildResult Build(BuildPartition buildPartition, string rootArtifactsFolderPath, ILogger buildLogger)
+        private static BuildResult Build(BuildPartition buildPartition, IToolchain toolchain, string rootArtifactsFolderPath, ILogger buildLogger)
         {
-            var toolchain = buildPartition.RepresentativeBenchmarkCase.GetToolchain(); // it's guaranteed that all the benchmarks in single partition have same toolchain
-
             var generateResult = toolchain.Generator.GenerateProject(buildPartition, buildLogger, rootArtifactsFolderPath);
 
             try

src/BenchmarkDotNet/Toolchains/CsProj/CsProjGenerator.cs

@@ -40,8 +40,8 @@ public class CsProjGenerator : DotNetCliGenerator, IEquatable<CsProjGenerator>
 
         public string RuntimeFrameworkVersion { get; }
 
-        public CsProjGenerator(string targetFrameworkMoniker, string cliPath, string packagesPath, string runtimeFrameworkVersion, bool isNetCore = true)
-            : base(targetFrameworkMoniker, cliPath, packagesPath, isNetCore)
+        public CsProjGenerator(string targetFrameworkMoniker, string cliPath, string packagesPath, string runtimeFrameworkVersion, bool isNetCore = true, bool useArtifactsPathIfSupported = true)
+            : base(targetFrameworkMoniker, cliPath, packagesPath, isNetCore, useArtifactsPathIfSupported)
         {
             RuntimeFrameworkVersion = runtimeFrameworkVersion;
         }

src/BenchmarkDotNet/Toolchains/DotNetCli/DotNetCliBuilder.cs

@@ -10,6 +10,7 @@ namespace BenchmarkDotNet.Toolchains.DotNetCli
     public abstract class DotNetCliBuilderBase : IBuilder
     {
         public string? CustomDotNetCliPath { get; protected set; }
+        internal bool UseArtifactsPathIfSupported { get; private protected set; } = true;
         public abstract BuildResult Build(GenerateResult generateResult, BuildPartition buildPartition, ILogger logger);
     }
 
@@ -25,6 +26,13 @@ public DotNetCliBuilder(string targetFrameworkMoniker, string? customDotNetCliPa
             LogOutput = logOutput;
         }
 
+        internal DotNetCliBuilder(string? customDotNetCliPath, bool logOutput, bool useArtifactsPathIfSupported)
+        {
+            CustomDotNetCliPath = customDotNetCliPath;
+            LogOutput = logOutput;
+            UseArtifactsPathIfSupported = useArtifactsPathIfSupported;
+        }
+
         public override BuildResult Build(GenerateResult generateResult, BuildPartition buildPartition, ILogger logger)
         {
             BuildResult buildResult = new DotNetCliCommand(
@@ -36,7 +44,7 @@ public override BuildResult Build(GenerateResult generateResult, BuildPartition
                     Array.Empty<EnvironmentVariable>(),
                     buildPartition.Timeout,
                     logOutput: LogOutput)
-                .RestoreThenBuild();
+                .RestoreThenBuild(UseArtifactsPathIfSupported);
             if (buildResult.IsBuildSuccess &&
                 buildPartition.RepresentativeBenchmarkCase.Job.Environment.LargeAddressAware)
             {

src/BenchmarkDotNet/Toolchains/DotNetCli/DotNetCliCommand.cs

@@ -53,7 +53,7 @@ public DotNetCliCommand WithCliPath(string cliPath)
             => new (cliPath, Arguments, GenerateResult, Logger, BuildPartition, EnvironmentVariables, Timeout, logOutput: LogOutput);
 
         [PublicAPI]
-        public BuildResult RestoreThenBuild()
+        public BuildResult RestoreThenBuild(bool useArtifactsPathIfSupported = true)
         {
             DotNetCliCommandExecutor.LogEnvVars(WithArguments(null));
 
@@ -65,35 +65,35 @@ public BuildResult RestoreThenBuild()
             // so when users go with custom build configuration, we must perform full build
             // which will internally restore for the right configuration
             if (BuildPartition.IsCustomBuildConfiguration)
-                return Build().ToBuildResult(GenerateResult);
+                return Build(useArtifactsPathIfSupported).ToBuildResult(GenerateResult);
 
             // On our CI, Integration tests take too much time, because each benchmark run rebuilds BenchmarkDotNet itself.
             // To reduce the total duration of the CI workflows, we build all the projects without dependencies
             if (BuildPartition.ForcedNoDependenciesForIntegrationTests)
             {
                 var restoreResult = DotNetCliCommandExecutor.Execute(WithArguments(
-                    GetRestoreCommand(GenerateResult.ArtifactsPaths, BuildPartition, GetWithArtifactsPath(), $"{Arguments} --no-dependencies", "restore-no-deps", excludeOutput: true)));
+                    GetRestoreCommand(GenerateResult.ArtifactsPaths, BuildPartition, GetWithArtifactsPath(useArtifactsPathIfSupported), $"{Arguments} --no-dependencies", "restore-no-deps", excludeOutput: true)));
                 if (!restoreResult.IsSuccess)
                     return BuildResult.Failure(GenerateResult, restoreResult.AllInformation);
 
                 return DotNetCliCommandExecutor.Execute(WithArguments(
-                    GetBuildCommand(GenerateResult.ArtifactsPaths, BuildPartition, GetWithArtifactsPath(), $"{Arguments} --no-restore --no-dependencies", "build-no-restore-no-deps", excludeOutput: true)))
+                    GetBuildCommand(GenerateResult.ArtifactsPaths, BuildPartition, GetWithArtifactsPath(useArtifactsPathIfSupported), $"{Arguments} --no-restore --no-dependencies", "build-no-restore-no-deps", excludeOutput: true)))
                     .ToBuildResult(GenerateResult);
             }
             else
             {
-                var restoreResult = Restore();
+                var restoreResult = Restore(useArtifactsPathIfSupported);
                 if (!restoreResult.IsSuccess)
                     return BuildResult.Failure(GenerateResult, restoreResult.AllInformation);
 
                 // We no longer retry with --no-dependencies, because it fails with --output set at the same time,
                 // and the artifactsPaths.BinariesDirectoryPath is set before we try to build, so we cannot overwrite it.
-                return BuildNoRestore().ToBuildResult(GenerateResult);
+                return BuildNoRestore(useArtifactsPathIfSupported).ToBuildResult(GenerateResult);
             }
         }
 
         [PublicAPI]
-        public BuildResult RestoreThenBuildThenPublish()
+        public BuildResult RestoreThenBuildThenPublish(bool useArtifactsPathIfSupported = true)
         {
             DotNetCliCommandExecutor.LogEnvVars(WithArguments(null));
 
@@ -105,14 +105,14 @@ public BuildResult RestoreThenBuildThenPublish()
             // so when users go with custom build configuration, we must perform full publish
             // which will internally restore and build for the right configuration
             if (BuildPartition.IsCustomBuildConfiguration)
-                return Publish().ToBuildResult(GenerateResult);
+                return Publish(useArtifactsPathIfSupported).ToBuildResult(GenerateResult);
 
-            var restoreResult = Restore();
+            var restoreResult = Restore(useArtifactsPathIfSupported);
             if (!restoreResult.IsSuccess)
                 return BuildResult.Failure(GenerateResult, restoreResult.AllInformation);
 
             // We use the implicit build in the publish command. We stopped doing a separate build step because we set the --output.
-            return PublishNoRestore().ToBuildResult(GenerateResult);
+            return PublishNoRestore(useArtifactsPathIfSupported).ToBuildResult(GenerateResult);
         }
 
         public DotNetCliCommandResult AddPackages()
@@ -129,28 +129,28 @@ public DotNetCliCommandResult AddPackages()
             return DotNetCliCommandResult.Success(executionTime, stdOutput.ToString());
         }
 
-        private bool GetWithArtifactsPath() => DotNetCliCommandExecutor.DotNetSdkSupportsArtifactsPath(CliPath);
+        private bool GetWithArtifactsPath(bool useArtifactsPathIfSupported) => useArtifactsPathIfSupported && DotNetCliCommandExecutor.DotNetSdkSupportsArtifactsPath(CliPath);
 
-        public DotNetCliCommandResult Restore()
+        public DotNetCliCommandResult Restore(bool useArtifactsPathIfSupported = true)
             => DotNetCliCommandExecutor.Execute(WithArguments(
-                GetRestoreCommand(GenerateResult.ArtifactsPaths, BuildPartition, GetWithArtifactsPath(), Arguments, "restore")));
+                GetRestoreCommand(GenerateResult.ArtifactsPaths, BuildPartition, GetWithArtifactsPath(useArtifactsPathIfSupported), Arguments, "restore")));
 
-        public DotNetCliCommandResult Build()
+        public DotNetCliCommandResult Build(bool useArtifactsPathIfSupported = true)
             => DotNetCliCommandExecutor.Execute(WithArguments(
-                GetBuildCommand(GenerateResult.ArtifactsPaths, BuildPartition, GetWithArtifactsPath(), Arguments, "build")));
+                GetBuildCommand(GenerateResult.ArtifactsPaths, BuildPartition, GetWithArtifactsPath(useArtifactsPathIfSupported), Arguments, "build")));
 
-        public DotNetCliCommandResult BuildNoRestore()
+        public DotNetCliCommandResult BuildNoRestore(bool useArtifactsPathIfSupported = true)
             => DotNetCliCommandExecutor.Execute(WithArguments(
-                GetBuildCommand(GenerateResult.ArtifactsPaths, BuildPartition, GetWithArtifactsPath(), $"{Arguments} --no-restore", "build-no-restore")));
+                GetBuildCommand(GenerateResult.ArtifactsPaths, BuildPartition, GetWithArtifactsPath(useArtifactsPathIfSupported), $"{Arguments} --no-restore", "build-no-restore")));
 
-        public DotNetCliCommandResult Publish()
+        public DotNetCliCommandResult Publish(bool useArtifactsPathIfSupported = true)
             => DotNetCliCommandExecutor.Execute(WithArguments(
-                GetPublishCommand(GenerateResult.ArtifactsPaths, BuildPartition, GetWithArtifactsPath(), Arguments, "publish")));
+                GetPublishCommand(GenerateResult.ArtifactsPaths, BuildPartition, GetWithArtifactsPath(useArtifactsPathIfSupported), Arguments, "publish")));
 
         // PublishNoBuildAndNoRestore was removed because we set --output in the build step. We use the implicit build included in the publish command.
-        public DotNetCliCommandResult PublishNoRestore()
+        public DotNetCliCommandResult PublishNoRestore(bool useArtifactsPathIfSupported = true)
             => DotNetCliCommandExecutor.Execute(WithArguments(
-                GetPublishCommand(GenerateResult.ArtifactsPaths, BuildPartition, GetWithArtifactsPath(), $"{Arguments} --no-restore", "publish-no-restore")));
+                GetPublishCommand(GenerateResult.ArtifactsPaths, BuildPartition, GetWithArtifactsPath(useArtifactsPathIfSupported), $"{Arguments} --no-restore", "publish-no-restore")));
 
         internal static IEnumerable<string> GetAddPackagesCommands(BuildPartition buildPartition)
             => GetNuGetAddPackageCommands(buildPartition.RepresentativeBenchmarkCase, buildPartition.Resolver);

src/BenchmarkDotNet/Toolchains/DotNetCli/DotNetCliGenerator.cs

@@ -20,13 +20,16 @@ public abstract class DotNetCliGenerator : GeneratorBase
 
         protected bool IsNetCore { get; }
 
+        protected bool UseArtifactsPathIfSupported { get; set; }
+
         [PublicAPI]
-        protected DotNetCliGenerator(string targetFrameworkMoniker, string cliPath, string packagesPath, bool isNetCore)
+        protected DotNetCliGenerator(string targetFrameworkMoniker, string cliPath, string packagesPath, bool isNetCore, bool useArtifactsPathIfSupported = true)
         {
             TargetFrameworkMoniker = targetFrameworkMoniker;
             CliPath = cliPath;
             PackagesPath = packagesPath;
             IsNetCore = isNetCore;
+            UseArtifactsPathIfSupported = useArtifactsPathIfSupported;
         }
 
         protected override string GetExecutableExtension() => IsNetCore ? ".dll" : ".exe";
@@ -100,9 +103,10 @@ protected override void CopyAllRequiredFiles(ArtifactsPaths artifactsPaths)
 
         protected override void GenerateBuildScript(BuildPartition buildPartition, ArtifactsPaths artifactsPaths)
         {
+            bool useArtifactsPath = UseArtifactsPathIfSupported && DotNetCliCommandExecutor.DotNetSdkSupportsArtifactsPath(CliPath);
             var content = new StringBuilder(300)
-                .AppendLine($"call {CliPath ?? "dotnet"} {DotNetCliCommand.GetRestoreCommand(artifactsPaths, buildPartition, DotNetCliCommandExecutor.DotNetSdkSupportsArtifactsPath(CliPath))}")
-                .AppendLine($"call {CliPath ?? "dotnet"} {DotNetCliCommand.GetBuildCommand(artifactsPaths, buildPartition, DotNetCliCommandExecutor.DotNetSdkSupportsArtifactsPath(CliPath))}")
+                .AppendLine($"call {CliPath ?? "dotnet"} {DotNetCliCommand.GetRestoreCommand(artifactsPaths, buildPartition, useArtifactsPath)}")
+                .AppendLine($"call {CliPath ?? "dotnet"} {DotNetCliCommand.GetBuildCommand(artifactsPaths, buildPartition, useArtifactsPath)}")
                 .ToString();
 
             File.WriteAllText(artifactsPaths.BuildScriptFilePath, content);

src/BenchmarkDotNet/Toolchains/DotNetCli/DotNetCliPublisher.cs

@@ -31,6 +31,6 @@ public override BuildResult Build(GenerateResult generateResult, BuildPartition
                     buildPartition,
                     EnvironmentVariables,
                     buildPartition.Timeout)
-                .RestoreThenBuildThenPublish();
+                .RestoreThenBuildThenPublish(UseArtifactsPathIfSupported);
     }
 }

src/BenchmarkDotNet/Toolchains/Mono/MonoPublisher.cs

@@ -7,7 +7,7 @@
 
 namespace BenchmarkDotNet.Toolchains.Mono
 {
-    public class MonoPublisher : IBuilder
+    public class MonoPublisher : DotNetCliBuilderBase
     {
         public MonoPublisher(string customDotNetCliPath)
         {
@@ -18,13 +18,11 @@ public MonoPublisher(string customDotNetCliPath)
             ExtraArguments = $"--self-contained -r {runtimeIdentifier} /p:UseMonoRuntime=true /p:RuntimeIdentifiers={runtimeIdentifier}";
         }
 
-        private string CustomDotNetCliPath { get; }
-
         private string ExtraArguments { get; }
 
         private IReadOnlyList<EnvironmentVariable> EnvironmentVariables { get; }
 
-        public BuildResult Build(GenerateResult generateResult, BuildPartition buildPartition, ILogger logger)
+        public override BuildResult Build(GenerateResult generateResult, BuildPartition buildPartition, ILogger logger)
             => new DotNetCliCommand(
                     CustomDotNetCliPath,
                     ExtraArguments,
@@ -33,6 +31,6 @@ public BuildResult Build(GenerateResult generateResult, BuildPartition buildPart
                     buildPartition,
                     EnvironmentVariables,
                     buildPartition.Timeout)
-                .Publish().ToBuildResult(generateResult);
+                .Publish(UseArtifactsPathIfSupported).ToBuildResult(generateResult);
     }
 }