Skip to content

Commit

Permalink
Don't use ArtifactsPath for wasm and monoaotllvm.
Browse files Browse the repository at this point in the history 
MonoPublisher inherits from DotNetCliBuilderBase.
Don't repeat sequential builds.
  • Loading branch information
@timcassell
timcassell committed Apr 17, 2024
1 parent 44a79b0 commit 3285f50
Show file tree
Hide file tree
Showing 13 changed files with 86 additions and 66 deletions.
46 changes: 22 additions & 24 deletions src/BenchmarkDotNet/Running/BenchmarkRunnerClean.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -373,22 +372,17 @@ private static ImmutableArray<ValidationError> Validate(params BenchmarkRunInfo[

private static Dictionary<BuildPartition, BuildResult> BuildInParallel(ILogger logger, string rootArtifactsFolderPath, BuildPartition[] buildPartitions, in StartedClock globalChronometer, EventProcessor eventProcessor)
{
var parallelPartitions = new List<BuildPartition[]>();
var sequentialPartitions = new List<BuildPartition>();
var parallelPartitions = new List<(BuildPartition buildPartition, IToolchain toolchain, bool isSequential)[]>();
var sequentialPartitions = new List<(BuildPartition, IToolchain, bool)>();
foreach (var partition in buildPartitions)
{
// If dotnet sdk does not support ArtifactsPath, it's unsafe to build the same project in parallel.
// We cannot rely on falling back to sequential after failure, because the builds can be corrupted.
// Therefore we have to build them sequentially. #2425
if (partition.RepresentativeBenchmarkCase.GetToolchain().Builder is DotNetCliBuilderBase dotnetBuilder
&& !DotNetCliCommandExecutor.DotNetSdkSupportsArtifactsPath(dotnetBuilder.CustomDotNetCliPath))
{
sequentialPartitions.Add(partition);
}
// It's guaranteed that all the benchmarks in single partition have same toolchain.
var toolchain = partition.RepresentativeBenchmarkCase.GetToolchain();
// #2425
if (toolchain.IsSafeToBuildInParallel())
parallelPartitions.Add([(partition, toolchain, false)]);
else
{
parallelPartitions.Add([partition]);
}
sequentialPartitions.Add((partition, toolchain, true));
}
if (sequentialPartitions.Count > 0)
{
Expand All @@ -403,37 +397,43 @@ private static ImmutableArray<ValidationError> 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]);
}
Expand All @@ -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
Expand Down
4 changes: 2 additions & 2 deletions src/BenchmarkDotNet/Toolchains/CsProj/CsProjGenerator.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -40,8 +40,8 @@ public class CsProjGenerator : DotNetCliGenerator, IEquatable<CsProjGenerator>

public string RuntimeFrameworkVersion { get; }

public CsProjGenerator(string targetFrameworkMoniker, string cliPath, string packagesPath, string runtimeFrameworkVersion, bool isNetCore = true)
: base(targetFrameworkMoniker, cliPath, packagesPath, isNetCore)
public CsProjGenerator(string targetFrameworkMoniker, string cliPath, string packagesPath, string runtimeFrameworkVersion, bool isNetCore = true, bool useArtifactsPathIfSupported = true)
: base(targetFrameworkMoniker, cliPath, packagesPath, isNetCore, useArtifactsPathIfSupported)
{
RuntimeFrameworkVersion = runtimeFrameworkVersion;
}
Expand Down
10 changes: 9 additions & 1 deletion src/BenchmarkDotNet/Toolchains/DotNetCli/DotNetCliBuilder.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ namespace BenchmarkDotNet.Toolchains.DotNetCli
public abstract class DotNetCliBuilderBase : IBuilder
{
public string? CustomDotNetCliPath { get; protected set; }
internal bool UseArtifactsPathIfSupported { get; private protected set; } = true;
public abstract BuildResult Build(GenerateResult generateResult, BuildPartition buildPartition, ILogger logger);
}

Expand All @@ -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(
Expand All @@ -36,7 +44,7 @@ public override BuildResult Build(GenerateResult generateResult, BuildPartition
Array.Empty<EnvironmentVariable>(),
buildPartition.Timeout,
logOutput: LogOutput)
.RestoreThenBuild();
.RestoreThenBuild(UseArtifactsPathIfSupported);
if (buildResult.IsBuildSuccess &&
buildPartition.RepresentativeBenchmarkCase.Job.Environment.LargeAddressAware)
{
Expand Down
42 changes: 21 additions & 21 deletions src/BenchmarkDotNet/Toolchains/DotNetCli/DotNetCliCommand.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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));

Expand All @@ -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));

Expand All @@ -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()
Expand All @@ -129,28 +129,28 @@ public DotNetCliCommandResult AddPackages()
return DotNetCliCommandResult.Success(executionTime, stdOutput.ToString());
}

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

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

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

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

public DotNetCliCommandResult Publish()
public DotNetCliCommandResult Publish(bool useArtifactsPathIfSupported = true)
=> DotNetCliCommandExecutor.Execute(WithArguments(
GetPublishCommand(GenerateResult.ArtifactsPaths, BuildPartition, GetWithArtifactsPath(), Arguments, "publish")));
GetPublishCommand(GenerateResult.ArtifactsPaths, BuildPartition, GetWithArtifactsPath(useArtifactsPathIfSupported), Arguments, "publish")));

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

internal static IEnumerable<string> GetAddPackagesCommands(BuildPartition buildPartition)
=> GetNuGetAddPackageCommands(buildPartition.RepresentativeBenchmarkCase, buildPartition.Resolver);
Expand Down
10 changes: 7 additions & 3 deletions src/BenchmarkDotNet/Toolchains/DotNetCli/DotNetCliGenerator.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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";
Expand Down Expand Up @@ -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);
Expand Down
2 changes: 1 addition & 1 deletion src/BenchmarkDotNet/Toolchains/DotNetCli/DotNetCliPublisher.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,6 @@ public override BuildResult Build(GenerateResult generateResult, BuildPartition
buildPartition,
EnvironmentVariables,
buildPartition.Timeout)
.RestoreThenBuildThenPublish();
.RestoreThenBuildThenPublish(UseArtifactsPathIfSupported);
}
}
8 changes: 3 additions & 5 deletions src/BenchmarkDotNet/Toolchains/Mono/MonoPublisher.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@

namespace BenchmarkDotNet.Toolchains.Mono
{
public class MonoPublisher : IBuilder
public class MonoPublisher : DotNetCliBuilderBase
{
public MonoPublisher(string customDotNetCliPath)
{
Expand All @@ -18,13 +18,11 @@ public MonoPublisher(string customDotNetCliPath)
ExtraArguments = $"--self-contained -r {runtimeIdentifier} /p:UseMonoRuntime=true /p:RuntimeIdentifiers={runtimeIdentifier}";
}

private string CustomDotNetCliPath { get; }

private string ExtraArguments { get; }

private IReadOnlyList<EnvironmentVariable> EnvironmentVariables { get; }

public BuildResult Build(GenerateResult generateResult, BuildPartition buildPartition, ILogger logger)
public override BuildResult Build(GenerateResult generateResult, BuildPartition buildPartition, ILogger logger)
=> new DotNetCliCommand(
CustomDotNetCliPath,
ExtraArguments,
Expand All @@ -33,6 +31,6 @@ public BuildResult Build(GenerateResult generateResult, BuildPartition buildPart
buildPartition,
EnvironmentVariables,
buildPartition.Timeout)
.Publish().ToBuildResult(generateResult);
.Publish(UseArtifactsPathIfSupported).ToBuildResult(generateResult);
}
}

0 comments on commit 3285f50

Please sign in to comment.