Skip to content

Commit 3285f50

Browse files
timcasselltimcassell
committed
Don't use ArtifactsPath for wasm and monoaotllvm.
MonoPublisher inherits from DotNetCliBuilderBase. Don't repeat sequential builds.
1 parent 44a79b0 commit 3285f50

13 files changed

+86
-66
lines changed

‎src/BenchmarkDotNet/Running/BenchmarkRunnerClean.cs

+22-24
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,6 @@
2121
using BenchmarkDotNet.Portability;
2222
using BenchmarkDotNet.Reports;
2323
using BenchmarkDotNet.Toolchains;
24-
using BenchmarkDotNet.Toolchains.DotNetCli;
2524
using BenchmarkDotNet.Toolchains.Parameters;
2625
using BenchmarkDotNet.Toolchains.Results;
2726
using BenchmarkDotNet.Validators;
@@ -373,22 +372,17 @@ private static ImmutableArray<ValidationError> Validate(params BenchmarkRunInfo[
373372

374373
private static Dictionary<BuildPartition, BuildResult> BuildInParallel(ILogger logger, string rootArtifactsFolderPath, BuildPartition[] buildPartitions, in StartedClock globalChronometer, EventProcessor eventProcessor)
375374
{
376-
var parallelPartitions = new List<BuildPartition[]>();
377-
var sequentialPartitions = new List<BuildPartition>();
375+
var parallelPartitions = new List<(BuildPartition buildPartition, IToolchain toolchain, bool isSequential)[]>();
376+
var sequentialPartitions = new List<(BuildPartition, IToolchain, bool)>();
378377
foreach (var partition in buildPartitions)
379378
{
380-
// If dotnet sdk does not support ArtifactsPath, it's unsafe to build the same project in parallel.
381-
// We cannot rely on falling back to sequential after failure, because the builds can be corrupted.
382-
// Therefore we have to build them sequentially. #2425
383-
if (partition.RepresentativeBenchmarkCase.GetToolchain().Builder is DotNetCliBuilderBase dotnetBuilder
384-
&& !DotNetCliCommandExecutor.DotNetSdkSupportsArtifactsPath(dotnetBuilder.CustomDotNetCliPath))
385-
{
386-
sequentialPartitions.Add(partition);
387-
}
379+
// It's guaranteed that all the benchmarks in single partition have same toolchain.
380+
var toolchain = partition.RepresentativeBenchmarkCase.GetToolchain();
381+
// #2425
382+
if (toolchain.IsSafeToBuildInParallel())
383+
parallelPartitions.Add([(partition, toolchain, false)]);
388384
else
389-
{
390-
parallelPartitions.Add([partition]);
391-
}
385+
sequentialPartitions.Add((partition, toolchain, true));
392386
}
393387
if (sequentialPartitions.Count > 0)
394388
{
@@ -403,37 +397,43 @@ private static Dictionary<BuildPartition, BuildResult> BuildInParallel(ILogger l
403397

404398
var beforeParallelBuild = globalChronometer.GetElapsed();
405399

406-
var buildResults = parallelPartitions
400+
var parallelBuildResults = parallelPartitions
407401
.AsParallel()
408402
.SelectMany(partitions => partitions
409-
.Select(buildPartition => (Partition: buildPartition, Result: Build(buildPartition, rootArtifactsFolderPath, buildLogger))))
403+
.Select(build => (Partition: build.buildPartition,
404+
Result: Build(build.buildPartition, build.toolchain, rootArtifactsFolderPath, buildLogger),
405+
WasSequential: build.isSequential)
406+
)
407+
)
410408
.AsSequential() // Ensure that build completion events are processed sequentially
411409
.Select(build =>
412410
{
413411
// If the generation was successful, but the build was not, we will try building sequentially
414412
// so don't send the OnBuildComplete event yet.
415-
if (buildPartitions.Length <= 1 || !build.Result.IsGenerateSuccess || build.Result.IsBuildSuccess)
413+
if (build.WasSequential || buildPartitions.Length <= 1 || !build.Result.IsGenerateSuccess || build.Result.IsBuildSuccess)
416414
eventProcessor.OnBuildComplete(build.Partition, build.Result);
417415

418416
return build;
419417
})
420-
.ToDictionary(build => build.Partition, build => build.Result);
418+
.ToArray();
421419

422420
var afterParallelBuild = globalChronometer.GetElapsed();
423421

424422
logger.WriteLineHeader($"// ***** Done, took {GetFormattedDifference(beforeParallelBuild, afterParallelBuild)} *****");
425423

426-
if (buildPartitions.Length <= 1 || !buildResults.Values.Any(result => result.IsGenerateSuccess && !result.IsBuildSuccess))
424+
var buildResults = parallelBuildResults.ToDictionary(build => build.Partition, build => build.Result);
425+
426+
if (parallelPartitions.Count <= 1 || !parallelBuildResults.Any(build => !build.WasSequential && build.Result.IsGenerateSuccess && !build.Result.IsBuildSuccess))
427427
return buildResults;
428428

429429
logger.WriteLineHeader("// ***** Failed to build in Parallel, switching to sequential build *****");
430430

431-
foreach (var buildPartition in buildPartitions)
431+
foreach (var (buildPartition, toolchain, _) in parallelPartitions.Skip(1).Select(arr => arr[0]))
432432
{
433433
if (buildResults[buildPartition].IsGenerateSuccess && !buildResults[buildPartition].IsBuildSuccess)
434434
{
435435
if (!buildResults[buildPartition].TryToExplainFailureReason(out string _))
436-
buildResults[buildPartition] = Build(buildPartition, rootArtifactsFolderPath, buildLogger);
436+
buildResults[buildPartition] = Build(buildPartition, toolchain, rootArtifactsFolderPath, buildLogger);
437437

438438
eventProcessor.OnBuildComplete(buildPartition, buildResults[buildPartition]);
439439
}
@@ -449,10 +449,8 @@ static string GetFormattedDifference(ClockSpan before, ClockSpan after)
449449
=> (after.GetTimeSpan() - before.GetTimeSpan()).ToFormattedTotalTime(DefaultCultureInfo.Instance);
450450
}
451451

452-
private static BuildResult Build(BuildPartition buildPartition, string rootArtifactsFolderPath, ILogger buildLogger)
452+
private static BuildResult Build(BuildPartition buildPartition, IToolchain toolchain, string rootArtifactsFolderPath, ILogger buildLogger)
453453
{
454-
var toolchain = buildPartition.RepresentativeBenchmarkCase.GetToolchain(); // it's guaranteed that all the benchmarks in single partition have same toolchain
455-
456454
var generateResult = toolchain.Generator.GenerateProject(buildPartition, buildLogger, rootArtifactsFolderPath);
457455

458456
try

‎src/BenchmarkDotNet/Toolchains/CsProj/CsProjGenerator.cs

+2-2
Original file line numberDiff line numberDiff line change
@@ -40,8 +40,8 @@ public class CsProjGenerator : DotNetCliGenerator, IEquatable<CsProjGenerator>
4040

4141
public string RuntimeFrameworkVersion { get; }
4242

43-
public CsProjGenerator(string targetFrameworkMoniker, string cliPath, string packagesPath, string runtimeFrameworkVersion, bool isNetCore = true)
44-
: base(targetFrameworkMoniker, cliPath, packagesPath, isNetCore)
43+
public CsProjGenerator(string targetFrameworkMoniker, string cliPath, string packagesPath, string runtimeFrameworkVersion, bool isNetCore = true, bool useArtifactsPathIfSupported = true)
44+
: base(targetFrameworkMoniker, cliPath, packagesPath, isNetCore, useArtifactsPathIfSupported)
4545
{
4646
RuntimeFrameworkVersion = runtimeFrameworkVersion;
4747
}

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

+9-1
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ namespace BenchmarkDotNet.Toolchains.DotNetCli
1010
public abstract class DotNetCliBuilderBase : IBuilder
1111
{
1212
public string? CustomDotNetCliPath { get; protected set; }
13+
internal bool UseArtifactsPathIfSupported { get; private protected set; } = true;
1314
public abstract BuildResult Build(GenerateResult generateResult, BuildPartition buildPartition, ILogger logger);
1415
}
1516

@@ -25,6 +26,13 @@ public DotNetCliBuilder(string targetFrameworkMoniker, string? customDotNetCliPa
2526
LogOutput = logOutput;
2627
}
2728

29+
internal DotNetCliBuilder(string? customDotNetCliPath, bool logOutput, bool useArtifactsPathIfSupported)
30+
{
31+
CustomDotNetCliPath = customDotNetCliPath;
32+
LogOutput = logOutput;
33+
UseArtifactsPathIfSupported = useArtifactsPathIfSupported;
34+
}
35+
2836
public override BuildResult Build(GenerateResult generateResult, BuildPartition buildPartition, ILogger logger)
2937
{
3038
BuildResult buildResult = new DotNetCliCommand(
@@ -36,7 +44,7 @@ public override BuildResult Build(GenerateResult generateResult, BuildPartition
3644
Array.Empty<EnvironmentVariable>(),
3745
buildPartition.Timeout,
3846
logOutput: LogOutput)
39-
.RestoreThenBuild();
47+
.RestoreThenBuild(UseArtifactsPathIfSupported);
4048
if (buildResult.IsBuildSuccess &&
4149
buildPartition.RepresentativeBenchmarkCase.Job.Environment.LargeAddressAware)
4250
{

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

+21-21
Original file line numberDiff line numberDiff line change
@@ -53,7 +53,7 @@ public DotNetCliCommand WithCliPath(string cliPath)
5353
=> new (cliPath, Arguments, GenerateResult, Logger, BuildPartition, EnvironmentVariables, Timeout, logOutput: LogOutput);
5454

5555
[PublicAPI]
56-
public BuildResult RestoreThenBuild()
56+
public BuildResult RestoreThenBuild(bool useArtifactsPathIfSupported = true)
5757
{
5858
DotNetCliCommandExecutor.LogEnvVars(WithArguments(null));
5959

@@ -65,35 +65,35 @@ public BuildResult RestoreThenBuild()
6565
// so when users go with custom build configuration, we must perform full build
6666
// which will internally restore for the right configuration
6767
if (BuildPartition.IsCustomBuildConfiguration)
68-
return Build().ToBuildResult(GenerateResult);
68+
return Build(useArtifactsPathIfSupported).ToBuildResult(GenerateResult);
6969

7070
// On our CI, Integration tests take too much time, because each benchmark run rebuilds BenchmarkDotNet itself.
7171
// To reduce the total duration of the CI workflows, we build all the projects without dependencies
7272
if (BuildPartition.ForcedNoDependenciesForIntegrationTests)
7373
{
7474
var restoreResult = DotNetCliCommandExecutor.Execute(WithArguments(
75-
GetRestoreCommand(GenerateResult.ArtifactsPaths, BuildPartition, GetWithArtifactsPath(), $"{Arguments} --no-dependencies", "restore-no-deps", excludeOutput: true)));
75+
GetRestoreCommand(GenerateResult.ArtifactsPaths, BuildPartition, GetWithArtifactsPath(useArtifactsPathIfSupported), $"{Arguments} --no-dependencies", "restore-no-deps", excludeOutput: true)));
7676
if (!restoreResult.IsSuccess)
7777
return BuildResult.Failure(GenerateResult, restoreResult.AllInformation);
7878

7979
return DotNetCliCommandExecutor.Execute(WithArguments(
80-
GetBuildCommand(GenerateResult.ArtifactsPaths, BuildPartition, GetWithArtifactsPath(), $"{Arguments} --no-restore --no-dependencies", "build-no-restore-no-deps", excludeOutput: true)))
80+
GetBuildCommand(GenerateResult.ArtifactsPaths, BuildPartition, GetWithArtifactsPath(useArtifactsPathIfSupported), $"{Arguments} --no-restore --no-dependencies", "build-no-restore-no-deps", excludeOutput: true)))
8181
.ToBuildResult(GenerateResult);
8282
}
8383
else
8484
{
85-
var restoreResult = Restore();
85+
var restoreResult = Restore(useArtifactsPathIfSupported);
8686
if (!restoreResult.IsSuccess)
8787
return BuildResult.Failure(GenerateResult, restoreResult.AllInformation);
8888

8989
// We no longer retry with --no-dependencies, because it fails with --output set at the same time,
9090
// and the artifactsPaths.BinariesDirectoryPath is set before we try to build, so we cannot overwrite it.
91-
return BuildNoRestore().ToBuildResult(GenerateResult);
91+
return BuildNoRestore(useArtifactsPathIfSupported).ToBuildResult(GenerateResult);
9292
}
9393
}
9494

9595
[PublicAPI]
96-
public BuildResult RestoreThenBuildThenPublish()
96+
public BuildResult RestoreThenBuildThenPublish(bool useArtifactsPathIfSupported = true)
9797
{
9898
DotNetCliCommandExecutor.LogEnvVars(WithArguments(null));
9999

@@ -105,14 +105,14 @@ public BuildResult RestoreThenBuildThenPublish()
105105
// so when users go with custom build configuration, we must perform full publish
106106
// which will internally restore and build for the right configuration
107107
if (BuildPartition.IsCustomBuildConfiguration)
108-
return Publish().ToBuildResult(GenerateResult);
108+
return Publish(useArtifactsPathIfSupported).ToBuildResult(GenerateResult);
109109

110-
var restoreResult = Restore();
110+
var restoreResult = Restore(useArtifactsPathIfSupported);
111111
if (!restoreResult.IsSuccess)
112112
return BuildResult.Failure(GenerateResult, restoreResult.AllInformation);
113113

114114
// We use the implicit build in the publish command. We stopped doing a separate build step because we set the --output.
115-
return PublishNoRestore().ToBuildResult(GenerateResult);
115+
return PublishNoRestore(useArtifactsPathIfSupported).ToBuildResult(GenerateResult);
116116
}
117117

118118
public DotNetCliCommandResult AddPackages()
@@ -129,28 +129,28 @@ public DotNetCliCommandResult AddPackages()
129129
return DotNetCliCommandResult.Success(executionTime, stdOutput.ToString());
130130
}
131131

132-
private bool GetWithArtifactsPath() => DotNetCliCommandExecutor.DotNetSdkSupportsArtifactsPath(CliPath);
132+
private bool GetWithArtifactsPath(bool useArtifactsPathIfSupported) => useArtifactsPathIfSupported && DotNetCliCommandExecutor.DotNetSdkSupportsArtifactsPath(CliPath);
133133

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

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

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

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

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

155155
internal static IEnumerable<string> GetAddPackagesCommands(BuildPartition buildPartition)
156156
=> GetNuGetAddPackageCommands(buildPartition.RepresentativeBenchmarkCase, buildPartition.Resolver);

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

+7-3
Original file line numberDiff line numberDiff line change
@@ -20,13 +20,16 @@ public abstract class DotNetCliGenerator : GeneratorBase
2020

2121
protected bool IsNetCore { get; }
2222

23+
protected bool UseArtifactsPathIfSupported { get; set; }
24+
2325
[PublicAPI]
24-
protected DotNetCliGenerator(string targetFrameworkMoniker, string cliPath, string packagesPath, bool isNetCore)
26+
protected DotNetCliGenerator(string targetFrameworkMoniker, string cliPath, string packagesPath, bool isNetCore, bool useArtifactsPathIfSupported = true)
2527
{
2628
TargetFrameworkMoniker = targetFrameworkMoniker;
2729
CliPath = cliPath;
2830
PackagesPath = packagesPath;
2931
IsNetCore = isNetCore;
32+
UseArtifactsPathIfSupported = useArtifactsPathIfSupported;
3033
}
3134

3235
protected override string GetExecutableExtension() => IsNetCore ? ".dll" : ".exe";
@@ -100,9 +103,10 @@ protected override void CopyAllRequiredFiles(ArtifactsPaths artifactsPaths)
100103

101104
protected override void GenerateBuildScript(BuildPartition buildPartition, ArtifactsPaths artifactsPaths)
102105
{
106+
bool useArtifactsPath = UseArtifactsPathIfSupported && DotNetCliCommandExecutor.DotNetSdkSupportsArtifactsPath(CliPath);
103107
var content = new StringBuilder(300)
104-
.AppendLine($"call {CliPath ?? "dotnet"} {DotNetCliCommand.GetRestoreCommand(artifactsPaths, buildPartition, DotNetCliCommandExecutor.DotNetSdkSupportsArtifactsPath(CliPath))}")
105-
.AppendLine($"call {CliPath ?? "dotnet"} {DotNetCliCommand.GetBuildCommand(artifactsPaths, buildPartition, DotNetCliCommandExecutor.DotNetSdkSupportsArtifactsPath(CliPath))}")
108+
.AppendLine($"call {CliPath ?? "dotnet"} {DotNetCliCommand.GetRestoreCommand(artifactsPaths, buildPartition, useArtifactsPath)}")
109+
.AppendLine($"call {CliPath ?? "dotnet"} {DotNetCliCommand.GetBuildCommand(artifactsPaths, buildPartition, useArtifactsPath)}")
106110
.ToString();
107111

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

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

+1-1
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,6 @@ public override BuildResult Build(GenerateResult generateResult, BuildPartition
3131
buildPartition,
3232
EnvironmentVariables,
3333
buildPartition.Timeout)
34-
.RestoreThenBuildThenPublish();
34+
.RestoreThenBuildThenPublish(UseArtifactsPathIfSupported);
3535
}
3636
}

‎src/BenchmarkDotNet/Toolchains/Mono/MonoPublisher.cs

+3-5
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@
77

88
namespace BenchmarkDotNet.Toolchains.Mono
99
{
10-
public class MonoPublisher : IBuilder
10+
public class MonoPublisher : DotNetCliBuilderBase
1111
{
1212
public MonoPublisher(string customDotNetCliPath)
1313
{
@@ -18,13 +18,11 @@ public MonoPublisher(string customDotNetCliPath)
1818
ExtraArguments = $"--self-contained -r {runtimeIdentifier} /p:UseMonoRuntime=true /p:RuntimeIdentifiers={runtimeIdentifier}";
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,
@@ -33,6 +31,6 @@ public BuildResult Build(GenerateResult generateResult, BuildPartition buildPart
3331
buildPartition,
3432
EnvironmentVariables,
3533
buildPartition.Timeout)
36-
.Publish().ToBuildResult(generateResult);
34+
.Publish(UseArtifactsPathIfSupported).ToBuildResult(generateResult);
3735
}
3836
}

0 commit comments

Comments
 (0)