[Perf] ubuntu 20.04/arm64 : Regressions in System.Text.Json.Tests.Utf8JsonReaderCommentsTests on 8/11/2022 10:02:44 AM

[performanceautofiler]

Run Information

Architecture	arm64
OS	ubuntu 20.04
Baseline	85aa0521681592bb0ae4c6c89f7fb97895e2c7fc
Compare	deac993c3640730e5f4acaed8d257dde0b17c2fe
Diff	Diff

Regressions in System.Text.Json.Tests.Utf8JsonReaderCommentsTests

Benchmark	Baseline	Test	Test/Base	Test Quality	Edge Detector	Baseline IR	Compare IR	IR Ratio	Baseline ETL	Compare ETL
Utf8JsonReaderCommentParsing - Duration of single invocation	16.63 μs	19.90 μs	1.20	0.01	False

Test Report

Repro

git clone https://github.com/dotnet/performance.git
py .\performance\scripts\benchmarks_ci.py -f net6.0 --filter 'System.Text.Json.Tests.Utf8JsonReaderCommentsTests*'

Payloads

Baseline
Compare

Histogram

System.Text.Json.Tests.Utf8JsonReaderCommentsTests.Utf8JsonReaderCommentParsing(CommentHandling: Allow, SegmentSize: 100, TestCase: LongMultiLine)

Description of detection logic

IsRegressionChecked: Marked as regression because the three check build points were 0.05 greater than the baseline.
IsImprovementBase: Marked as not an improvement because the compare was not 5% less than the baseline, or the value was too small.
IsRegressionBase: Marked as regression because the compare was 5% greater than the baseline, and the value was not too small.
IsRegressionChecked: Marked as regression because the three check build points were 0.05 greater than the baseline.
IsRegressionWindowed: Marked as regression because 19.90144877700127 > 17.44842946942619.
IsChangePoint: Marked as a change because one of 7/2/2022 7:35:06 PM, 7/6/2022 3:55:24 AM, 8/11/2022 4:03:37 AM, 8/18/2022 11:13:04 AM falls between 8/9/2022 9:33:37 PM and 8/18/2022 11:13:04 AM.
IsRegressionStdDev: Marked as regression because -88.24562840213467 (T) = (0 -19694.055205405868) / Math.Sqrt((10152.604883938444 / (21)) + (23337.830850333976 / (35))) is less than -2.0048792881871513 = MathNet.Numerics.Distributions.StudentT.InvCDF(0, 1, (21) + (35) - 2, .025) and -0.17920200235726796 = (16701.171780608183 - 19694.055205405868) / 16701.171780608183 is less than -0.05.
IsImprovementBase: Marked as not an improvement because the compare was not 5% less than the baseline, or the value was too small.
IsChangeEdgeDetector: Marked not as a regression because Edge Detector said so.

Docs

Profiling workflow for dotnet/runtime repository
Benchmarking workflow for dotnet/runtime repository

[ghost]

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

Issue Details

---

Run Information

Architecture	arm64
OS	ubuntu 20.04
Baseline	85aa0521681592bb0ae4c6c89f7fb97895e2c7fc
Compare	deac993c3640730e5f4acaed8d257dde0b17c2fe
Diff	Diff

Regressions in System.Text.Json.Tests.Utf8JsonReaderCommentsTests

Benchmark	Baseline	Test	Test/Base	Test Quality	Edge Detector	Baseline IR	Compare IR	IR Ratio	Baseline ETL	Compare ETL
Utf8JsonReaderCommentParsing - Duration of single invocation	16.63 μs	19.90 μs	1.20	0.01	False

Test Report

Repro

git clone https://github.com/dotnet/performance.git
py .\performance\scripts\benchmarks_ci.py -f net6.0 --filter 'System.Text.Json.Tests.Utf8JsonReaderCommentsTests*'

Payloads

Baseline
Compare

Histogram

System.Text.Json.Tests.Utf8JsonReaderCommentsTests.Utf8JsonReaderCommentParsing(CommentHandling: Allow, SegmentSize: 100, TestCase: LongMultiLine)

Description of detection logic

IsRegressionChecked: Marked as regression because the three check build points were 0.05 greater than the baseline.
IsImprovementBase: Marked as not an improvement because the compare was not 5% less than the baseline, or the value was too small.
IsRegressionBase: Marked as regression because the compare was 5% greater than the baseline, and the value was not too small.
IsRegressionChecked: Marked as regression because the three check build points were 0.05 greater than the baseline.
IsRegressionWindowed: Marked as regression because 19.90144877700127 > 17.44842946942619.
IsChangePoint: Marked as a change because one of 7/2/2022 7:35:06 PM, 7/6/2022 3:55:24 AM, 8/11/2022 4:03:37 AM, 8/18/2022 11:13:04 AM falls between 8/9/2022 9:33:37 PM and 8/18/2022 11:13:04 AM.
IsRegressionStdDev: Marked as regression because -88.24562840213467 (T) = (0 -19694.055205405868) / Math.Sqrt((10152.604883938444 / (21)) + (23337.830850333976 / (35))) is less than -2.0048792881871513 = MathNet.Numerics.Distributions.StudentT.InvCDF(0, 1, (21) + (35) - 2, .025) and -0.17920200235726796 = (16701.171780608183 - 19694.055205405868) / 16701.171780608183 is less than -0.05.
IsImprovementBase: Marked as not an improvement because the compare was not 5% less than the baseline, or the value was too small.
IsChangeEdgeDetector: Marked not as a regression because Edge Detector said so.

Docs

Profiling workflow for dotnet/runtime repository
Benchmarking workflow for dotnet/runtime repository

Author:	performanceautofiler[bot]
Assignees:	kunalspathak
Labels:	area-System.Text.Json, untriaged, refs/heads/main, RunKind=micro, Regression, CoreClr, arm64, ubuntu 20.04, Ampere
Milestone:	-

[DrewScoggins]

Unclear what the culprit is, but it falls under this range: 7be3790...bb97114

[eiriktsarpalis]

Looking at that commit range, #73481 appears to stand out since single-line comment parsing is implemented using IndexOfAny:

runtime/src/libraries/System.Text.Json/src/System/Text/Json/Reader/Utf8JsonReader.cs

Lines 2376 to 2381 in 7aef48a

	private int FindLineSeparator(ReadOnlySpan<byte> localBuffer)
	{
	int totalIdx = 0;
	while (true)
	{
	int idx = localBuffer.IndexOfAny(JsonConstants.LineFeed, JsonConstants.CarriageReturn, JsonConstants.StartingByteOfNonStandardSeparator);

Could this change have the potential to regress ARM64? cc @adamsitnik

[adamsitnik]

Could this change have the potential to regress ARM64?

It could. Span.IndexOfAnyThreeValues has not regressed on arm64 during this period:

https://pvscmdupload.blob.core.windows.net/reports/allTestHistory%2frefs%2fheads%2fmain_arm64_ubuntu%2018.04%2fSystem.Memory.Span(Byte).IndexOfAnyThreeValues(Size%3a%20512).html

But this benchmark uses span of 512 bytes. The benchmark for which the regression was reported here uses multiple 100-byte long spans and I would not expect it to regress.

@eiriktsarpalis the best thing to do would be to profile it. Do you have access to Surface Pro X? The EtwProfiler is now supported on arm64 (dotnet/BenchmarkDotNet#2030) so you could quickly get the profiles.

[eiriktsarpalis]

I've been able to isolate the performance regression to #73481 locally:

Method	Job	Commit	CommentHandling	SegmentSize	TestCase	Mean	Error	StdDev	Median	Min	Max	Ratio	MannWhitney(3%)	Allocated	Alloc Ratio
Utf8JsonReaderCommentParsing	Job-RLGOGT	cbf2e32	Allow	100	LongMultiLine	19,630.1 ns	75.98 ns	63.45 ns	19,613.6 ns	19,526.0 ns	19,780.1 ns	1.00	Base	-	NA
Utf8JsonReaderCommentParsing	Job-PUXNWH	402aa85	Allow	100	LongMultiLine	22,952.5 ns	70.62 ns	62.60 ns	22,957.2 ns	22,873.4 ns	23,107.2 ns	1.17	Slower	-	NA
Utf8JsonReaderCommentParsing	Job-RLGOGT	cbf2e32	Skip	100	LongMultiLine	18,724.5 ns	49.57 ns	41.39 ns	18,705.5 ns	18,672.5 ns	18,787.0 ns	1.00	Base	-	NA
Utf8JsonReaderCommentParsing	Job-PUXNWH	402aa85	Skip	100	LongMultiLine	21,984.6 ns	72.15 ns	63.96 ns	21,978.3 ns	21,871.6 ns	22,101.3 ns	1.17	Slower	-	NA

All other benchmark parameters record no regressions, so I've skipped them from this list. @adamsitnik could you take a look?

[eiriktsarpalis]

Profiling indicates that most time is spent in SpanHelpers.IndexOfAny: profiledata.speedscope.zip

[ghost]

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

Issue Details

---

Run Information

Architecture	arm64
OS	ubuntu 20.04
Baseline	85aa0521681592bb0ae4c6c89f7fb97895e2c7fc
Compare	deac993c3640730e5f4acaed8d257dde0b17c2fe
Diff	Diff

Regressions in System.Text.Json.Tests.Utf8JsonReaderCommentsTests

Benchmark	Baseline	Test	Test/Base	Test Quality	Edge Detector	Baseline IR	Compare IR	IR Ratio	Baseline ETL	Compare ETL
Utf8JsonReaderCommentParsing - Duration of single invocation	16.63 μs	19.90 μs	1.20	0.01	False

Test Report

Repro

git clone https://github.com/dotnet/performance.git
py .\performance\scripts\benchmarks_ci.py -f net6.0 --filter 'System.Text.Json.Tests.Utf8JsonReaderCommentsTests*'

Payloads

Baseline
Compare

Histogram

System.Text.Json.Tests.Utf8JsonReaderCommentsTests.Utf8JsonReaderCommentParsing(CommentHandling: Allow, SegmentSize: 100, TestCase: LongMultiLine)

Description of detection logic

IsRegressionChecked: Marked as regression because the three check build points were 0.05 greater than the baseline.
IsImprovementBase: Marked as not an improvement because the compare was not 5% less than the baseline, or the value was too small.
IsRegressionBase: Marked as regression because the compare was 5% greater than the baseline, and the value was not too small.
IsRegressionChecked: Marked as regression because the three check build points were 0.05 greater than the baseline.
IsRegressionWindowed: Marked as regression because 19.90144877700127 > 17.44842946942619.
IsChangePoint: Marked as a change because one of 7/2/2022 7:35:06 PM, 7/6/2022 3:55:24 AM, 8/11/2022 4:03:37 AM, 8/18/2022 11:13:04 AM falls between 8/9/2022 9:33:37 PM and 8/18/2022 11:13:04 AM.
IsRegressionStdDev: Marked as regression because -88.24562840213467 (T) = (0 -19694.055205405868) / Math.Sqrt((10152.604883938444 / (21)) + (23337.830850333976 / (35))) is less than -2.0048792881871513 = MathNet.Numerics.Distributions.StudentT.InvCDF(0, 1, (21) + (35) - 2, .025) and -0.17920200235726796 = (16701.171780608183 - 19694.055205405868) / 16701.171780608183 is less than -0.05.
IsImprovementBase: Marked as not an improvement because the compare was not 5% less than the baseline, or the value was too small.
IsChangeEdgeDetector: Marked not as a regression because Edge Detector said so.

Docs

Profiling workflow for dotnet/runtime repository
Benchmarking workflow for dotnet/runtime repository

Author:	performanceautofiler[bot]
Assignees:	kunalspathak
Labels:	arch-arm64, area-System.Memory, os-linux, tenet-performance, tenet-performance-benchmarks
Milestone:	7.0.0

[eiriktsarpalis]

FWIW I'm reproducing the regression on Windows.

[jeffhandley]

@adamsitnik Should we consider reverting #73481 for 7.0? It looks like the gains from that change are less significant than this regression.

[jeffhandley]

The revert for this change won't be straightforward because the underlying code has been changed since the performance changes were made. Because the changes benefit other scenarios and the regressed benchmark is related to JSON comments (vs. data), we are going to accept this regression for 7.0 but we will investigate it for a potential fix early in 8.0.

[tannergooding]

We're in between the regression point and baseline currently. This isn't really critical to fix and is overall within an acceptable range considering the current code is maintainable and supports multiple platforms and hardware configurations without being overly complex.

PRs to improve the perf would be accepted, but we need to take into account any increases in code maintenance burden as part of that.