From 286de66c8485297740a6d7c14aa58d8f7b1a4576 Mon Sep 17 00:00:00 2001 From: Tim Date: Wed, 17 Apr 2024 01:10:27 -0400 Subject: [PATCH] Don't use ArtifactsPath for wasm and monoaotllvm. MonoPublisher inherits from DotNetCliBuilderBase. Don't repeat sequential builds. --- .../Running/BenchmarkRunnerClean.cs | 46 +++++++++---------- .../Toolchains/CsProj/CsProjGenerator.cs | 4 +- .../Toolchains/DotNetCli/DotNetCliBuilder.cs | 10 +++- .../Toolchains/DotNetCli/DotNetCliCommand.cs | 42 ++++++++--------- .../DotNetCli/DotNetCliGenerator.cs | 10 ++-- .../DotNetCli/DotNetCliPublisher.cs | 3 +- .../Toolchains/Mono/MonoPublisher.cs | 9 ++-- .../MonoAotLLVM/MonoAotLLVMGenerator.cs | 3 +- .../MonoAotLLVM/MonoAotLLVMToolChain.cs | 4 +- .../Toolchains/MonoWasm/WasmGenerator.cs | 3 +- .../Toolchains/MonoWasm/WasmToolchain.cs | 7 +-- .../Toolchains/NativeAot/Generator.cs | 5 +- .../Toolchains/ToolchainExtensions.cs | 8 ++++ 13 files changed, 88 insertions(+), 66 deletions(-) diff --git a/src/BenchmarkDotNet/Running/BenchmarkRunnerClean.cs b/src/BenchmarkDotNet/Running/BenchmarkRunnerClean.cs index 2a1f64c37c..ac6708b6a3 100644 --- a/src/BenchmarkDotNet/Running/BenchmarkRunnerClean.cs +++ b/src/BenchmarkDotNet/Running/BenchmarkRunnerClean.cs @@ -21,7 +21,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; @@ -373,22 +372,17 @@ private static ImmutableArray Validate(params BenchmarkRunInfo[ private static Dictionary BuildInParallel(ILogger logger, string rootArtifactsFolderPath, BuildPartition[] buildPartitions, in StartedClock globalChronometer, EventProcessor eventProcessor) { - var parallelPartitions = new List(); - var sequentialPartitions = new List(); + 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) { @@ -403,37 +397,43 @@ private static ImmutableArray Validate(params BenchmarkRunInfo[ 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]); } @@ -449,10 +449,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 diff --git a/src/BenchmarkDotNet/Toolchains/CsProj/CsProjGenerator.cs b/src/BenchmarkDotNet/Toolchains/CsProj/CsProjGenerator.cs index 7e91c36ff4..227967989e 100644 --- a/src/BenchmarkDotNet/Toolchains/CsProj/CsProjGenerator.cs +++ b/src/BenchmarkDotNet/Toolchains/CsProj/CsProjGenerator.cs @@ -40,8 +40,8 @@ public class CsProjGenerator : DotNetCliGenerator, IEquatable 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; } diff --git a/src/BenchmarkDotNet/Toolchains/DotNetCli/DotNetCliBuilder.cs b/src/BenchmarkDotNet/Toolchains/DotNetCli/DotNetCliBuilder.cs index 2ad11b4e5d..0d459593ec 100644 --- a/src/BenchmarkDotNet/Toolchains/DotNetCli/DotNetCliBuilder.cs +++ b/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; } 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(), buildPartition.Timeout, logOutput: LogOutput) - .RestoreThenBuild(); + .RestoreThenBuild(UseArtifactsPathIfSupported); if (buildResult.IsBuildSuccess && buildPartition.RepresentativeBenchmarkCase.Job.Environment.LargeAddressAware) { diff --git a/src/BenchmarkDotNet/Toolchains/DotNetCli/DotNetCliCommand.cs b/src/BenchmarkDotNet/Toolchains/DotNetCli/DotNetCliCommand.cs index ae14512e9d..b41925e6d5 100644 --- a/src/BenchmarkDotNet/Toolchains/DotNetCli/DotNetCliCommand.cs +++ b/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 GetAddPackagesCommands(BuildPartition buildPartition) => GetNuGetAddPackageCommands(buildPartition.RepresentativeBenchmarkCase, buildPartition.Resolver); diff --git a/src/BenchmarkDotNet/Toolchains/DotNetCli/DotNetCliGenerator.cs b/src/BenchmarkDotNet/Toolchains/DotNetCli/DotNetCliGenerator.cs index dd2663f664..f953a27729 100644 --- a/src/BenchmarkDotNet/Toolchains/DotNetCli/DotNetCliGenerator.cs +++ b/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); diff --git a/src/BenchmarkDotNet/Toolchains/DotNetCli/DotNetCliPublisher.cs b/src/BenchmarkDotNet/Toolchains/DotNetCli/DotNetCliPublisher.cs index 4056dbf14e..24afeaa4d1 100644 --- a/src/BenchmarkDotNet/Toolchains/DotNetCli/DotNetCliPublisher.cs +++ b/src/BenchmarkDotNet/Toolchains/DotNetCli/DotNetCliPublisher.cs @@ -16,6 +16,7 @@ public class DotNetCliPublisher : DotNetCliBuilderBase CustomDotNetCliPath = customDotNetCliPath; ExtraArguments = extraArguments; EnvironmentVariables = environmentVariables; + UseArtifactsPathIfSupported = true; } private string? ExtraArguments { get; } @@ -31,6 +32,6 @@ public override BuildResult Build(GenerateResult generateResult, BuildPartition buildPartition, EnvironmentVariables, buildPartition.Timeout) - .RestoreThenBuildThenPublish(); + .RestoreThenBuildThenPublish(UseArtifactsPathIfSupported); } } diff --git a/src/BenchmarkDotNet/Toolchains/Mono/MonoPublisher.cs b/src/BenchmarkDotNet/Toolchains/Mono/MonoPublisher.cs index ff98540cbf..93bfdebec4 100644 --- a/src/BenchmarkDotNet/Toolchains/Mono/MonoPublisher.cs +++ b/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) { @@ -16,15 +16,14 @@ public MonoPublisher(string customDotNetCliPath) // /p:RuntimeIdentifiers is set explicitly here because --self-contained requires it, see https://github.com/dotnet/sdk/issues/10566 ExtraArguments = $"--self-contained -r {runtimeIdentifier} /p:UseMonoRuntime=true /p:RuntimeIdentifiers={runtimeIdentifier}"; + UseArtifactsPathIfSupported = true; } - private string CustomDotNetCliPath { get; } - private string ExtraArguments { get; } private IReadOnlyList 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 +32,6 @@ public BuildResult Build(GenerateResult generateResult, BuildPartition buildPart buildPartition, EnvironmentVariables, buildPartition.Timeout) - .Publish().ToBuildResult(generateResult); + .Publish(UseArtifactsPathIfSupported).ToBuildResult(generateResult); } } diff --git a/src/BenchmarkDotNet/Toolchains/MonoAotLLVM/MonoAotLLVMGenerator.cs b/src/BenchmarkDotNet/Toolchains/MonoAotLLVM/MonoAotLLVMGenerator.cs index 833c113d84..05f18bff18 100644 --- a/src/BenchmarkDotNet/Toolchains/MonoAotLLVM/MonoAotLLVMGenerator.cs +++ b/src/BenchmarkDotNet/Toolchains/MonoAotLLVM/MonoAotLLVMGenerator.cs @@ -17,7 +17,8 @@ public class MonoAotLLVMGenerator : CsProjGenerator private readonly MonoAotCompilerMode AotCompilerMode; public MonoAotLLVMGenerator(string targetFrameworkMoniker, string cliPath, string packagesPath, string customRuntimePack, string aotCompilerPath, MonoAotCompilerMode aotCompilerMode) - : base(targetFrameworkMoniker, cliPath, packagesPath, runtimeFrameworkVersion: null) + // We have no tests for this toolchain, so not including ArtifactsPath in case it fails the same as wasm. + : base(targetFrameworkMoniker, cliPath, packagesPath, runtimeFrameworkVersion: null, useArtifactsPathIfSupported: false) { CustomRuntimePack = customRuntimePack; AotCompilerPath = aotCompilerPath; diff --git a/src/BenchmarkDotNet/Toolchains/MonoAotLLVM/MonoAotLLVMToolChain.cs b/src/BenchmarkDotNet/Toolchains/MonoAotLLVM/MonoAotLLVMToolChain.cs index 06ef8ea8c6..0526971114 100644 --- a/src/BenchmarkDotNet/Toolchains/MonoAotLLVM/MonoAotLLVMToolChain.cs +++ b/src/BenchmarkDotNet/Toolchains/MonoAotLLVM/MonoAotLLVMToolChain.cs @@ -24,8 +24,8 @@ public static IToolchain From(NetCoreAppSettings netCoreAppSettings) netCoreAppSettings.CustomRuntimePack, netCoreAppSettings.AOTCompilerPath, netCoreAppSettings.AOTCompilerMode), - new DotNetCliBuilder(netCoreAppSettings.TargetFrameworkMoniker, - netCoreAppSettings.CustomDotNetCliPath), + // We have no tests for this toolchain, so not including ArtifactsPath in case it fails the same as wasm. + new DotNetCliBuilder(netCoreAppSettings.CustomDotNetCliPath, logOutput: false, useArtifactsPathIfSupported: false), new Executor(), netCoreAppSettings.CustomDotNetCliPath); diff --git a/src/BenchmarkDotNet/Toolchains/MonoWasm/WasmGenerator.cs b/src/BenchmarkDotNet/Toolchains/MonoWasm/WasmGenerator.cs index 7c9aa8826f..6e4892de78 100644 --- a/src/BenchmarkDotNet/Toolchains/MonoWasm/WasmGenerator.cs +++ b/src/BenchmarkDotNet/Toolchains/MonoWasm/WasmGenerator.cs @@ -17,7 +17,8 @@ public class WasmGenerator : CsProjGenerator private readonly string MainJS; public WasmGenerator(string targetFrameworkMoniker, string cliPath, string packagesPath, string customRuntimePack, bool aot) - : base(targetFrameworkMoniker, cliPath, packagesPath, runtimeFrameworkVersion: null) + // Building wasm with ArtifactsPath set fails for some reason (haven't figured out why yet). + : base(targetFrameworkMoniker, cliPath, packagesPath, runtimeFrameworkVersion: null, useArtifactsPathIfSupported: false) { Aot = aot; CustomRuntimePack = customRuntimePack; diff --git a/src/BenchmarkDotNet/Toolchains/MonoWasm/WasmToolchain.cs b/src/BenchmarkDotNet/Toolchains/MonoWasm/WasmToolchain.cs index 42e5c428d2..ba2f53dd15 100644 --- a/src/BenchmarkDotNet/Toolchains/MonoWasm/WasmToolchain.cs +++ b/src/BenchmarkDotNet/Toolchains/MonoWasm/WasmToolchain.cs @@ -47,10 +47,11 @@ public static IToolchain From(NetCoreAppSettings netCoreAppSettings) netCoreAppSettings.PackagesPath, netCoreAppSettings.CustomRuntimePack, netCoreAppSettings.AOTCompilerMode == MonoAotLLVM.MonoAotCompilerMode.wasm), - new DotNetCliBuilder(netCoreAppSettings.TargetFrameworkMoniker, - netCoreAppSettings.CustomDotNetCliPath, + new DotNetCliBuilder(netCoreAppSettings.CustomDotNetCliPath, // aot builds can be very slow - logOutput: netCoreAppSettings.AOTCompilerMode == MonoAotLLVM.MonoAotCompilerMode.wasm), + logOutput: netCoreAppSettings.AOTCompilerMode == MonoAotLLVM.MonoAotCompilerMode.wasm, + // Building wasm with ArtifactsPath set fails for some reason (haven't figured out why yet). + useArtifactsPathIfSupported: false), new WasmExecutor(), netCoreAppSettings.CustomDotNetCliPath); } diff --git a/src/BenchmarkDotNet/Toolchains/NativeAot/Generator.cs b/src/BenchmarkDotNet/Toolchains/NativeAot/Generator.cs index 2d1627ef04..86cb4a2fbd 100644 --- a/src/BenchmarkDotNet/Toolchains/NativeAot/Generator.cs +++ b/src/BenchmarkDotNet/Toolchains/NativeAot/Generator.cs @@ -69,11 +69,12 @@ protected override string GetBinariesDirectoryPath(string buildArtifactsDirector protected override void GenerateBuildScript(BuildPartition buildPartition, ArtifactsPaths artifactsPaths) { + bool useArtifactsPath = UseArtifactsPathIfSupported && DotNetCliCommandExecutor.DotNetSdkSupportsArtifactsPath(CliPath); string extraArguments = NativeAotToolchain.GetExtraArguments(runtimeIdentifier); var content = new StringBuilder(300) - .AppendLine($"call {CliPath ?? "dotnet"} {DotNetCliCommand.GetRestoreCommand(artifactsPaths, buildPartition, DotNetCliCommandExecutor.DotNetSdkSupportsArtifactsPath(CliPath), extraArguments)}") - .AppendLine($"call {CliPath ?? "dotnet"} {DotNetCliCommand.GetPublishCommand(artifactsPaths, buildPartition, DotNetCliCommandExecutor.DotNetSdkSupportsArtifactsPath(CliPath), extraArguments)}") + .AppendLine($"call {CliPath ?? "dotnet"} {DotNetCliCommand.GetRestoreCommand(artifactsPaths, buildPartition, useArtifactsPath, extraArguments)}") + .AppendLine($"call {CliPath ?? "dotnet"} {DotNetCliCommand.GetPublishCommand(artifactsPaths, buildPartition, useArtifactsPath, extraArguments)}") .ToString(); File.WriteAllText(artifactsPaths.BuildScriptFilePath, content); diff --git a/src/BenchmarkDotNet/Toolchains/ToolchainExtensions.cs b/src/BenchmarkDotNet/Toolchains/ToolchainExtensions.cs index c2872810fc..1dd7f01934 100644 --- a/src/BenchmarkDotNet/Toolchains/ToolchainExtensions.cs +++ b/src/BenchmarkDotNet/Toolchains/ToolchainExtensions.cs @@ -182,5 +182,13 @@ private static IToolchain GetToolchain(RuntimeMoniker runtimeMoniker) throw new ArgumentOutOfRangeException(nameof(runtimeMoniker), runtimeMoniker, "RuntimeMoniker not supported"); } } + + internal static bool IsSafeToBuildInParallel(this IToolchain toolchain) + // Toolchains that do not use dotnet sdk are assumed to be safe to build in parallel. + // This might not be true for custom toolchains, but the old behavior was to build everything in parallel, so it's at least not a regression. + => toolchain.Builder is not DotNetCliBuilderBase dotnetBuilder + // If the builder does not use ArtifactsPath, or the dotnet sdk does not support it, 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. #2425 + || (dotnetBuilder.UseArtifactsPathIfSupported && DotNetCliCommandExecutor.DotNetSdkSupportsArtifactsPath(dotnetBuilder.CustomDotNetCliPath)); } } \ No newline at end of file