Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Implement standalone GC loading in NativeAOT #91038

Merged
merged 19 commits into from
Oct 6, 2023

Conversation

cshung
Copy link
Member

@cshung cshung commented Aug 23, 2023

This is currently a prototype - the silly naming reflects that. It probably doesn't work in the integrated settings yet, creating this PR to test only.

Highlights:

  • Known to work on Windows/Linux targetting 8.0 preview 7 to power ASP.Net BasicMinimalApi benchmark
  • Module loading code is confirmed to be not in the final binary using dead code elimination in the linker.
  • MSBuild integration

Lowlights:

  • We have to build a NativeAOT-specific variant of the clrgc. (See Converge Representations between NativeAOT and CoreCLR #91821 for the progress towards fixing this)
  • All diagnostics events from CLRGC are not fired
  • Naming of various classes
  • Reading module name from configuration
  • Fallback to default GC in the standalone mode if the configuration is not set.
  • Proper error reporting

@dotnet/gc

@ghost
Copy link

ghost commented Aug 23, 2023

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

Issue Details

This is currently a prototype - the silly naming reflects that. It probably doesn't work in the integrated settings yet, creating this PR to test only.

Highlights:

  • Known to work on Windows/Linux targetting 8.0 preview 7 to initialize the standalone GC and run Hello World.
  • Module loading code is confirmed to be not in the final binary using dead code elimination in the linker.

Lowlights:

  • MSBuild integration
  • Naming of various classes
  • Reading module name from configuration
  • Fallback to default GC in the standalone mode if the configuration is not set.
Author: cshung
Assignees: cshung
Labels:

area-NativeAOT-coreclr

Milestone: -

@cshung cshung marked this pull request as draft August 25, 2023 01:00
@cshung cshung force-pushed the public/native-clrgc branch 2 times, most recently from 4792c9d to f7c2083 Compare September 5, 2023 19:30
@cshung cshung force-pushed the public/native-clrgc branch from f7c2083 to f0ca71c Compare September 6, 2023 17:14
@cshung cshung force-pushed the public/native-clrgc branch 3 times, most recently from 0533b9c to 5d0031a Compare September 7, 2023 20:08
@cshung cshung force-pushed the public/native-clrgc branch from 5d0031a to 08ab7b4 Compare September 13, 2023 00:18
@cshung cshung force-pushed the public/native-clrgc branch 2 times, most recently from d930166 to a52c6a7 Compare September 28, 2023 19:50
@cshung cshung force-pushed the public/native-clrgc branch from a52c6a7 to c789a56 Compare September 29, 2023 21:12
@cshung cshung marked this pull request as ready for review October 2, 2023 21:32
@cshung cshung force-pushed the public/native-clrgc branch from c7b48e7 to fc2554d Compare October 3, 2023 19:07
@cshung cshung added the NO-MERGE The PR is not ready for merge yet (see discussion for detailed reasons) label Oct 4, 2023
@cshung cshung removed the NO-MERGE The PR is not ready for merge yet (see discussion for detailed reasons) label Oct 4, 2023
@MichalStrehovsky
Copy link
Member

Look like something is broken after applying the code review feedback. Debugging now, meanwhile, do not merge.

Do we want a test so that the CI tests this? How is standalone GC tested on CoreCLR?

src/coreclr/gc/gcload.cpp Outdated Show resolved Hide resolved
@cshung
Copy link
Member Author

cshung commented Oct 5, 2023

Look like something is broken after applying the code review feedback. Debugging now, meanwhile, do not merge.

Do we want a test so that the CI tests this? How is standalone GC tested on CoreCLR?

StandaloneGC is supposed to be tested here as a separate pipeline that turns it on for all tests. To my surprise, the pipeline is currently disabled. I will follow up with the @dotnet/gc team on this.

src/coreclr/gc/gcload.cpp Outdated Show resolved Hide resolved
Copy link
Member

@jkotas jkotas left a comment

Choose a reason for hiding this comment

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

LGTM. Thank you!

@cshung cshung merged commit cdd3255 into dotnet:main Oct 6, 2023
@cshung cshung deleted the public/native-clrgc branch October 6, 2023 22:42
@lewing
Copy link
Member

lewing commented Oct 7, 2023

@lewing
Copy link
Member

lewing commented Oct 7, 2023

I opened #93181 based on the error message

@ghost ghost locked as resolved and limited conversation to collaborators Nov 7, 2023
Sign up for free to 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