[S.T.J] Audit is ... or ... pattern matching

[jozkee]

Continuation of #132452.

While writing the RC1 servicing template I realized more pattern-matching callsites had been missed, so I took a more mechanical approach:

1. Reverted Modernize System.Text.Json product code #130976.
2. Re-applied the commits from that PR that are unrelated to pattern matching.
3. Restored the Debug.Assert changes.
4. Restored the is [not] null changes.

This narrows the diff to 7 changes, since some were already reverted in #132452.
To find any pattern-matching that predates #130976, I also ran:

cd /mnt/e/runtime/src/libraries/System.Text.Json/src
rg -n -g '*.cs' "\bis\b[-\w.'\"+\\\\() ]*\bor\b" . \
| rg -v 'Debug\.Assert' \ # filter assertions
| rg -v ':\s*(//|///|\*)' # filter comments

This returned 36 results. A few notes on how they break down:

Of the genuine matches, 8 were introduced in .NET 11. One of those is an exception filter, discarded (DefaultJsonTypeInfoResolver.Helpers.cs:478).
That leaves 7 pattern-matching callsites new in .NET 11, all in cold paths (metadata/type-info configuration, escaping for JsonNode.GetPath()/error messages) — none on the serialization hot path AFAICT.

[azure-pipelines]

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]

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

[Copilot]

Pull request overview

Warning

Copilot couldn't run its full agentic review because it didn't start before the timeout. Make sure your repository has a runner available, or add a copilot-code-review.yml file specifying one with the runs-on attribute. See the docs for more details.

This PR mechanically replaces is ... or ... pattern matching constructs with equivalent boolean expressions in System.Text.Json, aligning the code with the servicing template requirements and reducing remaining pattern-matching callsites.

Changes:

• Replaced is X or Y token checks with == ... || == ... comparisons.
• Replaced is A or B byte checks with == ... || == ... comparisons.
• Adjusted select null-check expressions around NumberHandling.

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.

Show a summary per file

File	Description
src/libraries/System.Text.Json/src/System/Text/Json/ThrowHelper.cs	Replaced range pattern (is >= ... and < ...) with equivalent numeric comparisons.
src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonConverterOfT.cs	Tweaked null-check style around state.Current.NumberHandling.
src/libraries/System.Text.Json/src/System/Text/Json/Reader/Utf8JsonReader.cs	Replaced is ... or ... checks for token types/bytes with ==/`
src/libraries/System.Text.Json/src/System/Text/Json/Reader/Utf8JsonReader.MultiSegment.cs	Replaced is ... or ... byte checks with ==/`
src/libraries/System.Text.Json/src/System/Text/Json/Document/JsonDocument.Parse.cs	Replaced is ... or ... token checks with ==/`

[Copilot]

Pull request overview

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

Suppressed comments (1)

src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonConverterOfT.cs:188

• This null check is the only remaining != null in this file and is inconsistent with the surrounding is not null checks (e.g., earlier in TryRead and later in TryWrite). Using is not null also matches the repo’s C# null-check convention and avoids the mix of styles in the same method.

if (state.Current.NumberHandling != null && IsInternalConverterForNumberType)
{

[eiriktsarpalis]

I think it would be very unfortunate if we had to resort to merging this PR. Assuming this is not fixing any active regressions, I would strongly recommend working with the @dotnet/roslyn team to improve codegen for pattern matching instead.

[EgorBo]

Is there a minimal repro we can look at from the JIT's perspective?

[jozkee]

@EgorBo, .NET Lab for the changes here and #132452 (comment) for the previously fixed ones.

This surfaced as an ARM64 performance regression (#131600), and the bisect isolated the pattern-matching changes as the cause. The Roslyn lowering behavior is tracked by dotnet/roslyn#80052.

[Copilot]

Pull request overview

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

[jozkee]

Running the three System.Text.Json benchmarks from dotnet/performance commit 41fcabd6f5dc929253086bd117c4adaf24c83ed9 on ARM64 Ampere.

@EgorBot -ubuntu24_azure_ampere

usingSystem;usingSystem.Buffers;usingSystem.Text;usingSystem.Text.Json;usingBenchmarkDotNet.Attributes;publicclassPerf_ElementParseValue{privatebyte[]_string=null!;privatebyte[]_number=null!;privatebyte[]_object=null!;[GlobalSetup]publicvoidSetup(){_string=Encoding.UTF8.GetBytes("\"a short json string value\"");_number=Encoding.UTF8.GetBytes("123456789");_object=Encoding.UTF8.GetBytes("{\"value\":123456789}");}[Benchmark]publicJsonValueKindParseString()=>Parse(_string);[Benchmark]publicJsonValueKindParseNumber()=>Parse(_number);[Benchmark]publicJsonValueKindParseObject()=>Parse(_object);privatestaticJsonValueKindParse(byte[]utf8Json){varreader=newUtf8JsonReader(utf8Json);returnJsonElement.ParseValue(refreader).ValueKind;}}publicclassPerf_CommentLineSeparators{privateconstintSegmentSize=100;[Params(JsonCommentHandling.Skip,JsonCommentHandling.Allow)]publicJsonCommentHandlingCommentHandling;[Params(false,true)]publicboolMultiSegment;privatebyte[]_jsonPayload=null!;privateReadOnlySequence<byte>_jsonPayloadSequence;[GlobalSetup]publicvoidSetup(){_jsonPayload=Encoding.UTF8.GetBytes("{}//"+newstring('\u2027',2000)+"\n");_jsonPayloadSequence=SequenceFactory.Create(_jsonPayload,SegmentSize);}[Benchmark]publicvoidReadCommentWithSeparators(){varstate=newJsonReaderState(newJsonReaderOptions{CommentHandling=CommentHandling});Utf8JsonReaderreader=MultiSegment?newUtf8JsonReader(_jsonPayloadSequence,isFinalBlock:true,state):newUtf8JsonReader(_jsonPayload,isFinalBlock:true,state);while(reader.Read()){}}}publicclassPerf_ValueTextEquals{privateconstintPropertyCount=100;[Params(false,true)]publicboolEscaped;[Params(false,true)]publicboolMultiSegment;privatebyte[]_dataUtf8=null!;privateReadOnlySequence<byte>_sequence;privatebyte[]_lookupUtf8=null!;[GlobalSetup]publicvoidSetup(){_lookupUtf8=Encoding.UTF8.GetBytes("property_"+PropertyCount);varbuilder=newStringBuilder("{");for(inti=0;i<PropertyCount;i++){if(i!=0){builder.Append(',');}builder.Append('"');AppendPropertyName(builder,i,Escaped);builder.Append("\":");builder.Append(i);}builder.Append('}');_dataUtf8=Encoding.UTF8.GetBytes(builder.ToString());_sequence=SequenceFactory.Create(_dataUtf8,_dataUtf8.Length/2);}[Benchmark]publicintMatchPropertyNames(){Utf8JsonReaderreader=MultiSegment?newUtf8JsonReader(_sequence):newUtf8JsonReader(_dataUtf8);intmatches=0;while(reader.Read()){if(reader.TokenType==JsonTokenType.PropertyName&&reader.ValueTextEquals(_lookupUtf8)){matches++;}}returnmatches;}privatestaticvoidAppendPropertyName(StringBuilderbuilder,intindex,boolescaped){conststringPrefix="property_";if(!escaped){builder.Append(Prefix).Append(index);return;}foreach(charcinPrefix){builder.Append("\\u").Append(((int)c).ToString("x4"));}builder.Append(index);}}internalstaticclassSequenceFactory{publicstaticReadOnlySequence<byte>Create(byte[]data,intsegmentSize){varfirst=newBufferSegment(data.AsMemory(0,Math.Min(segmentSize,data.Length)));BufferSegmentlast=first;for(intoffset=segmentSize;offset<data.Length;offset+=segmentSize){last=last.Append(data.AsMemory(offset,Math.Min(segmentSize,data.Length-offset)));}returnnewReadOnlySequence<byte>(first,0,last,last.Memory.Length);}privatesealedclassBufferSegment:ReadOnlySequenceSegment<byte>{publicBufferSegment(ReadOnlyMemory<byte>memory){Memory=memory;}publicBufferSegmentAppend(ReadOnlyMemory<byte>memory){varsegment=newBufferSegment(memory){RunningIndex=RunningIndex+Memory.Length};Next=segment;returnsegment;}}}

Note

This benchmark request was prepared with GitHub Copilot.

[RikkiGibson]

It sounded like @jjonescz is looking at the underlying codegen issue

[jozkee]

@RikkiGibson will that make it to .NET 11?

[jozkee]

Maybe I was overly pessimistic, if Roslyn provides a fix for this, we should be fine, assuming we ingest it before GA snap around mid October.

[333fred]

@RikkiGibson will that make it to .NET 11?

Barring something unexpected, it seems highly likely that it would make .NET 11, probably by RC2. Again, unknown unknowns excepted.