Skip to content

Suppress OSR for crossgen2 execution - #62968

Merged
AndyAyersMS merged 2 commits into
dotnet:mainfrom
AndyAyersMS:ExcludeOSRForCrossgen2Execution
Dec 18, 2021
Merged

Suppress OSR for crossgen2 execution#62968
AndyAyersMS merged 2 commits into
dotnet:mainfrom
AndyAyersMS:ExcludeOSRForCrossgen2Execution

Conversation

@AndyAyersMS

Copy link
Copy Markdown
Member

In CI testing, crossgen2 currently is run via a .NET 6 runtime, and
that runtime has some bugs in OSR.

Work around by suppressing OSR for the duration of the run. We should
be able to revert this once we update the crossgen2 runtime
to a .NET 7 version.

In CI testing, crossgen2 currently is run via a .NET 6 runtime, and
that runtime has some bugs in OSR.
Work around by suppressing OSR for the duration of the run. We should
be able to revert this once we update the crossgen2 runtime
to a .NET 7 version.
@ghostghost added the area-ReadyToRun label Dec 17, 2021
@AndyAyersMS

Copy link
Copy Markdown
MemberAuthor

cc @trylek @dotnet/jit-contrib

This should allow the jit-experimental CI legs to run cleanly.

@AndyAyersMSAndyAyersMS mentioned this pull request Dec 17, 2021
72 tasks
@AndyAyersMS

Copy link
Copy Markdown
MemberAuthor

/azp run runtime-jit-experimental

@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines successfully started running 1 pipeline(s).

@trylektrylek left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks great, thank you! These suppressions are a nasty hack that's likely to continue to bite us in the future, I'm wondering whether we might be able to devise some more robust scheme, I'll think about it during the holidays ;-).

@AndyAyersMS

Copy link
Copy Markdown
MemberAuthor

So far: one crossgen2 test doesn't use the normal execution path, so is still getting run and is failing for OSR: readytorun\multifolder. Will push a commit to try and fix that once the rest of the tests have finished.

@trylek

Copy link
Copy Markdown
Member

Hmm, sorry about that one, I think that was originally created by me, it was supposed to exercise then new multi-component composite builds that are to this day beyond the scope of scripting used for "normal" CG2 tests.

@trylek

Copy link
Copy Markdown
Member

In other words, if you end up having a hard time to make the test work, it shouldn't be a big deal to just disable it with an issues.targets entry and the core runtime team will tackle it as part of the .NET 7 feature work.

@AndyAyersMS

Copy link
Copy Markdown
MemberAuthor

It looks like it can get a similar fix -- it may end up disabling OSR for the running of the test as well, but that seems ok for now.

@AndyAyersMS

Copy link
Copy Markdown
MemberAuthor

That turned out to be the only issue remaining.

@MichalStrehovsky

Copy link
Copy Markdown
Member

Looks great, thank you! These suppressions are a nasty hack that's likely to continue to bite us in the future, I'm wondering whether we might be able to devise some more robust scheme, I'll think about it during the holidays ;-).

Crossgen2 is one of the 1st party candidates for NativeAOT adoption in .NET 7. If we AOT compile crossgen2, these environment variables will no longer affect it. AOT compiling crossgen2 will also have other advantages based on my past experiments (#37411 (comment)).

@AndyAyersMS
AndyAyersMS merged commit 2abbe5b into dotnet:mainDec 18, 2021
@AndyAyersMS
AndyAyersMS deleted the ExcludeOSRForCrossgen2Execution branch December 18, 2021 07:09
@ghostghost locked as resolved and limited conversation to collaborators Jan 17, 2022
Sign up for freeto subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants

@AndyAyersMS@trylek@MichalStrehovsky@BruceForstall