Fixes for partial compilation of methods #80635

AndyAyersMS · 2023-01-13T23:07:49Z

[edited: was enable PC by default -- we will pursue that in a subsequent PR]

Fix some issues seen with partial compilation in wider testing:

PC jit helper must not block GC
proper flow cleanup when we put a PC patchpoint at a switch or degenerate cond

Enable partial compilation of Tier0 methods, using the simple strategy that any stack-empty non-entry non-handler rarely run block is worth deferring. Since Tier0 curently does not access static profile data, this means we mainly defer compilation of blocks with explicit throws. This leverages patchpoints to trigger compilation of the missing parts of methods in a manner similar to OSR. It is enabled for x64 and arm64.

ghost · 2023-01-13T23:08:00Z

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

Issue Details

Enable partial compilation of Tier0 methods, using the simple strategy that any stack-empty non-entry non-handler rarely run block is worth deferring.

Since Tier0 curently does not access static profile data, this means we mainly defer compilation of blocks with explicit throws.

This leverages patchpoints to trigger compilation of the missing parts of methods in a manner similar to OSR. It is enabled for x64 and arm64.

Author:	AndyAyersMS
Assignees:	AndyAyersMS
Labels:	`area-CodeGen-coreclr`
Milestone:	-

AndyAyersMS · 2023-01-13T23:13:04Z

@EgorBo PTAL
cc @dotnet/jit-contrib

Possibly should mark this as "draft" as I'm not sure it is worth enabling just yet. At any rate this is a trivial change since the PC mechanism has been in the code (and run in various experimental CI legs) for quite a while now.

TP impact (per SPMI) will be both interesting and misleading. We hopefully should see a bit of movement in the ASP.NET startup data.

EgorBo · 2023-01-13T23:15:22Z

@AndyAyersMS perhaps worth running some stress tests?

AndyAyersMS · 2023-01-13T23:17:13Z

@AndyAyersMS perhaps worth running some stress tests?

Sure. Let's see if it makes it through normal CI first ~~(guessing it will)~~ -- looks like there may be a failure to investigate.

AndyAyersMS · 2023-01-14T18:16:59Z

Not sure if the failures are related or not -- locally I can't repro.

Diffs. TP impact is pretty small per SPMI, will be interesting to see on actual apps.

EgorBo

Wow, this is quite the diffs 🙂
Let's also see if TE benchmarks will be affected, e.g. FullPGO ones where we disable R2R

AndyAyersMS · 2023-01-17T20:58:21Z

Still a bit puzzled by the failures. I can't repro them so far locally and can't get anything useful out of the core dumps.

AndyAyersMS · 2023-02-03T17:13:12Z

Looks like all the recent pred list stuff is causing PC to faill -- will work up a fix. Also there is one other failure; can't tell for sure if it is the same issue that I mentioned above since CI has purged the logs, but I think it probably is.

We may have degenerate flow out of a partial compilation block, so make sure to fully remove the block from all successor pred lists. Fixes issue seen in dotnet#80635.

We may have degenerate flow out of a partial compilation block, so make sure to fully remove the block from all successor pred lists. Fixes issue seen in #80635.

AndyAyersMS · 2023-02-06T02:53:27Z

Merged to pick up fix from #81605.

…ation

AndyAyersMS · 2023-02-24T20:45:20Z

@jkotas I was seeing sporadic failures in OSX/Linux libraries tests. Debugging locally, I could repro a GC suspension timeout -- and while I didn't catch the PartialCompilation helper on the stack it seems likely that the looping /waiting we were doing in there was the culprit.

Can you look over how I modified this helper to enable preemptive GC when you get a chance?

jkotas · 2023-02-24T21:03:02Z

src/coreclr/vm/jithelpers.cpp

@@ -5219,15 +5219,11 @@ void JIT_Patchpoint(int* counter, int ilOffset)
 // Unlike regular patchpoints, partial compilation patchpoints
 // must always transitio.
 //
-void JIT_PartialCompilationPatchpoint(int ilOffset)
+HCIMPL1(VOID, JIT_PartialCompilationPatchpoint, int ilOffset)


Does JIT_Patchpoint need the same treatment?

It doesn't loop, so (at least so far) I've never seen it cause problems.

If it does end up jitting a method, that thread switches to preemptive, and any other threads that hit the patchpoint don't need to wait, they just keep running the Tier0 code.

Does the HELPER_METHOD_FRAME_BEGIN_0 setup need to be moved in front of EECodeInfo codeInfo(ip); for the same reason you had to move here?

jkotas · 2023-02-24T21:03:33Z

src/coreclr/vm/jithelpers.cpp

+            // (but consider: throw path in method with try/catch, OSR method will contain more than just the throw?)
+            //
+            LOG((LF_TIEREDCOMPILATION, LL_INFO10, "Jit_PartialCompilationPatchpoint: patchpoint [%d] (0x%p) TRIGGER\n", ppId, ip));
+            PCODE newMethodCode = JitPatchpointWorker(pMD, codeInfo, ilOffset);


It would be nice to also add STANDARD_VM_CONTRACT; annotation to JitPatchpointWorker, and make callers switch to preemptive mode before calling it.

AndyAyersMS · 2023-02-24T22:04:23Z

Arm failure is perhaps another instance of #82252 (which is closed as fixed).

AndyAyersMS · 2023-02-24T22:05:27Z

With the latest commit, PC may be enabling GC at places we wouldn't have GC'd before, so it could be exposing some latent reporting issue.

AndyAyersMS · 2023-02-24T22:15:13Z

Hmm, seems like more widespread problems with contract violations.

jkotas · 2023-02-25T02:29:09Z

src/coreclr/vm/jithelpers.cpp

    ::SetLastError(dwLastError);
+    ENDFORBIDGC();


ENDFORBIDGC may need to be before SetLastError.

Thanks -- will reorder these.

AndyAyersMS · 2023-02-25T03:30:24Z

Latest Diffs.

SPMI sees about at 1.5% TP improvement for minopts, and (I'm guessing) a similar decrease in code size.

AndyAyersMS · 2023-02-25T03:34:05Z

/azp run runtime-coreclr outerloop, runtime-coreclr jitstress, runtime-coreclr pgo, runtime-coreclr libraries-pgo

azure-pipelines · 2023-02-25T03:34:29Z

Azure Pipelines successfully started running 4 pipeline(s).

AndyAyersMS · 2023-02-25T17:44:33Z

Basic CI tests are all good; extended tests have some issues:

libraries pgo failure looks like it might be unrelated? F
not sure about the outerloop failure -- error is an an arm test, but PC is only enabled for x64/arm64.
A number of jitstress failures to sort through that seem clearly related. Will tackle those first.

AndyAyersMS · 2023-02-26T17:25:12Z

Jitstress issue was random PGO stress putting PC patchpoints in blocks with switches; we weren't using the more careful successor iterator and so were seeing duplicate successors.

AndyAyersMS · 2023-02-26T17:42:24Z

/azp run runtime-coreclr outerloop, runtime-coreclr jitstress, runtime-coreclr pgo, runtime-coreclr libraries-pgo

azure-pipelines · 2023-02-26T17:43:00Z

Azure Pipelines successfully started running 4 pipeline(s).

AndyAyersMS · 2023-02-26T22:56:09Z

Libraries PGO failure:

Assert failure(PID 28 [0x0000001c], Thread: 37 [0x0025]): Assertion failed '(igInLoop->igLoopBackEdge == nullptr) || (igInLoop->igLoopBackEdge == igLoopHeader)' in 'System.Enum:TryParseByValueOrName[int](System.RuntimeType,System.ReadOnlySpan`1[ushort],bool,bool,byref):bool' during 'Generate code' (IL size 724; hash 0xef12aa33; Tier1)

    File: /__w/1/s/src/coreclr/jit/emit.cpp Line: 5713
    Image: /root/helix/work/correlation/dotnet

Looks like emitter accounting is off -- since this is Tier1 (and not OSR) seems like it is probably an existing issue. Looks like it popped up in runs against main done this weekend. @EgorBo let me know if you're going to track this one with a new test monitor issue.

CG2 failure is

Fatal error. Internal CLR error. (0x80131506)
at System.GC.AllocateNewArray(IntPtr, Int32, GC_ALLOC_FLAGS)
at System.Text.StringBuilder.ExpandByABlock(Int32)
at System.Text.StringBuilder.Append(Char, Int32)
at System.Text.StringBuilder.Append(Char)
at System.Diagnostics.StackTrace.ToString(TraceFormat, System.Text.StringBuilder)
at System.Diagnostics.StackTrace.ToString(TraceFormat)
at System.Exception.get_StackTrace()
at System.Exception.ToString()
at Internal.JitInterface.CorInfoImpl.AllocException(System.Exception)

I am seeing this same failure in recent outerloop runs on main too.

EgorBo · 2023-02-27T18:54:25Z

@EgorBo let me know if you're going to track this one with a new test monitor issue.

Sure, let me see if it pops up in different PRs, but presumably indeed worth filing an issue since it should not be connected with your PR directly

AndyAyersMS · 2023-02-27T19:38:49Z

@EgorBo let me know if you're going to track this one with a new test monitor issue.

Sure, let me see if it pops up in different PRs, but presumably indeed worth filing an issue since it should not be connected with your PR directly

#82729

BruceForstall · 2023-03-04T19:47:13Z

Is it worthwhile to separately PR/merge the various fixes and the TC_PartialCompilation=1 enabling change?

AndyAyersMS · 2023-03-04T23:49:42Z

Is it worthwhile to separately PR/merge the various fixes and the TC_PartialCompilation=1 enabling change?

Yeah, makes sense. I will hijack this PR to push the fixes.

…ation

ghost assigned AndyAyersMS Jan 13, 2023

dotnet-issue-labeler bot added the area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI label Jan 13, 2023

EgorBo approved these changes Jan 14, 2023

View reviewed changes

Merge remote-tracking branch 'local0/main' into EnablePartialCompilation

d764041

AndyAyersMS mentioned this pull request Feb 3, 2023

JIT: fix issue with partial comp pred list maintenance #81605

Merged

Merge branch 'main' into EnablePartialCompilation

3e91974

AndyAyersMS added 2 commits February 15, 2023 16:11

Merge branch 'main' into EnablePartialCompilation

940614c

Merge remote-tracking branch 'upstream/main' into EnablePartialCompil…

2df5068

…ation

This was referenced Feb 16, 2023

OSX infra issue - prereq check for 'pkg-config' missing #82240

Closed

SupportedLinuxPlatforms_IsSupportedIsTrue fails on distros without quic #82077

Closed

jakobbotsch mentioned this pull request Feb 17, 2023

[QUIC] Missing libmsquic on tested platforms #81901

Closed

4 tasks

Merge remote-tracking branch 'upstream/main' into EnablePartialCompil…

805d488

…ation

build-analysis bot mentioned this pull request Feb 23, 2023

Tracking issue for CI build timeouts #76454

Closed

AndyAyersMS closed this Feb 23, 2023

AndyAyersMS reopened this Feb 23, 2023

don't block GC while waiting for PC continuation to jit

e961715

jkotas reviewed Feb 24, 2023

View reviewed changes

AndyAyersMS added 2 commits February 24, 2023 14:17

review feedback

18283dc

revise to fix contract violations

52f729f

jkotas reviewed Feb 25, 2023

View reviewed changes

reorder endforbidgc and setlasterror

1a7791f

build-analysis bot mentioned this pull request Feb 25, 2023

NuGet failing with Response status code does not indicate success: 503 (Service Unavailable) dotnet/arcade#11723

Open

5 tasks

fix flow cleanup at partial compilation point

7ff6ed7

AndyAyersMS added 2 commits March 4, 2023 15:51

Merge remote-tracking branch 'upstream/main' into EnablePartialCompil…

1be7a6f

…ation

revert enabling by default

2acc2d9

AndyAyersMS changed the title ~~Enable partial compilation of methods~~ Fixes for partial compilation of methods Mar 4, 2023

AndyAyersMS merged commit c16c7ae into dotnet:main Mar 6, 2023

ghost locked as resolved and limited conversation to collaborators Apr 5, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fixes for partial compilation of methods #80635

Fixes for partial compilation of methods #80635

AndyAyersMS commented Jan 13, 2023 •

edited

Loading

ghost commented Jan 13, 2023

AndyAyersMS commented Jan 13, 2023

EgorBo commented Jan 13, 2023

AndyAyersMS commented Jan 13, 2023 •

edited

Loading

AndyAyersMS commented Jan 14, 2023

EgorBo left a comment •

edited

Loading

AndyAyersMS commented Jan 17, 2023

AndyAyersMS commented Feb 3, 2023

AndyAyersMS commented Feb 6, 2023

AndyAyersMS commented Feb 24, 2023

jkotas Feb 24, 2023

AndyAyersMS Feb 24, 2023

jkotas Feb 25, 2023

jkotas Feb 24, 2023

AndyAyersMS Feb 25, 2023

AndyAyersMS commented Feb 24, 2023

AndyAyersMS commented Feb 24, 2023

AndyAyersMS commented Feb 24, 2023

jkotas Feb 25, 2023

AndyAyersMS Feb 25, 2023

AndyAyersMS commented Feb 25, 2023

AndyAyersMS commented Feb 25, 2023

azure-pipelines bot commented Feb 25, 2023

AndyAyersMS commented Feb 25, 2023 •

edited

Loading

AndyAyersMS commented Feb 26, 2023

AndyAyersMS commented Feb 26, 2023

azure-pipelines bot commented Feb 26, 2023

AndyAyersMS commented Feb 26, 2023 •

edited

Loading

EgorBo commented Feb 27, 2023

AndyAyersMS commented Feb 27, 2023

BruceForstall commented Mar 4, 2023

AndyAyersMS commented Mar 4, 2023

Fixes for partial compilation of methods #80635

Fixes for partial compilation of methods #80635

Conversation

AndyAyersMS commented Jan 13, 2023 • edited Loading

ghost commented Jan 13, 2023

AndyAyersMS commented Jan 13, 2023

EgorBo commented Jan 13, 2023

AndyAyersMS commented Jan 13, 2023 • edited Loading

AndyAyersMS commented Jan 14, 2023

EgorBo left a comment • edited Loading

Choose a reason for hiding this comment

AndyAyersMS commented Jan 17, 2023

AndyAyersMS commented Feb 3, 2023

AndyAyersMS commented Feb 6, 2023

AndyAyersMS commented Feb 24, 2023

jkotas Feb 24, 2023

Choose a reason for hiding this comment

AndyAyersMS Feb 24, 2023

Choose a reason for hiding this comment

jkotas Feb 25, 2023

Choose a reason for hiding this comment

jkotas Feb 24, 2023

Choose a reason for hiding this comment

AndyAyersMS Feb 25, 2023

Choose a reason for hiding this comment

AndyAyersMS commented Feb 24, 2023

AndyAyersMS commented Feb 24, 2023

AndyAyersMS commented Feb 24, 2023

jkotas Feb 25, 2023

Choose a reason for hiding this comment

AndyAyersMS Feb 25, 2023

Choose a reason for hiding this comment

AndyAyersMS commented Feb 25, 2023

AndyAyersMS commented Feb 25, 2023

azure-pipelines bot commented Feb 25, 2023

AndyAyersMS commented Feb 25, 2023 • edited Loading

AndyAyersMS commented Feb 26, 2023

AndyAyersMS commented Feb 26, 2023

azure-pipelines bot commented Feb 26, 2023

AndyAyersMS commented Feb 26, 2023 • edited Loading

EgorBo commented Feb 27, 2023

AndyAyersMS commented Feb 27, 2023

BruceForstall commented Mar 4, 2023

AndyAyersMS commented Mar 4, 2023

AndyAyersMS commented Jan 13, 2023 •

edited

Loading

AndyAyersMS commented Jan 13, 2023 •

edited

Loading

EgorBo left a comment •

edited

Loading

AndyAyersMS commented Feb 25, 2023 •

edited

Loading

AndyAyersMS commented Feb 26, 2023 •

edited

Loading