Regressions in System.Text.Json.Document.Tests.Perf_EnumerateArray

[danmoseley]

broken out of #67101

Commit range: 6bf873a...bc5e386.

Seems the only relevant change is JIT: enable reading PGO data when switching to optimized (https://github.com/dotnet/runtime/pull/66618[)](https://github.com/dotnet/runtime/commit/2737da5817ab121fb43ff5577d1e146e0ec6d213)

category:performance
theme:benchmarks

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

Issue Details

---

broken out of #67101

Commit range: 6bf873a...bc5e386.

Seems the only relevant change is JIT: enable reading PGO data when switching to optimized (https://github.com/dotnet/runtime/pull/66618[)](https://github.com/dotnet/runtime/commit/2737da5817ab121fb43ff5577d1e146e0ec6d213)

Author:	danmoseley
Assignees:	-
Labels:	area-CodeGen-coreclr
Milestone:	-

[AndyAyersMS]

Hopefully addressed by the PGO update we're waiting for... cc @EgorBo

If not I'll take a look...

[AndyAyersMS]

Windows ARM64

Related benchmark shows similar but smaller regression

Windows x64 shows similar but much smaller regression

These are likely the same issues we see in #67318 where block reordering driven by PGO data is interfering with loop optimizations. Will need to drill in to be sure.

[AndyAyersMS]

Regression has persisted even after we deferred some block reordering in #69878

Need to investigate this one.

[AndyAyersMS]

Can repro this locally, digging in.

Method	Job	Toolchain	TestCase	Mean	Error	StdDev	Median	Min	Max	Ratio	Allocated	Alloc Ratio
EnumerateArray	Job-YJYIXG	\base-rel\corerun.exe	Json400KB	2.052 us	0.0020 us	0.0019 us	2.052 us	2.049 us	2.055 us	1.00	-	NA
EnumerateArray	Job-SGSSCH	\diff-rel\corerun.exe	Json400KB	2.400 us	0.0045 us	0.0042 us	2.398 us	2.396 us	2.410 us	1.17	-	NA

[AndyAyersMS]

Trying to find a way to run these with the -p ETW but running into issues because the host is arm64.

Got past it by updating BDN to use BenchmarkDotNet.Diagnostics.Windows 3.0.1 and doing a private build (which I had to do over on an x64 box, see dotnet/BenchmarkDotNet#2028) and then updating the performance repo to use this build of BDN.

[AndyAyersMS]

Assembly for the key loops. Main differences are:

• the block layout. In the after picture one of the loop blocks (IG25) ends up being placed after some non loop blocks (not shown)
• the address of the callee CheckNotDisposed is indirect in the after picture, and the indirection cell requires a large constant materialization
• there is one missed CSE.

Likely it is the difference in the call to CheckNotDisposed that accounts for most of the perf difference. Because the indir introduced for the call only shows up in lowering it's not a candidate for CSE/hoisting (there are two other call sites in this method). We should possibly expand this earlier.

Hoisting the IAT_PVALUE indir may end up being risky as the indir target presumably might get changed. Perhaps for frequently running loops we could split the loop somehow and refresh the hoisted value every so often.

But we could easily hoist/cse the creation of the indir addresses.

Before

After

G_M9251_IG17: ;; offset=0180H B9404BB3 ldr w19, [fp,#72] B9404FB4 ldr w20, [fp,#76] 6B14027F cmp w19, w20 5400062A bge G_M9251_IG29 G_M9251_IG18: ;; offset=0190H 7100027F cmp w19, #0 540000AA bge G_M9251_IG20 G_M9251_IG19: ;; offset=0198H B9405BA0 ldr w0, [fp,#88] 11003000 add w0, w0, #12 B9004BA0 str w0, [fp,#72] 1400001D b G_M9251_IG24 G_M9251_IG20: ;; offset=01A8H F9402BB5 ldr x21, [fp,#80] 39C002BF ldrsb wzr, [x21] AA1503E0 mov x0, x21 ;; CheckNotDisposed 97F95131 bl ** F9401AA0 ldr x0, [x21,#48] B50000A0 cbnz x0, G_M9251_IG21 350003B3 cbnz w19, G_M9251_IG27 AA1F03E1 mov x1, xzr 2A1F03E2 mov w2, wzr 14000009 b G_M9251_IG22 G_M9251_IG21: ;; offset=01D0H B9400801 ldr w1, [x0,#8] 6B13003F cmp w1, w19 540002E3 blo G_M9251_IG27 91004002 add x2, x0, #16 4B130020 sub w0, w1, w19 2A1303E1 mov w1, w19 8B010041 add x1, x2, x1 2A0003E2 mov w2, w0 G_M9251_IG22: ;; offset=01F0H 7100305F cmp w2, #12 540001AB blt G_M9251_IG26 B940003F ldr wzr, [x1] B9400820 ldr w0, [x1,#8] 531C7C01 lsr w1, w0, #28 53001C21 uxtb w1, w1 7100143F cmp w1, #5 5400018B blt G_M9251_IG28 11003261 add w1, w19, #12 G_M9251_IG23: ;; offset=0214H B9004BA1 str w1, [fp,#72] G_M9251_IG24: ;; offset=0218H B9404BA0 ldr w0, [fp,#72] 6B14001F cmp w0, w20 54FFFB0B blt G_M9251_IG17

G_M9251_IG14: ;; offset=0168H B94043B4 ldr w20, [fp,#64] B94047B6 ldr w22, [fp,#68] 6B16029F cmp w20, w22 540007AA bge G_M9251_IG27 G_M9251_IG15: ;; offset=0178H 7100029F cmp w20, #0 540000AA bge G_M9251_IG17 G_M9251_IG16: ;; offset=0180H B94053A0 ldr w0, [fp,#80] 11003000 add w0, w0, #12 B90043A0 str w0, [fp,#64] 1400001D b G_M9251_IG21 G_M9251_IG17: ;; offset=0190H F94027B3 ldr x19, [fp,#72] 39C0027F ldrsb wzr, [x19] AA1303E0 mov x0, x19 D2935001 movz x1, #0x9a80 F2AE1C21 movk x1, #0x70e1 LSL #16 F2CFFFA1 movk x1, #0x7ffd LSL #32 F9400021 ldr x1, [x1] ;; CheckNotDisposed D63F0020 blr x1 F9401A60 ldr x0, [x19,#48] B4000480 cbz x0, G_M9251_IG25 B9400801 ldr w1, [x0,#8] 6B14003F cmp w1, w20 54000283 blo G_M9251_IG23 91004001 add x1, x0, #16 2A1403E2 mov w2, w20 8B020021 add x1, x1, x2 B9400800 ldr w0, [x0,#8] ;; -CSE 4B140002 sub w2, w0, w20 G_M9251_IG18: ;; offset=01D8H 7100305F cmp w2, #12 5400026B blt G_M9251_IG24 B940003F ldr wzr, [x1] B9400820 ldr w0, [x1,#8] 531C7C01 lsr w1, w0, #28 53001C21 uxtb w1, w1 7100143F cmp w1, #5 5400030B blt G_M9251_IG26 G_M9251_IG19: ;; offset=01F8H 11003281 add w1, w20, #12 G_M9251_IG20: ;; offset=01FCH B90043A1 str w1, [fp,#64] G_M9251_IG21: ;; offset=0200H B94043A0 ldr w0, [fp,#64] 6B16001F cmp w0, w22 54FFFB0B blt G_M9251_IG14 G_M9251_IG25: ;; offset=0244H 35FFFE74 cbnz w20, G_M9251_IG23 AA1F03E1 mov x1, xzr 2A1F03E2 mov w2, wzr 17FFFFE2 b G_M9251_IG18

[AndyAyersMS]

I cherry-picked the changes for #70988 and while I see more compact call in AFTER case, it doesn't make the benchmark any faster. So seemingly the perf issue lies elsewhere?

Profiling is strongly implicating code within the method. Comparing two BDN runs with same number of iterations/invocations we see all the extra exclusive cycles in EnumerateArray.