Update bdn 0.16 binarydata and async#5217
Merged
DrewScoggins merged 6 commits intoMay 19, 2026
Merged
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
This PR updates the performance harness and microbenchmark suite to work with newer BenchmarkDotNet (0.16 nightly), including BDN’s new async validator APIs and addressing a BinaryData naming collision, while also adding an experiment-gated runtime-async feature flag for net11+ runs.
Changes:
- Update BenchmarkDotNet version and migrate custom validators to
ValidateAsyncreturningIAsyncEnumerable<ValidationError>. - Rename microbenchmarks’
BinaryDatapayload type to avoid collision withSystem.BinaryData. - Adjust harness exporter implementation for BDN 0.16’s exporter API changes and add an experiment-gated
runtime-asyncMSBuild feature flag.
Reviewed changes
Copilot reviewed 18 out of 19 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| src/tests/harness/BenchmarkDotNet.Extensions.Tests/UniqueArgumentsValidatorTests.cs | Switch test to use async validator API. |
| src/harness/BenchmarkDotNet.Extensions/ValuesGenerator.cs | Add notnull constraint to dictionary key generation to align with modern TFMs. |
| src/harness/BenchmarkDotNet.Extensions/UniqueArgumentsValidator.cs | Migrate to ValidateAsync and return IAsyncEnumerable. |
| src/harness/BenchmarkDotNet.Extensions/TooManyTestCasesValidator.cs | Migrate to ValidateAsync and nullability updates. |
| src/harness/BenchmarkDotNet.Extensions/NoWasmValidator.cs | Migrate to ValidateAsync. |
| src/harness/BenchmarkDotNet.Extensions/MandatoryCategoryValidator.cs | Migrate to ValidateAsync. |
| src/harness/BenchmarkDotNet.Extensions/PerfLabExporter.cs | Rework exporter to implement IExporter and write JSON artifact asynchronously. |
| src/harness/BenchmarkDotNet.Extensions/DiffableDisassemblyExporter.cs | Add null-forgiving operators for reflection results. |
| src/harness/BenchmarkDotNet.Extensions/BenchmarkDotNet.Extensions.csproj | Retarget harness to net8.0 and adjust BDN source reference TFM. |
| src/Directory.Build.targets | Add experiment-gated runtime-async feature flag for net11+. |
| src/Directory.Build.props | Whitespace-only formatting adjustments. |
| src/benchmarks/micro/Serializers/DataGenerator.cs | Rename BinaryData to BinaryDataPayload and update generation/metadata. |
| src/benchmarks/micro/libraries/System.Text.Json/Serializer/WriteJson.cs | Update generic args to BinaryDataPayload. |
| src/benchmarks/micro/libraries/System.Text.Json/Serializer/ReadJson.cs | Update generic args to BinaryDataPayload. |
| src/benchmarks/micro/README.md | Update documented “available” TFMs to net8.0+. |
| src/benchmarks/micro/MicroBenchmarks.csproj | Remove net472-only package references block. |
| scripts/ci_setup.py | Add runtimeasync experiment env-var plumbing (EnableRuntimeAsync=true). |
| eng/Versions.props | Bump BenchmarkDotNetVersion to a newer nightly. |
| docs/benchmarkdotnet.md | Update runtime options list and remove outdated/private CLR-build guidance. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
c2fb987 to
57d3cd2
Compare
57d3cd2 to
fbbc502
Compare
| @@ -29,10 +29,11 @@ public IEnumerable<ValidationError> Validate(ValidationParameters validationPara | |||
| new ValidationError( | |||
| isCritical: true, | |||
| message: $"{group.Key.Descriptor.Type.Name}.{group.Key.Descriptor.WorkloadMethod.Name} has {group.Count()} test cases. It MUST NOT have more than {Limit} test cases. We don't have inifinite amount of time to run all the benchmarks!!", | |||
LoopedBard3
previously approved these changes
May 7, 2026
Member
LoopedBard3
left a comment
There was a problem hiding this comment.
Looks good if the test runs succeed 👍.
timcassell
reviewed
May 7, 2026
fbbc502 to
2ee9131
Compare
Open
3 tasks
Bumps BenchmarkDotNet to the latest master nightly, built from dotnet/BenchmarkDotNet@c7632225 ("Disassembly follow jump trampolines (#3136)") and pushed to the benchmark-dotnet-prerelease internal feed. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
BDN 0.16.0 ('Async Refactor', PR #2958) made breaking API changes that
the harness still consumed:
- ExporterBase.ExportToLog(Summary, ILogger) was removed; the new abstract
ExportAsync uses an internal CancelableStreamWriter that subclasses can't
satisfy. PerfLabExporter now implements IExporter directly.
- IValidator.Validate returning IEnumerable<ValidationError> was replaced
with ValidateAsync returning IAsyncEnumerable<ValidationError>. The four
validators (UniqueArguments, TooManyTestCases, NoWasm, MandatoryCategory)
use .ToAsyncEnumerable() (transitively from BDN's
System.Linq.AsyncEnumerable dep), not async + yield return - the latter
produces an AsyncIteratorMethodBuilder state machine that deadlocks with
BDN's BenchmarkSynchronizationContext.
Switched BenchmarkDotNet.Extensions from netstandard2.0 to net8.0. The
netstandard2.0 target was only used to enable an opt-in net472 path, but
PERFLAB_TARGET_FRAMEWORKS=net472 is no longer exercised in CI and the new
BDN APIs would otherwise require polyfill packages. Removed the net472
package conditional from MicroBenchmarks.csproj and the net472/.NET
Framework references from docs/benchmarkdotnet.md and the micro README.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Switching BenchmarkDotNet.Extensions from netstandard2.0 to net8.0 brought in stricter nullable annotations from the BCL and BDN package, which combined with TreatWarningsAsErrors=true (set in src/Directory.Build.props) turned previously-hidden nullability warnings into build errors. - ValuesGenerator.Dictionary<TKey, TValue>: add 'where TKey : notnull' constraint required by Dictionary<,>. - UniqueArgumentsValidator.BenchmarkArgumentsComparer.Equals: match the IEqualityComparer<T>.Equals(T?, T?) interface signature; handle nulls. - TooManyTestCasesValidator.SkipValidation: parameter must be MemberInfo? since MemberInfo.DeclaringType returns Type?. - DiffableDisassemblyExporter: add null-forgiving (!) on reflection lookups whose return values are intentionally trusted by this type (it's a copy of internal BDN code that operates on known types). - PerfLabExporter: BuildJson can return null; use string?. Verified by running the exact CI commands locally (dotnet restore + build with --framework net11.0) using the SDK from global.json. Build succeeds with 0 errors. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
BDN 0.16's sync entrypoints (BenchmarkSwitcher.Run / BenchmarkRunner.Run) install BenchmarkDotNetSynchronizationContext (a single-threaded message pump) before benchmark discovery. Discovery executes [ParamsSource] and [ArgumentsSource] callbacks; some perf-repo callbacks do sync-over-async (notably SslStreamTests.GetTls13Support, which calls HandshakeAsync(...) .GetAwaiter().GetResult()). Sync-over-async deadlocks on the single-threaded SyncCtx because the awaited continuation is queued back to a pump that the caller is blocking. Switch Program.Main to async Task<int> and await BenchmarkSwitcher.RunAsync. The async entrypoint never installs BenchmarkDotNetSynchronizationContext on the caller, so discovery runs on the default context and sync-over-async in source callbacks no longer deadlocks. Real benchmark execution still gets the SyncCtx semantics it needs because BDN installs it inside the per-benchmark execute path, not on the entrypoint thread. This is the supported BDN-recommended fix for the discovery-deadlock symptom; no BDN code change is required. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
PR #5195 enabled the runtime-async language feature unconditionally for net11.0+ via `<Features>`. To avoid affecting baseline measurement runs, gate it behind the existing experiment infrastructure so it only takes effect in the dedicated experiment lane. - src/Directory.Build.targets: only set `runtime-async=on` when the `EnableRuntimeAsync` MSBuild property is `true` (in addition to the existing TFM check). - scripts/ci_setup.py: when `--experiment-name=runtimeasync` is passed, emit `EnableRuntimeAsync=true` as an env var so MSBuild picks it up as a property (matches the pattern used by the `jitoptrepeat` experiment). Verified locally: BDN dry runs against the BinaryDataPayload tests succeed both with and without the env var (18/18 benchmarks each). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
2ee9131 to
e3e6736
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This change enables runtime-async. It is not on by default, but must be activated by an experiment. The runs will be added in a separate PR.
This change also includes updating BDN to latest as it was required for runtime-async.