Skip to content

Commit 61beb50

Browse files
committed
Avoid a race condition during BDN build of dotnet/performance benchmarks
1 parent 82d2cd0 commit 61beb50

1 file changed

Lines changed: 43 additions & 11 deletions

File tree

Runner/Jobs/BenchmarkLibrariesJob.cs

Lines changed: 43 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -80,7 +80,6 @@ private async Task CloneDotnetPerformanceAndSetupToolsAsync()
8080

8181
await RunProcessAsync("git", $"clone --no-tags --depth=1 -b {branch} --progress https://github.com/{repo} performance", logPrefix: "Clone performance");
8282

83-
if (TryGetFlag("medium") || TryGetFlag("long"))
8483
{
8584
string? path = Directory.EnumerateFiles("performance", "*.cs", SearchOption.AllDirectories)
8685
.FirstOrDefault(f => f.EndsWith("RecommendedConfig.cs", StringComparison.Ordinal));
@@ -91,22 +90,55 @@ private async Task CloneDotnetPerformanceAndSetupToolsAsync()
9190
}
9291
else
9392
{
94-
string jobType = $"Job.{(TryGetFlag("medium") ? "Medium" : "Long")}Run";
95-
9693
string source = File.ReadAllText(path);
9794

98-
// https://github.com/dotnet/performance/blob/d76d845972c2a231f31e88802457d8aaf7263a5b/src/harness/BenchmarkDotNet.Extensions/RecommendedConfig.cs#L53
99-
string newSource = source.Replace(".AddJob(job", $".AddJob((job.ToString().Length >= 0 ? {jobType} : job)", StringComparison.Ordinal);
100-
101-
if (source == newSource)
95+
// Disable BenchmarkDotNet's parallel build of the generated benchmark host projects.
96+
// Every generated host project has a ProjectReference to the shared MicroBenchmarks project,
97+
// whose assembly is post-processed by BDN's MSBuild weaver (BenchmarkDotNet.Weaver) to add
98+
// [MethodImpl(MethodImplOptions.NoInlining)] to every [Benchmark] method. We always build at
99+
// least two partitions (e.g. one per corerun we compare), and BDN builds partitions in
100+
// parallel by default. Concurrent builds race on the shared project's intermediate assembly,
101+
// which makes the weaver fail (it only logs a warning because we set TreatWarningsAsErrors to
102+
// false above) and leaves an un-woven assembly behind. That assembly later fails the run with
103+
// "Benchmark method `X` does not have MethodImplOptions.NoInlining flag set." Building the
104+
// partitions sequentially avoids the race.
102105
{
103-
await LogAsync("Failed to find the existing Job type");
106+
const string Marker = "ManualConfig.CreateEmpty()";
107+
string newSource = source.Replace(
108+
Marker,
109+
$"{Marker}.WithOption(BenchmarkDotNet.Configs.ConfigOptions.DisableParallelBuild, true)",
110+
StringComparison.Ordinal);
111+
112+
if (source == newSource)
113+
{
114+
await LogAsync("Failed to disable BenchmarkDotNet parallel build in RecommendedConfig.cs");
115+
}
116+
else
117+
{
118+
source = newSource;
119+
await LogAsync("Disabled BenchmarkDotNet parallel build to avoid the assembly weaving race");
120+
}
104121
}
105-
else
122+
123+
if (TryGetFlag("medium") || TryGetFlag("long"))
106124
{
107-
File.WriteAllText(path, newSource);
108-
await LogAsync($"Replaced Job type with {jobType}");
125+
string jobType = $"Job.{(TryGetFlag("medium") ? "Medium" : "Long")}Run";
126+
127+
// https://github.com/dotnet/performance/blob/d76d845972c2a231f31e88802457d8aaf7263a5b/src/harness/BenchmarkDotNet.Extensions/RecommendedConfig.cs#L53
128+
string newSource = source.Replace(".AddJob(job", $".AddJob((job.ToString().Length >= 0 ? {jobType} : job)", StringComparison.Ordinal);
129+
130+
if (source == newSource)
131+
{
132+
await LogAsync("Failed to find the existing Job type");
133+
}
134+
else
135+
{
136+
source = newSource;
137+
await LogAsync($"Replaced Job type with {jobType}");
138+
}
109139
}
140+
141+
File.WriteAllText(path, source);
110142
}
111143
}
112144

0 commit comments

Comments
 (0)