Skip to content

Commit

Permalink
Fix parallel builds with dotnet sdk 8+, build sequentially for older …
Browse files Browse the repository at this point in the history 
…sdks.
  • Loading branch information
@timcassell
timcassell committed Apr 15, 2024
1 parent a24d689 commit 44a79b0
Show file tree
Hide file tree
Showing 7 changed files with 80 additions and 35 deletions.
34 changes: 30 additions & 4 deletions src/BenchmarkDotNet/Running/BenchmarkRunnerClean.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
using BenchmarkDotNet.Portability;
using BenchmarkDotNet.Reports;
using BenchmarkDotNet.Toolchains;
using BenchmarkDotNet.Toolchains.DotNetCli;
using BenchmarkDotNet.Toolchains.Parameters;
using BenchmarkDotNet.Toolchains.Results;
using BenchmarkDotNet.Validators;
Expand Down Expand Up @@ -372,15 +373,40 @@ private static ImmutableArray<ValidationError> Validate(params BenchmarkRunInfo[

private static Dictionary<BuildPartition, BuildResult> 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[]>();
var sequentialPartitions = new List<BuildPartition>();
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);
}
else
{
parallelPartitions.Add([partition]);
}
}
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]);
}

logger.WriteLineHeader($"// ***** Building {parallelPartitions.Count} exe(s) in Parallel{(sequentialPartitions.Count <= 1 ? "" : $", {sequentialPartitions.Count} sequentially")} *****");

var buildLogger = buildPartitions.Length == 1 ? logger : NullLogger.Instance; // when we have just one partition we can print to std out
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 buildResults = parallelPartitions
.AsParallel()
.Select(buildPartition => (Partition: buildPartition, Result: Build(buildPartition, rootArtifactsFolderPath, buildLogger)))
.SelectMany(partitions => partitions
.Select(buildPartition => (Partition: buildPartition, Result: Build(buildPartition, rootArtifactsFolderPath, buildLogger))))
.AsSequential() // Ensure that build completion events are processed sequentially
.Select(build =>
{
Expand Down
14 changes: 8 additions & 6 deletions src/BenchmarkDotNet/Toolchains/DotNetCli/DotNetCliBuilder.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,23 +7,25 @@

namespace BenchmarkDotNet.Toolchains.DotNetCli
{
[PublicAPI]
public class DotNetCliBuilder : IBuilder
public abstract class DotNetCliBuilderBase : IBuilder
{
private string TargetFrameworkMoniker { get; }
public string? CustomDotNetCliPath { get; protected set; }
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)
public override BuildResult Build(GenerateResult generateResult, BuildPartition buildPartition, ILogger logger)
{
BuildResult buildResult = new DotNetCliCommand(
CustomDotNetCliPath,
Expand Down
38 changes: 23 additions & 15 deletions src/BenchmarkDotNet/Toolchains/DotNetCli/DotNetCliCommand.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -71,12 +72,12 @@ public BuildResult RestoreThenBuild()
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(), $"{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(), $"{Arguments} --no-restore --no-dependencies", "build-no-restore-no-deps", excludeOutput: true)))
.ToBuildResult(GenerateResult);
}
else
Expand Down Expand Up @@ -128,61 +129,66 @@ public DotNetCliCommandResult AddPackages()
return DotNetCliCommandResult.Success(executionTime, stdOutput.ToString());
}

private bool GetWithArtifactsPath() => DotNetCliCommandExecutor.DotNetSdkSupportsArtifactsPath(CliPath);

public DotNetCliCommandResult Restore()
=> DotNetCliCommandExecutor.Execute(WithArguments(
GetRestoreCommand(GenerateResult.ArtifactsPaths, BuildPartition, Arguments, "restore")));
GetRestoreCommand(GenerateResult.ArtifactsPaths, BuildPartition, GetWithArtifactsPath(), Arguments, "restore")));

public DotNetCliCommandResult Build()
=> DotNetCliCommandExecutor.Execute(WithArguments(
GetBuildCommand(GenerateResult.ArtifactsPaths, BuildPartition, Arguments, "build")));
GetBuildCommand(GenerateResult.ArtifactsPaths, BuildPartition, GetWithArtifactsPath(), Arguments, "build")));

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

public DotNetCliCommandResult Publish()
=> DotNetCliCommandExecutor.Execute(WithArguments(
GetPublishCommand(GenerateResult.ArtifactsPaths, BuildPartition, Arguments, "publish")));
GetPublishCommand(GenerateResult.ArtifactsPaths, BuildPartition, GetWithArtifactsPath(), 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()
=> DotNetCliCommandExecutor.Execute(WithArguments(
GetPublishCommand(GenerateResult.ArtifactsPaths, BuildPartition, $"{Arguments} --no-restore", "publish-no-restore")));
GetPublishCommand(GenerateResult.ArtifactsPaths, BuildPartition, GetWithArtifactsPath(), $"{Arguments} --no-restore", "publish-no-restore")));

internal static IEnumerable<string> 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}\"")
.AppendArgument(GetCustomMsBuildArguments(buildPartition.RepresentativeBenchmarkCase, buildPartition.Resolver))
.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))
.AppendArgument(extraArguments)
.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))
.AppendArgument(extraArguments)
.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)
Expand Down Expand Up @@ -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
Expand All @@ -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}\"");
}
}
15 changes: 13 additions & 2 deletions src/BenchmarkDotNet/Toolchains/DotNetCli/DotNetCliCommandExecutor.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
using System.Linq;
using System.Text;
using System.Text.RegularExpressions;
using BenchmarkDotNet.Environments;
using BenchmarkDotNet.Extensions;
using BenchmarkDotNet.Helpers;
using BenchmarkDotNet.Jobs;
Expand Down Expand Up @@ -54,9 +55,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") })
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") })
using (new ConsoleExitHandler(process, NullLogger.Instance))
{
try
Expand Down
4 changes: 2 additions & 2 deletions src/BenchmarkDotNet/Toolchains/DotNetCli/DotNetCliGenerator.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -101,8 +101,8 @@ protected override void CopyAllRequiredFiles(ArtifactsPaths artifactsPaths)
protected override void GenerateBuildScript(BuildPartition buildPartition, ArtifactsPaths artifactsPaths)
{
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, DotNetCliCommandExecutor.DotNetSdkSupportsArtifactsPath(CliPath))}")
.AppendLine($"call {CliPath ?? "dotnet"} {DotNetCliCommand.GetBuildCommand(artifactsPaths, buildPartition, DotNetCliCommandExecutor.DotNetSdkSupportsArtifactsPath(CliPath))}")
.ToString();

File.WriteAllText(artifactsPaths.BuildScriptFilePath, content);
Expand Down
6 changes: 2 additions & 4 deletions src/BenchmarkDotNet/Toolchains/DotNetCli/DotNetCliPublisher.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@

namespace BenchmarkDotNet.Toolchains.DotNetCli
{
public class DotNetCliPublisher : IBuilder
public class DotNetCliPublisher : DotNetCliBuilderBase
{
public DotNetCliPublisher(
string? customDotNetCliPath = null,
Expand All @@ -18,13 +18,11 @@ public class DotNetCliPublisher : IBuilder
EnvironmentVariables = environmentVariables;
}

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,
Expand Down
4 changes: 2 additions & 2 deletions src/BenchmarkDotNet/Toolchains/NativeAot/Generator.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -72,8 +72,8 @@ protected override void GenerateBuildScript(BuildPartition buildPartition, Artif
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, DotNetCliCommandExecutor.DotNetSdkSupportsArtifactsPath(CliPath), extraArguments)}")
.AppendLine($"call {CliPath ?? "dotnet"} {DotNetCliCommand.GetPublishCommand(artifactsPaths, buildPartition, DotNetCliCommandExecutor.DotNetSdkSupportsArtifactsPath(CliPath), extraArguments)}")
.ToString();

File.WriteAllText(artifactsPaths.BuildScriptFilePath, content);
Expand Down

0 comments on commit 44a79b0

Please sign in to comment.