Skip to content

Rename MicroBenchmarks.Serializers.BinaryData to BinaryDataPayload#5216

Merged
DrewScoggins merged 1 commit intodotnet:mainfrom
DrewScoggins:fix/rename-binarydata-payload
May 7, 2026
Merged

Rename MicroBenchmarks.Serializers.BinaryData to BinaryDataPayload#5216
DrewScoggins merged 1 commit intodotnet:mainfrom
DrewScoggins:fix/rename-binarydata-payload

Conversation

@DrewScoggins
Copy link
Copy Markdown
Member

@DrewScoggins DrewScoggins commented May 7, 2026

Description from copilot.

The serialization-test helper class MicroBenchmarks.Serializers.BinaryData shadowed the in-box System.BinaryData type (introduced in net6+ via System.Memory.Data, brought in transitively by Azure.Core).

Inside DataGenerator.cs (namespace MicroBenchmarks.Serializers) the unqualified typeof(BinaryData) always bound to the local helper class. Inside ReadJson.cs / WriteJson.cs (namespace System.Text.Json.Serialization.Tests) C# enclosing-namespace lookup found System.BinaryData first, so the [GenericTypeArguments(typeof(BinaryData))] attribute resolved to a DIFFERENT type than the Generate<T>() switch was checking. The mismatch was latent on main (where System.Memory.Data wasn't compile-visible to ReadJson.cs) but surfaced as System.NotImplementedException in DataGenerator.Generate<T>() whenever the reference closure changed (e.g. retargeting BenchmarkDotNet.Extensions from netstandard2.0 to net8.0).

Renaming the helper class to BinaryDataPayload removes the shadowing once and for all. No behavior change on main; future-proofs against reference-closure churn.

The serialization-test helper class `MicroBenchmarks.Serializers.BinaryData`
shadowed the in-box `System.BinaryData` type (introduced in net6+ via
`System.Memory.Data`, brought in transitively by Azure.Core).

Inside `DataGenerator.cs` (namespace MicroBenchmarks.Serializers) the
unqualified `typeof(BinaryData)` always bound to the local helper class.
Inside `ReadJson.cs` / `WriteJson.cs` (namespace System.Text.Json.Serialization.Tests)
C# enclosing-namespace lookup found `System.BinaryData` first, so the
`[GenericTypeArguments(typeof(BinaryData))]` attribute resolved to a
DIFFERENT type than the `Generate<T>()` switch was checking. The mismatch
was latent on main (where System.Memory.Data wasn't compile-visible to
ReadJson.cs) but surfaced as `System.NotImplementedException` in
`DataGenerator.Generate<T>()` whenever the reference closure changed
(e.g. retargeting BenchmarkDotNet.Extensions from netstandard2.0 to net8.0).

Renaming the helper class to `BinaryDataPayload` removes the shadowing
once and for all. No behavior change on main; future-proofs against
reference-closure churn.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This pull request renames the microbenchmark serialization helper type MicroBenchmarks.Serializers.BinaryData to BinaryDataPayload to avoid shadowing System.BinaryData (net6+), which could cause GenericTypeArguments to bind to the wrong type and trigger runtime NotImplementedException in DataGenerator.Generate<T>().

Changes:

  • Renamed the local helper model BinaryData to BinaryDataPayload in DataGenerator.cs.
  • Updated DataGenerator.Generate<T>() and JSON source-generator registration to use BinaryDataPayload.
  • Updated ReadJson<T> / WriteJson<T> benchmark generic type argument lists to reference BinaryDataPayload.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.

File Description
src/benchmarks/micro/Serializers/DataGenerator.cs Renames the helper model and updates generator/type registrations to eliminate System.BinaryData shadowing issues.
src/benchmarks/micro/libraries/System.Text.Json/Serializer/WriteJson.cs Updates benchmark generic type argument from BinaryData to BinaryDataPayload.
src/benchmarks/micro/libraries/System.Text.Json/Serializer/ReadJson.cs Updates benchmark generic type argument from BinaryData to BinaryDataPayload.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@DrewScoggins DrewScoggins enabled auto-merge (squash) May 7, 2026 19:43
@DrewScoggins DrewScoggins disabled auto-merge May 7, 2026 20:25
@DrewScoggins DrewScoggins merged commit a836298 into dotnet:main May 7, 2026
75 of 79 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants