diff --git a/src/BenchmarkDotNet/Running/BenchmarkRunnerClean.cs b/src/BenchmarkDotNet/Running/BenchmarkRunnerClean.cs index 6d9d48c3cc..cbe5f4507b 100644 --- a/src/BenchmarkDotNet/Running/BenchmarkRunnerClean.cs +++ b/src/BenchmarkDotNet/Running/BenchmarkRunnerClean.cs @@ -361,42 +361,68 @@ private static ImmutableArray Validate(params BenchmarkRunInfo[ private static Dictionary BuildInParallel(ILogger logger, string rootArtifactsFolderPath, BuildPartition[] buildPartitions, in StartedClock globalChronometer, EventProcessor eventProcessor) { - logger.WriteLineHeader($"// ***** Building {buildPartitions.Length} exe(s) in Parallel: Start *****"); + var parallelPartitions = new List<(BuildPartition buildPartition, IToolchain toolchain, bool isSequential)[]>(); + var sequentialPartitions = new List<(BuildPartition, IToolchain, bool)>(); + foreach (var partition in buildPartitions) + { + // 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 + sequentialPartitions.Add((partition, toolchain, true)); + } + if (sequentialPartitions.Count > 0) + { + // We build the sequential partitions in parallel with the other partitions to complete as fast as possible. + // Place them first to make sure they are started first. + parallelPartitions.Insert(0, [.. sequentialPartitions]); + } - var buildLogger = buildPartitions.Length == 1 ? logger : NullLogger.Instance; // when we have just one partition we can print to std out + logger.WriteLineHeader($"// ***** Building {parallelPartitions.Count} exe(s) in Parallel{(sequentialPartitions.Count <= 1 ? "" : $", {sequentialPartitions.Count} sequentially")} *****"); + + var buildLogger = parallelPartitions.Count == 1 ? logger : NullLogger.Instance; // when we have just one parallel partition we can print to std out var beforeParallelBuild = globalChronometer.GetElapsed(); - var buildResults = buildPartitions + var parallelBuildResults = parallelPartitions .AsParallel() - .Select(buildPartition => (Partition: buildPartition, Result: Build(buildPartition, rootArtifactsFolderPath, buildLogger))) + .SelectMany(partitions => partitions + .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]); } @@ -412,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 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 7a7afc3184..b48da6629e 100644 --- a/src/BenchmarkDotNet/Toolchains/DotNetCli/DotNetCliBuilder.cs +++ b/src/BenchmarkDotNet/Toolchains/DotNetCli/DotNetCliBuilder.cs @@ -7,23 +7,33 @@ namespace BenchmarkDotNet.Toolchains.DotNetCli { - [PublicAPI] - public class DotNetCliBuilder : IBuilder + public abstract class DotNetCliBuilderBase : IBuilder { - private string TargetFrameworkMoniker { get; } + public string? CustomDotNetCliPath { get; protected set; } + internal bool UseArtifactsPathIfSupported { get; private protected set; } = true; + public abstract BuildResult Build(GenerateResult generateResult, BuildPartition buildPartition, ILogger logger); + } - private string CustomDotNetCliPath { get; } + [PublicAPI] + public class DotNetCliBuilder : DotNetCliBuilderBase + { private bool LogOutput { get; } [PublicAPI] public DotNetCliBuilder(string targetFrameworkMoniker, string? customDotNetCliPath = null, bool logOutput = false) { - TargetFrameworkMoniker = targetFrameworkMoniker; CustomDotNetCliPath = customDotNetCliPath; LogOutput = logOutput; } - public BuildResult Build(GenerateResult generateResult, BuildPartition buildPartition, ILogger logger) + 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( CustomDotNetCliPath, @@ -34,7 +44,7 @@ public BuildResult Build(GenerateResult generateResult, BuildPartition buildPart 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 e9078f293f..b41925e6d5 100644 --- a/src/BenchmarkDotNet/Toolchains/DotNetCli/DotNetCliCommand.cs +++ b/src/BenchmarkDotNet/Toolchains/DotNetCli/DotNetCliCommand.cs @@ -4,6 +4,7 @@ using System.Linq; using System.Text; using BenchmarkDotNet.Characteristics; +using BenchmarkDotNet.Environments; using BenchmarkDotNet.Extensions; using BenchmarkDotNet.Jobs; using BenchmarkDotNet.Loggers; @@ -52,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)); @@ -64,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, $"{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, $"{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)); @@ -104,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() @@ -128,31 +129,34 @@ public DotNetCliCommandResult AddPackages() return DotNetCliCommandResult.Success(executionTime, stdOutput.ToString()); } - public DotNetCliCommandResult Restore() + private bool GetWithArtifactsPath(bool useArtifactsPathIfSupported) => useArtifactsPathIfSupported && DotNetCliCommandExecutor.DotNetSdkSupportsArtifactsPath(CliPath); + + public DotNetCliCommandResult Restore(bool useArtifactsPathIfSupported = true) => DotNetCliCommandExecutor.Execute(WithArguments( - GetRestoreCommand(GenerateResult.ArtifactsPaths, BuildPartition, 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, 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, $"{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, 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, $"{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); - internal static string GetRestoreCommand(ArtifactsPaths artifactsPaths, BuildPartition buildPartition, string? extraArguments = null, string? binLogSuffix = null, bool excludeOutput = false) + internal static string GetRestoreCommand(ArtifactsPaths artifactsPaths, BuildPartition buildPartition, + bool withArtifactsPath, string? extraArguments = null, string? binLogSuffix = null, bool excludeOutput = false) => new StringBuilder() .AppendArgument("restore") .AppendArgument(string.IsNullOrEmpty(artifactsPaths.PackagesDirectoryName) ? string.Empty : $"--packages \"{artifactsPaths.PackagesDirectoryName}\"") @@ -160,10 +164,11 @@ internal static string GetRestoreCommand(ArtifactsPaths artifactsPaths, BuildPar .AppendArgument(extraArguments) .AppendArgument(GetMandatoryMsBuildSettings(buildPartition.BuildConfiguration)) .AppendArgument(GetMsBuildBinLogArgument(buildPartition, binLogSuffix)) - .MaybeAppendOutputPaths(artifactsPaths, true, excludeOutput) + .MaybeAppendOutputPaths(artifactsPaths, withArtifactsPath, true, excludeOutput) .ToString(); - internal static string GetBuildCommand(ArtifactsPaths artifactsPaths, BuildPartition buildPartition, string? extraArguments = null, string? binLogSuffix = null, bool excludeOutput = false) + internal static string GetBuildCommand(ArtifactsPaths artifactsPaths, BuildPartition buildPartition, + bool withArtifactsPath, string? extraArguments = null, string? binLogSuffix = null, bool excludeOutput = false) => new StringBuilder() .AppendArgument($"build -c {buildPartition.BuildConfiguration}") // we don't need to specify TFM, our auto-generated project contains always single one .AppendArgument(GetCustomMsBuildArguments(buildPartition.RepresentativeBenchmarkCase, buildPartition.Resolver)) @@ -171,10 +176,11 @@ internal static string GetBuildCommand(ArtifactsPaths artifactsPaths, BuildParti .AppendArgument(GetMandatoryMsBuildSettings(buildPartition.BuildConfiguration)) .AppendArgument(string.IsNullOrEmpty(artifactsPaths.PackagesDirectoryName) ? string.Empty : $"/p:NuGetPackageRoot=\"{artifactsPaths.PackagesDirectoryName}\"") .AppendArgument(GetMsBuildBinLogArgument(buildPartition, binLogSuffix)) - .MaybeAppendOutputPaths(artifactsPaths, excludeOutput: excludeOutput) + .MaybeAppendOutputPaths(artifactsPaths, withArtifactsPath, excludeOutput: excludeOutput) .ToString(); - internal static string GetPublishCommand(ArtifactsPaths artifactsPaths, BuildPartition buildPartition, string? extraArguments = null, string? binLogSuffix = null) + internal static string GetPublishCommand(ArtifactsPaths artifactsPaths, BuildPartition buildPartition, + bool withArtifactsPath, string? extraArguments = null, string? binLogSuffix = null) => new StringBuilder() .AppendArgument($"publish -c {buildPartition.BuildConfiguration}") // we don't need to specify TFM, our auto-generated project contains always single one .AppendArgument(GetCustomMsBuildArguments(buildPartition.RepresentativeBenchmarkCase, buildPartition.Resolver)) @@ -182,7 +188,7 @@ internal static string GetPublishCommand(ArtifactsPaths artifactsPaths, BuildPar .AppendArgument(GetMandatoryMsBuildSettings(buildPartition.BuildConfiguration)) .AppendArgument(string.IsNullOrEmpty(artifactsPaths.PackagesDirectoryName) ? string.Empty : $"/p:NuGetPackageRoot=\"{artifactsPaths.PackagesDirectoryName}\"") .AppendArgument(GetMsBuildBinLogArgument(buildPartition, binLogSuffix)) - .MaybeAppendOutputPaths(artifactsPaths) + .MaybeAppendOutputPaths(artifactsPaths, withArtifactsPath) .ToString(); private static string GetMsBuildBinLogArgument(BuildPartition buildPartition, string suffix) @@ -257,7 +263,7 @@ internal static class DotNetCliCommandExtensions // We force the project to output binaries to a new directory. // Specifying --output and --no-dependencies breaks the build (because the previous build was not done using the custom output path), // so we don't include it if we're building no-deps (only supported for integration tests). - internal static StringBuilder MaybeAppendOutputPaths(this StringBuilder stringBuilder, ArtifactsPaths artifactsPaths, bool isRestore = false, bool excludeOutput = false) + internal static StringBuilder MaybeAppendOutputPaths(this StringBuilder stringBuilder, ArtifactsPaths artifactsPaths, bool withArtifactsPath, bool isRestore = false, bool excludeOutput = false) => excludeOutput ? stringBuilder : stringBuilder @@ -266,6 +272,8 @@ internal static StringBuilder MaybeAppendOutputPaths(this StringBuilder stringBu .AppendArgument($"/p:OutDir=\"{artifactsPaths.BinariesDirectoryPath}{Path.AltDirectorySeparatorChar}\"") // OutputPath is legacy, per-project version of OutDir. We set both just in case. https://github.com/dotnet/msbuild/issues/87 .AppendArgument($"/p:OutputPath=\"{artifactsPaths.BinariesDirectoryPath}{Path.AltDirectorySeparatorChar}\"") - .AppendArgument(isRestore ? string.Empty : $"--output \"{artifactsPaths.BinariesDirectoryPath}{Path.AltDirectorySeparatorChar}\""); + .AppendArgument(isRestore ? string.Empty : $"--output \"{artifactsPaths.BinariesDirectoryPath}{Path.AltDirectorySeparatorChar}\"") + // We set ArtifactsPath to support parallel builds (requires dotnet sdk 8+). #2425 + .AppendArgument(!withArtifactsPath ? string.Empty : $"/p:ArtifactsPath=\"{artifactsPaths.BuildArtifactsDirectoryPath}{Path.AltDirectorySeparatorChar}\""); } } diff --git a/src/BenchmarkDotNet/Toolchains/DotNetCli/DotNetCliCommandExecutor.cs b/src/BenchmarkDotNet/Toolchains/DotNetCli/DotNetCliCommandExecutor.cs index 5cbff89b47..dea9bb82bf 100644 --- a/src/BenchmarkDotNet/Toolchains/DotNetCli/DotNetCliCommandExecutor.cs +++ b/src/BenchmarkDotNet/Toolchains/DotNetCli/DotNetCliCommandExecutor.cs @@ -6,6 +6,7 @@ using System.Text; using System.Text.RegularExpressions; using BenchmarkDotNet.Detectors; +using BenchmarkDotNet.Environments; using BenchmarkDotNet.Extensions; using BenchmarkDotNet.Helpers; using BenchmarkDotNet.Jobs; @@ -55,9 +56,19 @@ public static DotNetCliCommandResult Execute(DotNetCliCommand parameters) } } - internal static string? GetDotNetSdkVersion() + internal static bool DotNetSdkSupportsArtifactsPath(string? customDotNetCliPath) { - using (var process = new Process { StartInfo = BuildStartInfo(customDotNetCliPath: null, workingDirectory: string.Empty, arguments: "--version", redirectStandardError: false) }) + var version = string.IsNullOrEmpty(customDotNetCliPath) + ? HostEnvironmentInfo.GetCurrent().DotNetSdkVersion.Value + : GetDotNetSdkVersion(customDotNetCliPath); + return Version.TryParse(version, out var semVer) && semVer.Major >= 8; + } + + internal static string? GetDotNetSdkVersion() => GetDotNetSdkVersion(null); + + internal static string? GetDotNetSdkVersion(string? customDotNetCliPath) + { + using (var process = new Process { StartInfo = BuildStartInfo(customDotNetCliPath, workingDirectory: string.Empty, arguments: "--version", redirectStandardError: false) }) using (new ConsoleExitHandler(process, NullLogger.Instance)) { try diff --git a/src/BenchmarkDotNet/Toolchains/DotNetCli/DotNetCliGenerator.cs b/src/BenchmarkDotNet/Toolchains/DotNetCli/DotNetCliGenerator.cs index a432fbf881..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)}") - .AppendLine($"call {CliPath ?? "dotnet"} {DotNetCliCommand.GetBuildCommand(artifactsPaths, buildPartition)}") + .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 83158e0b1e..d1df671ed5 100644 --- a/src/BenchmarkDotNet/Toolchains/DotNetCli/DotNetCliPublisher.cs +++ b/src/BenchmarkDotNet/Toolchains/DotNetCli/DotNetCliPublisher.cs @@ -6,7 +6,7 @@ namespace BenchmarkDotNet.Toolchains.DotNetCli { - public class DotNetCliPublisher : IBuilder + public class DotNetCliPublisher : DotNetCliBuilderBase { public DotNetCliPublisher( string? customDotNetCliPath = null, @@ -18,13 +18,11 @@ public DotNetCliPublisher( EnvironmentVariables = environmentVariables; } - 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 +31,6 @@ public BuildResult Build(GenerateResult generateResult, BuildPartition buildPart 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..279afae18b 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) { @@ -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 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); } } diff --git a/src/BenchmarkDotNet/Toolchains/MonoAotLLVM/MonoAotLLVMGenerator.cs b/src/BenchmarkDotNet/Toolchains/MonoAotLLVM/MonoAotLLVMGenerator.cs index de3cef53d3..1682a4a601 100644 --- a/src/BenchmarkDotNet/Toolchains/MonoAotLLVM/MonoAotLLVMGenerator.cs +++ b/src/BenchmarkDotNet/Toolchains/MonoAotLLVM/MonoAotLLVMGenerator.cs @@ -18,7 +18,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 71ad2ade2b..0b3a04705e 100644 --- a/src/BenchmarkDotNet/Toolchains/MonoWasm/WasmToolchain.cs +++ b/src/BenchmarkDotNet/Toolchains/MonoWasm/WasmToolchain.cs @@ -48,10 +48,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 b3adfc3e3c..17b3ee0e54 100644 --- a/src/BenchmarkDotNet/Toolchains/NativeAot/Generator.cs +++ b/src/BenchmarkDotNet/Toolchains/NativeAot/Generator.cs @@ -70,11 +70,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, extraArguments)}") - .AppendLine($"call {CliPath ?? "dotnet"} {DotNetCliCommand.GetPublishCommand(artifactsPaths, buildPartition, 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 499d5d8f40..6806934c40 100644 --- a/src/BenchmarkDotNet/Toolchains/ToolchainExtensions.cs +++ b/src/BenchmarkDotNet/Toolchains/ToolchainExtensions.cs @@ -183,5 +183,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