Skip to content

Commit c11fd2c

Browse files
timcasselltimcassell
committed
Fix parallel builds with dotnet sdk 8+, build sequentially for older sdks.
1 parent ca5dfdf commit c11fd2c

File tree

7 files changed

+80
-35
lines changed

7 files changed

+80
-35
lines changed

‎src/BenchmarkDotNet/Running/BenchmarkRunnerClean.cs‎

Lines changed: 30 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@
2222
using BenchmarkDotNet.Portability;
2323
using BenchmarkDotNet.Reports;
2424
using BenchmarkDotNet.Toolchains;
25+
using BenchmarkDotNet.Toolchains.DotNetCli;
2526
using BenchmarkDotNet.Toolchains.Parameters;
2627
using BenchmarkDotNet.Toolchains.Results;
2728
using BenchmarkDotNet.Validators;
@@ -361,15 +362,40 @@ private static ImmutableArray<ValidationError> Validate(params BenchmarkRunInfo[
361362

362363
private static Dictionary<BuildPartition, BuildResult> BuildInParallel(ILogger logger, string rootArtifactsFolderPath, BuildPartition[] buildPartitions, in StartedClock globalChronometer, EventProcessor eventProcessor)
363364
{
364-
logger.WriteLineHeader($"// ***** Building {buildPartitions.Length} exe(s) in Parallel: Start *****");
365+
var parallelPartitions = new List<BuildPartition[]>();
366+
var sequentialPartitions = new List<BuildPartition>();
367+
foreach (var partition in buildPartitions)
368+
{
369+
// If dotnet sdk does not support ArtifactsPath, it's unsafe to build the same project in parallel.
370+
// We cannot rely on falling back to sequential after failure, because the builds can be corrupted.
371+
// Therefore we have to build them sequentially. #2425
372+
if (partition.RepresentativeBenchmarkCase.GetToolchain().Builder is DotNetCliBuilderBase dotnetBuilder
373+
&& !DotNetCliCommandExecutor.DotNetSdkSupportsArtifactsPath(dotnetBuilder.CustomDotNetCliPath))
374+
{
375+
sequentialPartitions.Add(partition);
376+
}
377+
else
378+
{
379+
parallelPartitions.Add([partition]);
380+
}
381+
}
382+
if (sequentialPartitions.Count > 0)
383+
{
384+
// We build the sequential partitions in parallel with the other partitions to complete as fast as possible.
385+
// Place them first to make sure they are started first.
386+
parallelPartitions.Insert(0, [.. sequentialPartitions]);
387+
}
388+
389+
logger.WriteLineHeader($"// ***** Building {parallelPartitions.Count} exe(s) in Parallel{(sequentialPartitions.Count <= 1 ? "" : $", {sequentialPartitions.Count} sequentially")} *****");
365390

366-
var buildLogger = buildPartitions.Length == 1 ? logger : NullLogger.Instance; // when we have just one partition we can print to std out
391+
var buildLogger = parallelPartitions.Count == 1 ? logger : NullLogger.Instance; // when we have just one parallel partition we can print to std out
367392

368393
var beforeParallelBuild = globalChronometer.GetElapsed();
369394

370-
var buildResults = buildPartitions
395+
var buildResults = parallelPartitions
371396
.AsParallel()
372-
.Select(buildPartition => (Partition: buildPartition, Result: Build(buildPartition, rootArtifactsFolderPath, buildLogger)))
397+
.SelectMany(partitions => partitions
398+
.Select(buildPartition => (Partition: buildPartition, Result: Build(buildPartition, rootArtifactsFolderPath, buildLogger))))
373399
.AsSequential() // Ensure that build completion events are processed sequentially
374400
.Select(build =>
375401
{

‎src/BenchmarkDotNet/Toolchains/DotNetCli/DotNetCliBuilder.cs‎

Lines changed: 8 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -7,23 +7,25 @@
77

88
namespace BenchmarkDotNet.Toolchains.DotNetCli
99
{
10-
[PublicAPI]
11-
public class DotNetCliBuilder : IBuilder
10+
public abstract class DotNetCliBuilderBase : IBuilder
1211
{
13-
private string TargetFrameworkMoniker { get; }
12+
public string? CustomDotNetCliPath { get; protected set; }
13+
public abstract BuildResult Build(GenerateResult generateResult, BuildPartition buildPartition, ILogger logger);
14+
}
1415

15-
private string CustomDotNetCliPath { get; }
16+
[PublicAPI]
17+
public class DotNetCliBuilder : DotNetCliBuilderBase
18+
{
1619
private bool LogOutput { get; }
1720

1821
[PublicAPI]
1922
public DotNetCliBuilder(string targetFrameworkMoniker, string? customDotNetCliPath = null, bool logOutput = false)
2023
{
21-
TargetFrameworkMoniker = targetFrameworkMoniker;
2224
CustomDotNetCliPath = customDotNetCliPath;
2325
LogOutput = logOutput;
2426
}
2527

26-
public BuildResult Build(GenerateResult generateResult, BuildPartition buildPartition, ILogger logger)
28+
public override BuildResult Build(GenerateResult generateResult, BuildPartition buildPartition, ILogger logger)
2729
{
2830
BuildResult buildResult = new DotNetCliCommand(
2931
CustomDotNetCliPath,

‎src/BenchmarkDotNet/Toolchains/DotNetCli/DotNetCliCommand.cs‎

Lines changed: 23 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44
using System.Linq;
55
using System.Text;
66
using BenchmarkDotNet.Characteristics;
7+
using BenchmarkDotNet.Environments;
78
using BenchmarkDotNet.Extensions;
89
using BenchmarkDotNet.Jobs;
910
using BenchmarkDotNet.Loggers;
@@ -71,12 +72,12 @@ public BuildResult RestoreThenBuild()
7172
if (BuildPartition.ForcedNoDependenciesForIntegrationTests)
7273
{
7374
var restoreResult = DotNetCliCommandExecutor.Execute(WithArguments(
74-
GetRestoreCommand(GenerateResult.ArtifactsPaths, BuildPartition, $"{Arguments} --no-dependencies", "restore-no-deps", excludeOutput: true)));
75+
GetRestoreCommand(GenerateResult.ArtifactsPaths, BuildPartition, GetWithArtifactsPath(), $"{Arguments} --no-dependencies", "restore-no-deps", excludeOutput: true)));
7576
if (!restoreResult.IsSuccess)
7677
return BuildResult.Failure(GenerateResult, restoreResult.AllInformation);
7778

7879
return DotNetCliCommandExecutor.Execute(WithArguments(
79-
GetBuildCommand(GenerateResult.ArtifactsPaths, BuildPartition, $"{Arguments} --no-restore --no-dependencies", "build-no-restore-no-deps", excludeOutput: true)))
80+
GetBuildCommand(GenerateResult.ArtifactsPaths, BuildPartition, GetWithArtifactsPath(), $"{Arguments} --no-restore --no-dependencies", "build-no-restore-no-deps", excludeOutput: true)))
8081
.ToBuildResult(GenerateResult);
8182
}
8283
else
@@ -128,61 +129,66 @@ public DotNetCliCommandResult AddPackages()
128129
return DotNetCliCommandResult.Success(executionTime, stdOutput.ToString());
129130
}
130131

132+
private bool GetWithArtifactsPath() => DotNetCliCommandExecutor.DotNetSdkSupportsArtifactsPath(CliPath);
133+
131134
public DotNetCliCommandResult Restore()
132135
=> DotNetCliCommandExecutor.Execute(WithArguments(
133-
GetRestoreCommand(GenerateResult.ArtifactsPaths, BuildPartition, Arguments, "restore")));
136+
GetRestoreCommand(GenerateResult.ArtifactsPaths, BuildPartition, GetWithArtifactsPath(), Arguments, "restore")));
134137

135138
public DotNetCliCommandResult Build()
136139
=> DotNetCliCommandExecutor.Execute(WithArguments(
137-
GetBuildCommand(GenerateResult.ArtifactsPaths, BuildPartition, Arguments, "build")));
140+
GetBuildCommand(GenerateResult.ArtifactsPaths, BuildPartition, GetWithArtifactsPath(), Arguments, "build")));
138141

139142
public DotNetCliCommandResult BuildNoRestore()
140143
=> DotNetCliCommandExecutor.Execute(WithArguments(
141-
GetBuildCommand(GenerateResult.ArtifactsPaths, BuildPartition, $"{Arguments} --no-restore", "build-no-restore")));
144+
GetBuildCommand(GenerateResult.ArtifactsPaths, BuildPartition, GetWithArtifactsPath(), $"{Arguments} --no-restore", "build-no-restore")));
142145

143146
public DotNetCliCommandResult Publish()
144147
=> DotNetCliCommandExecutor.Execute(WithArguments(
145-
GetPublishCommand(GenerateResult.ArtifactsPaths, BuildPartition, Arguments, "publish")));
148+
GetPublishCommand(GenerateResult.ArtifactsPaths, BuildPartition, GetWithArtifactsPath(), Arguments, "publish")));
146149

147150
// PublishNoBuildAndNoRestore was removed because we set --output in the build step. We use the implicit build included in the publish command.
148151
public DotNetCliCommandResult PublishNoRestore()
149152
=> DotNetCliCommandExecutor.Execute(WithArguments(
150-
GetPublishCommand(GenerateResult.ArtifactsPaths, BuildPartition, $"{Arguments} --no-restore", "publish-no-restore")));
153+
GetPublishCommand(GenerateResult.ArtifactsPaths, BuildPartition, GetWithArtifactsPath(), $"{Arguments} --no-restore", "publish-no-restore")));
151154

152155
internal static IEnumerable<string> GetAddPackagesCommands(BuildPartition buildPartition)
153156
=> GetNuGetAddPackageCommands(buildPartition.RepresentativeBenchmarkCase, buildPartition.Resolver);
154157

155-
internal static string GetRestoreCommand(ArtifactsPaths artifactsPaths, BuildPartition buildPartition, string? extraArguments = null, string? binLogSuffix = null, bool excludeOutput = false)
158+
internal static string GetRestoreCommand(ArtifactsPaths artifactsPaths, BuildPartition buildPartition,
159+
bool withArtifactsPath, string? extraArguments = null, string? binLogSuffix = null, bool excludeOutput = false)
156160
=> new StringBuilder()
157161
.AppendArgument("restore")
158162
.AppendArgument(string.IsNullOrEmpty(artifactsPaths.PackagesDirectoryName) ? string.Empty : $"--packages \"{artifactsPaths.PackagesDirectoryName}\"")
159163
.AppendArgument(GetCustomMsBuildArguments(buildPartition.RepresentativeBenchmarkCase, buildPartition.Resolver))
160164
.AppendArgument(extraArguments)
161165
.AppendArgument(GetMandatoryMsBuildSettings(buildPartition.BuildConfiguration))
162166
.AppendArgument(GetMsBuildBinLogArgument(buildPartition, binLogSuffix))
163-
.MaybeAppendOutputPaths(artifactsPaths, true, excludeOutput)
167+
.MaybeAppendOutputPaths(artifactsPaths, withArtifactsPath, true, excludeOutput)
164168
.ToString();
165169

166-
internal static string GetBuildCommand(ArtifactsPaths artifactsPaths, BuildPartition buildPartition, string? extraArguments = null, string? binLogSuffix = null, bool excludeOutput = false)
170+
internal static string GetBuildCommand(ArtifactsPaths artifactsPaths, BuildPartition buildPartition,
171+
bool withArtifactsPath, string? extraArguments = null, string? binLogSuffix = null, bool excludeOutput = false)
167172
=> new StringBuilder()
168173
.AppendArgument($"build -c {buildPartition.BuildConfiguration}") // we don't need to specify TFM, our auto-generated project contains always single one
169174
.AppendArgument(GetCustomMsBuildArguments(buildPartition.RepresentativeBenchmarkCase, buildPartition.Resolver))
170175
.AppendArgument(extraArguments)
171176
.AppendArgument(GetMandatoryMsBuildSettings(buildPartition.BuildConfiguration))
172177
.AppendArgument(string.IsNullOrEmpty(artifactsPaths.PackagesDirectoryName) ? string.Empty : $"/p:NuGetPackageRoot=\"{artifactsPaths.PackagesDirectoryName}\"")
173178
.AppendArgument(GetMsBuildBinLogArgument(buildPartition, binLogSuffix))
174-
.MaybeAppendOutputPaths(artifactsPaths, excludeOutput: excludeOutput)
179+
.MaybeAppendOutputPaths(artifactsPaths, withArtifactsPath, excludeOutput: excludeOutput)
175180
.ToString();
176181

177-
internal static string GetPublishCommand(ArtifactsPaths artifactsPaths, BuildPartition buildPartition, string? extraArguments = null, string? binLogSuffix = null)
182+
internal static string GetPublishCommand(ArtifactsPaths artifactsPaths, BuildPartition buildPartition,
183+
bool withArtifactsPath, string? extraArguments = null, string? binLogSuffix = null)
178184
=> new StringBuilder()
179185
.AppendArgument($"publish -c {buildPartition.BuildConfiguration}") // we don't need to specify TFM, our auto-generated project contains always single one
180186
.AppendArgument(GetCustomMsBuildArguments(buildPartition.RepresentativeBenchmarkCase, buildPartition.Resolver))
181187
.AppendArgument(extraArguments)
182188
.AppendArgument(GetMandatoryMsBuildSettings(buildPartition.BuildConfiguration))
183189
.AppendArgument(string.IsNullOrEmpty(artifactsPaths.PackagesDirectoryName) ? string.Empty : $"/p:NuGetPackageRoot=\"{artifactsPaths.PackagesDirectoryName}\"")
184190
.AppendArgument(GetMsBuildBinLogArgument(buildPartition, binLogSuffix))
185-
.MaybeAppendOutputPaths(artifactsPaths)
191+
.MaybeAppendOutputPaths(artifactsPaths, withArtifactsPath)
186192
.ToString();
187193

188194
private static string GetMsBuildBinLogArgument(BuildPartition buildPartition, string suffix)
@@ -257,7 +263,7 @@ internal static class DotNetCliCommandExtensions
257263
// We force the project to output binaries to a new directory.
258264
// Specifying --output and --no-dependencies breaks the build (because the previous build was not done using the custom output path),
259265
// so we don't include it if we're building no-deps (only supported for integration tests).
260-
internal static StringBuilder MaybeAppendOutputPaths(this StringBuilder stringBuilder, ArtifactsPaths artifactsPaths, bool isRestore = false, bool excludeOutput = false)
266+
internal static StringBuilder MaybeAppendOutputPaths(this StringBuilder stringBuilder, ArtifactsPaths artifactsPaths, bool withArtifactsPath, bool isRestore = false, bool excludeOutput = false)
261267
=> excludeOutput
262268
? stringBuilder
263269
: stringBuilder
@@ -266,6 +272,8 @@ internal static StringBuilder MaybeAppendOutputPaths(this StringBuilder stringBu
266272
.AppendArgument($"/p:OutDir=\"{artifactsPaths.BinariesDirectoryPath}{Path.AltDirectorySeparatorChar}\"")
267273
// OutputPath is legacy, per-project version of OutDir. We set both just in case. https://github.com/dotnet/msbuild/issues/87
268274
.AppendArgument($"/p:OutputPath=\"{artifactsPaths.BinariesDirectoryPath}{Path.AltDirectorySeparatorChar}\"")
269-
.AppendArgument(isRestore ? string.Empty : $"--output \"{artifactsPaths.BinariesDirectoryPath}{Path.AltDirectorySeparatorChar}\"");
275+
.AppendArgument(isRestore ? string.Empty : $"--output \"{artifactsPaths.BinariesDirectoryPath}{Path.AltDirectorySeparatorChar}\"")
276+
// We set ArtifactsPath to support parallel builds (requires dotnet sdk 8+). #2425
277+
.AppendArgument(!withArtifactsPath ? string.Empty : $"/p:ArtifactsPath=\"{artifactsPaths.BuildArtifactsDirectoryPath}{Path.AltDirectorySeparatorChar}\"");
270278
}
271279
}

‎src/BenchmarkDotNet/Toolchains/DotNetCli/DotNetCliCommandExecutor.cs‎

Lines changed: 13 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66
using System.Text;
77
using System.Text.RegularExpressions;
88
using BenchmarkDotNet.Detectors;
9+
using BenchmarkDotNet.Environments;
910
using BenchmarkDotNet.Extensions;
1011
using BenchmarkDotNet.Helpers;
1112
using BenchmarkDotNet.Jobs;
@@ -55,9 +56,19 @@ public static DotNetCliCommandResult Execute(DotNetCliCommand parameters)
5556
}
5657
}
5758

58-
internal static string? GetDotNetSdkVersion()
59+
internal static bool DotNetSdkSupportsArtifactsPath(string? customDotNetCliPath)
5960
{
60-
using (var process = new Process { StartInfo = BuildStartInfo(customDotNetCliPath: null, workingDirectory: string.Empty, arguments: "--version", redirectStandardError: false) })
61+
var version = string.IsNullOrEmpty(customDotNetCliPath)
62+
? HostEnvironmentInfo.GetCurrent().DotNetSdkVersion.Value
63+
: GetDotNetSdkVersion(customDotNetCliPath);
64+
return Version.TryParse(version, out var semVer) && semVer.Major >= 8;
65+
}
66+
67+
internal static string? GetDotNetSdkVersion() => GetDotNetSdkVersion(null);
68+
69+
internal static string? GetDotNetSdkVersion(string? customDotNetCliPath)
70+
{
71+
using (var process = new Process { StartInfo = BuildStartInfo(customDotNetCliPath, workingDirectory: string.Empty, arguments: "--version", redirectStandardError: false) })
6172
using (new ConsoleExitHandler(process, NullLogger.Instance))
6273
{
6374
try

‎src/BenchmarkDotNet/Toolchains/DotNetCli/DotNetCliGenerator.cs‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -101,8 +101,8 @@ protected override void CopyAllRequiredFiles(ArtifactsPaths artifactsPaths)
101101
protected override void GenerateBuildScript(BuildPartition buildPartition, ArtifactsPaths artifactsPaths)
102102
{
103103
var content = new StringBuilder(300)
104-
.AppendLine($"call {CliPath ?? "dotnet"} {DotNetCliCommand.GetRestoreCommand(artifactsPaths, buildPartition)}")
105-
.AppendLine($"call {CliPath ?? "dotnet"} {DotNetCliCommand.GetBuildCommand(artifactsPaths, buildPartition)}")
104+
.AppendLine($"call {CliPath ?? "dotnet"} {DotNetCliCommand.GetRestoreCommand(artifactsPaths, buildPartition, DotNetCliCommandExecutor.DotNetSdkSupportsArtifactsPath(CliPath))}")
105+
.AppendLine($"call {CliPath ?? "dotnet"} {DotNetCliCommand.GetBuildCommand(artifactsPaths, buildPartition, DotNetCliCommandExecutor.DotNetSdkSupportsArtifactsPath(CliPath))}")
106106
.ToString();
107107

108108
File.WriteAllText(artifactsPaths.BuildScriptFilePath, content);

‎src/BenchmarkDotNet/Toolchains/DotNetCli/DotNetCliPublisher.cs‎

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@
66

77
namespace BenchmarkDotNet.Toolchains.DotNetCli
88
{
9-
public class DotNetCliPublisher : IBuilder
9+
public class DotNetCliPublisher : DotNetCliBuilderBase
1010
{
1111
public DotNetCliPublisher(
1212
string? customDotNetCliPath = null,
@@ -18,13 +18,11 @@ public DotNetCliPublisher(
1818
EnvironmentVariables = environmentVariables;
1919
}
2020

21-
private string? CustomDotNetCliPath { get; }
22-
2321
private string? ExtraArguments { get; }
2422

2523
private IReadOnlyList<EnvironmentVariable>? EnvironmentVariables { get; }
2624

27-
public BuildResult Build(GenerateResult generateResult, BuildPartition buildPartition, ILogger logger)
25+
public override BuildResult Build(GenerateResult generateResult, BuildPartition buildPartition, ILogger logger)
2826
=> new DotNetCliCommand(
2927
CustomDotNetCliPath,
3028
ExtraArguments,

‎src/BenchmarkDotNet/Toolchains/NativeAot/Generator.cs‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -73,8 +73,8 @@ protected override void GenerateBuildScript(BuildPartition buildPartition, Artif
7373
string extraArguments = NativeAotToolchain.GetExtraArguments(runtimeIdentifier);
7474

7575
var content = new StringBuilder(300)
76-
.AppendLine($"call {CliPath ?? "dotnet"} {DotNetCliCommand.GetRestoreCommand(artifactsPaths, buildPartition, extraArguments)}")
77-
.AppendLine($"call {CliPath ?? "dotnet"} {DotNetCliCommand.GetPublishCommand(artifactsPaths, buildPartition, extraArguments)}")
76+
.AppendLine($"call {CliPath ?? "dotnet"} {DotNetCliCommand.GetRestoreCommand(artifactsPaths, buildPartition, DotNetCliCommandExecutor.DotNetSdkSupportsArtifactsPath(CliPath), extraArguments)}")
77+
.AppendLine($"call {CliPath ?? "dotnet"} {DotNetCliCommand.GetPublishCommand(artifactsPaths, buildPartition, DotNetCliCommandExecutor.DotNetSdkSupportsArtifactsPath(CliPath), extraArguments)}")
7878
.ToString();
7979

8080
File.WriteAllText(artifactsPaths.BuildScriptFilePath, content);

0 commit comments

Comments
 (0)