Simplify trimming non significant digits in JsonUtf8Writer

[devsko]

Found this while thinking about DateTime only.

Benchmark

privatestaticbyte[]data;[Params(true,false)]publicboolWithOffset;[Params(0,3,7)]publicintSignificantDigits;[GlobalSetup]publicvoidSetup(){stringstr=SignificantDigitsswitch{0=>"2019-04-24T14:50:17.0000000",3=>"2019-04-24T14:50:17.1230000",7=>"2019-04-24T14:50:17.1234567",
_ =>thrownewException()};data=Encoding.UTF8.GetBytes(str+(WithOffset?"+12:34":""));}[Benchmark]publicvoidUnrolled()=>TrimDateTimeOffset_Unrolled(data,out_);[Benchmark(Baseline=true)]publicvoidOriginal()=>TrimDateTimeOffset_Original(data,out_);

Method	WithOffset	SignificantDigits	Mean	Error	StdDev	Ratio
Unrolled	False	0	6.490 ns	0.1511 ns	0.1413 ns	0.77
Original	False	0	8.448 ns	0.0376 ns	0.0333 ns	1.00
Unrolled	False	3	6.659 ns	0.0242 ns	0.0227 ns	0.19
Original	False	3	34.686 ns	0.0985 ns	0.0873 ns	1.00
Unrolled	False	7	5.318 ns	0.0303 ns	0.0253 ns	0.14
Original	False	7	37.438 ns	0.1090 ns	0.0966 ns	1.00
Unrolled	True	0	4.971 ns	0.0953 ns	0.0845 ns	0.11
Original	True	0	43.909 ns	0.1322 ns	0.1236 ns	1.00
Unrolled	True	3	5.321 ns	0.0203 ns	0.0190 ns	0.12
Original	True	3	43.836 ns	0.1169 ns	0.0976 ns	1.00
Unrolled	True	7	4.945 ns	0.0288 ns	0.0269 ns	0.11
Original	True	7	44.322 ns	0.1316 ns	0.1167 ns	1.00

[ghost]

Tagging subscribers to this area: @eiriktsarpalis, @layomia
See info in area-owners.md if you want to be subscribed.

Issue Details

---

Found this while thinking about DateTime only.

Benchmark

privatestaticbyte[]data;[Params(true,false)]publicboolWithOffset;[Params(0,3,7)]publicintSignificantDigits;[GlobalSetup]publicvoidSetup(){stringstr=SignificantDigitsswitch{0=>"2019-04-24T14:50:17.0000000",3=>"2019-04-24T14:50:17.1230000",7=>"2019-04-24T14:50:17.1234567",
_ =>thrownewException()};data=Encoding.UTF8.GetBytes(str+(WithOffset?"+12:34":""));}[Benchmark]publicvoidUnrolled()=>TrimDateTimeOffset_Unrolled(data,out_);[Benchmark(Baseline=true)]publicvoidOriginal()=>TrimDateTimeOffset_Original(data,out_);

Method	WithOffset	SignificantDigits	Mean	Error	StdDev	Ratio
Unrolled	False	0	6.490 ns	0.1511 ns	0.1413 ns	0.77
Original	False	0	8.448 ns	0.0376 ns	0.0333 ns	1.00
Unrolled	False	3	6.659 ns	0.0242 ns	0.0227 ns	0.19
Original	False	3	34.686 ns	0.0985 ns	0.0873 ns	1.00
Unrolled	False	7	5.318 ns	0.0303 ns	0.0253 ns	0.14
Original	False	7	37.438 ns	0.1090 ns	0.0966 ns	1.00
Unrolled	True	0	4.971 ns	0.0953 ns	0.0845 ns	0.11
Original	True	0	43.909 ns	0.1322 ns	0.1236 ns	1.00
Unrolled	True	3	5.321 ns	0.0203 ns	0.0190 ns	0.12
Original	True	3	43.836 ns	0.1169 ns	0.0976 ns	1.00
Unrolled	True	7	4.945 ns	0.0288 ns	0.0269 ns	0.11
Original	True	7	44.322 ns	0.1316 ns	0.1167 ns	1.00

Author:	devsko
Assignees:	-
Labels:	area-System.Text.Json
Milestone:	-

[eiriktsarpalis]

There's no question that the existing implementation is doing unnecessary work, but that being said I do find the formatting of this particular section to be slightly disagreeable. This is not a hot path method so I'm not sure it's worth unrolling. I'd be interested to see performance numbers with your curIndex calculation logic refactored into a loop.

[devsko]

The loop version is only slightly slower. I would say this path can get pretty hot depending on the payload. But I agree - will change.

[eiriktsarpalis]

Thanks

[devsko]

Hi @eiriktsarpalis. Unfortunately the fastest loop version i could come up with still regresses (about 1.4x) compared to the original version when there are no significant digits and no offset. In this case the original version is completely branchless. In all other cases the loop is much faster but i consider this the most common real-world scenario. Because this method runs for every single DateTime(Offset) that gets serialized, i suggest to either go with the unrolled version or close this PR and leave the trimming as it is.

Method	WithOffset	SignificantDigits	Mean	Error	StdDev	Ratio	RatioSD
Loop	False	0	10.402 ns	0.1525 ns	0.1427 ns	1.45	0.02
Unrolled	False	0	5.099 ns	0.1284 ns	0.1138 ns	0.71	0.02
Original	False	0	7.204 ns	0.0210 ns	0.0175 ns	1.00	0.00
Loop	False	3	8.579 ns	0.0430 ns	0.0402 ns	0.24	0.00
Unrolled	False	3	5.645 ns	0.0219 ns	0.0205 ns	0.16	0.00
Original	False	3	35.762 ns	0.0807 ns	0.0674 ns	1.00	0.00
Loop	False	7	5.182 ns	0.0274 ns	0.0256 ns	0.13	0.00
Unrolled	False	7	3.558 ns	0.0093 ns	0.0077 ns	0.09	0.00
Original	False	7	38.781 ns	0.1142 ns	0.1012 ns	1.00	0.00
Loop	True	0	10.795 ns	0.0179 ns	0.0150 ns	0.24	0.00
Unrolled	True	0	3.606 ns	0.0152 ns	0.0135 ns	0.08	0.00
Original	True	0	44.069 ns	0.0850 ns	0.0710 ns	1.00	0.00
Loop	True	3	10.793 ns	0.0128 ns	0.0114 ns	0.24	0.00
Unrolled	True	3	3.602 ns	0.0175 ns	0.0164 ns	0.08	0.00
Original	True	3	44.173 ns	0.2023 ns	0.1892 ns	1.00	0.00
Loop	True	7	10.845 ns	0.0475 ns	0.0444 ns	0.25	0.00
Unrolled	True	7	3.988 ns	0.0345 ns	0.0322 ns	0.09	0.00
Original	True	7	44.011 ns	0.0688 ns	0.0610 ns	1.00	0.00

Loop version

publicstaticvoidTrimDateTimeOffset(Span<byte>buffer,outintbytesWritten){// Assert buffer is the right length for:// YYYY-MM-DDThh:mm:ss.fffffff (JsonConstants.MaximumFormatDateTimeLength)// YYYY-MM-DDThh:mm:ss.fffffffZ (JsonConstants.MaximumFormatDateTimeLength + 1)// YYYY-MM-DDThh:mm:ss.fffffff(+|-)hh:mm (JsonConstants.MaximumFormatDateTimeOffsetLength)Debug.Assert(buffer.Length==JsonConstants.MaximumFormatDateTimeLength||buffer.Length==JsonConstants.MaximumFormatDateTimeLength+1||buffer.Length==JsonConstants.MaximumFormatDateTimeOffsetLength);// Find the position after the last significant digit in seconds fraction.intcurIndex;for(curIndex=JsonConstants.MaximumFormatDateTimeLength-1;buffer[curIndex]=='0';curIndex--);if(curIndex>=JsonConstants.MaximumFormatDateTimeLength-7){curIndex++;}// We are either trimming a// (a) DateTimeOffset, or a// DateTime with// (b) DateTimeKind.Local or// (c) DateTimeKind.Utcif(buffer.Length==JsonConstants.MaximumFormatDateTimeLength){// (b) There is no offset to copy.bytesWritten=curIndex;}elseif(buffer.Length==JsonConstants.MaximumFormatDateTimeOffsetLength){// (a) We have a non-UTC offset i.e. (+|-)hh:mm that are always 6 characters to copy.buffer[curIndex]=buffer[JsonConstants.MaximumFormatDateTimeLength];buffer[curIndex+1]=buffer[JsonConstants.MaximumFormatDateTimeLength+1];buffer[curIndex+2]=buffer[JsonConstants.MaximumFormatDateTimeLength+2];buffer[curIndex+3]=buffer[JsonConstants.MaximumFormatDateTimeLength+3];buffer[curIndex+4]=buffer[JsonConstants.MaximumFormatDateTimeLength+4];buffer[curIndex+5]=buffer[JsonConstants.MaximumFormatDateTimeLength+5];bytesWritten=curIndex+6;}else{// (c) There is a single 'Z'. Just write it at the current index.Debug.Assert(buffer[JsonConstants.MaximumFormatDateTimeLength+1-1]=='Z');buffer[curIndex]=(byte)'Z';bytesWritten=curIndex+1;}}

[eiriktsarpalis]

Thanks!