Skip to content

Use Volatile.ReadBarrier in JsonArray/JsonObject.GetUnderlyingReprese… - #136156

Merged
eiriktsarpalis merged 2 commits into
dotnet:mainfrom
asylkhan-azat:jsonnode-relax-barrier
Oct 21, 2026
Merged

Use Volatile.ReadBarrier in JsonArray/JsonObject.GetUnderlyingReprese…#136156
eiriktsarpalis merged 2 commits into
dotnet:mainfrom
asylkhan-azat:jsonnode-relax-barrier

Conversation

@asylkhan-azat

@asylkhan-azat asylkhan-azat commented Oct 20, 2026

Copy link
Copy Markdown
Contributor

Use Volatile.ReadBarrier instead of Interlocked.MemoryBarrier in JsonArray/JsonObject.GetUnderlyingRepresentation on .NET 10+

@dotnet-policy-service dotnet-policy-service Bot added the community-contribution Indicates that the PR has been added by a community member label Oct 20, 2026
@azure-pipelines

azure-pipelines Bot commented Oct 20, 2026

Copy link
Copy Markdown
Azure Pipelines:
Successfully started running 3 pipeline(s).
13 pipeline(s) were filtered out due to trigger conditions.
There may be pipelines that require an authorized user to comment /azp run to run.

@dotnet-policy-service

dotnet-policy-service Bot commented Oct 20, 2026

Copy link
Copy Markdown
Contributor

Tagging subscribers to this area: @dotnet/area-system-text-json
See info in area-owners.md if you want to be subscribed.

@asylkhan-azat

asylkhan-azat commented Oct 20, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor Author

No measurable difference on my machine (x64 Ryzen 7500F) using this benchmark

But the lock instruction is gone from the JIT output.

JsonArray.WriteTo before:

movzx    rdx, byte  ptr [rcx]
mov      rax, gword ptr [rcx+0x08]
mov      r8d, dword ptr [rcx+0x10]
lock
or       dword ptr [rsp], 0
cmp      qword ptr [rsi+0x18], 0

After:

movzx    rdx, byte  ptr [rcx]
mov      rax, gword ptr [rcx+0x08]
mov      r8d, dword ptr [rcx+0x10]
cmp      qword ptr [rsi+0x18], 0

@EgorBo

EgorBo commented Oct 20, 2026 •
edited
Loading

Copy link
Copy Markdown
Member

Not sure if this change is correct, but if it is - please introduce a pollyfill (C# 14.0 extension) for Volatile.ReadBarrier(); in src/libraries/Common/src/Polyfills. It's fine for it to use full memory barrier on old targets.

@asylkhan-azat

asylkhan-azat commented Oct 20, 2026

Copy link
Copy Markdown
Contributor Author

@EgorBo, done

@build-analysis build-analysis Bot mentioned this pull request Oct 20, 2026
Closed
@eiriktsarpalis
eiriktsarpalis merged commit ed49b6f into dotnet:main Oct 21, 2026
86 of 88 checks passed
This was referenced Oct 21, 2026
Merged
Merged
@dotnet-maestro dotnet-maestro Bot mentioned this pull request Oct 22, 2026
Merged
@dotnet-milestone-bot dotnet-milestone-bot Bot added this to the 12.0-preview1 milestone Oct 22, 2026
eiriktsarpalis added a commit that referenced this pull request Oct 22, 2026
@eiriktsarpalis
Use extension-member polyfills across libraries (#136221)
132d071 
<!-- -->

Use C# extension-member polyfills to consolidate framework-compatibility
code across `src/libraries`, following
[#136156](#136156). Call sites use
framework API names while downlevel implementations remain in internal
polyfill containers. Targets that already provide a member continue to
bind to the framework implementation.

The cleanup extends beyond System.Text.Json and its source generator to
CBOR, Metadata/NRBF, xxHash, Packaging, Dataflow, MEF2,
DiagnosticSource, Server-Sent Events, and several Microsoft.Extensions
libraries. It includes:

- Shared static-member and property polyfills for type and enum
inspection, bit and endian conversions, multiplication, array limits,
elapsed time, Unix epoch, and platform checks.
- Reuse of existing span, collection, stack, string, decimal, and
uninitialized-object polyfills instead of local compatibility helpers
and conditional branches.
- Standard argument guards in place of equivalent manual checks,
preserving validation order and parameter names.
- The same consolidation in existing CBOR tests and shared assertion
utilities, removing the paired CBOR test-helper files without adding
test coverage.

The source generator's `IsClosed` helper is expressed as an extension
property while retaining its existing Roslyn light-up and
syntax-fallback behavior. There are no public API changes or new test
cases.

Initial Release validation passed `.\build.cmd libs -rc Release -lc
Release /p:RunAnalyzersInBuild=true /p:RunILLinkInBuild=true`, followed
by targeted compatibility builds and existing test-suite runs on CoreCLR
and .NET Framework. Subsequent review adjustments were covered by
targeted rebuilds and tests.

> [!NOTE]
> This pull request was generated with GitHub Copilot.

---------

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 3c692d8a-03a2-4ed9-b337-2066c25a3cb6
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@eiriktsarpalis eiriktsarpalis eiriktsarpalis approved these changes

Assignees

No one assigned

Labels

area-System.Text.Json community-contribution Indicates that the PR has been added by a community member

Projects

None yet

Milestone

12.0-preview1

Development

Successfully merging this pull request may close these issues.

3 participants

@asylkhan-azat @EgorBo @eiriktsarpalis