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

Add clrgc binary to sharedFx #72032

Merged
merged 1 commit into from
Jul 12, 2022
Merged

Add clrgc binary to sharedFx #72032

merged 1 commit into from
Jul 12, 2022

Conversation

mangod9
Copy link
Member

@mangod9 mangod9 commented Jul 12, 2022

Adding clrgc.dll (.so/dylib) to the sharedFx build so we could fallback to GC segments if required.

We dont plan to update sdk to copy things over for "Self-contained" unless it would get copied since its in the right place.

@ghost
Copy link

ghost commented Jul 12, 2022

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

Issue Details

Adding clrgc.dll (.so/dylib) to the sharedFx build so we could fallback to GC segments if required.

We dont plan to update sdk to copy things over for "Self-contained" unless it would get copied since its in the right place.

Author: mangod9
Assignees: -
Labels:

area-GC-coreclr, area-Infrastructure-installer

Milestone: -

@ghost ghost assigned mangod9 Jul 12, 2022
@mangod9
Copy link
Member Author

mangod9 commented Jul 12, 2022

@trylek who would be the best person to review from the infra standpoint?

@trylek
Copy link
Member

trylek commented Jul 12, 2022

Hmm, I'm not 100% sure, perhaps @ViktorHofer could take a first look and decide whether he feels comfortable reviewing the change or recommend someone else.

@jkoritzinsky
Copy link
Member

I'm probably the best person to review this. It looks good to me

@mangod9
Copy link
Member Author

mangod9 commented Jul 12, 2022

Thanks @jkoritzinsky. Do you know what the implication of adding clrgc to Directory.Build.props is?

@jkoritzinsky
Copy link
Member

If you add it to Directory.Build.props, then it will be in the platform manifest for all of the .sfxproj files. That means that we can ship clrgc.dll in any of the shared framework projects there and the SDK won't get confused when restoring files. The CMake change makes the file automatically shipped in any of the shared framework packages that pull CoreCLR files directly (so Microsoft.NETCore.App.Runtime, Microsoft.NETCore.App.Runtime.Composite)

I think this file will automatically be copied as part of self-contained deployment since it's in the shared-framework package, but I don't remember how the SDK pulls files for that. It won't be included in single-file deployment though, I checked how that works.

@Maoni0
Copy link
Member

Maoni0 commented Jul 12, 2022

what's the criteria to include something in SCD? clrgc.dll is about 650k -

 Directory of C:\runtime-exp\artifacts\tests\coreclr\windows.x64.Release\Tests\Core_Root

07/11/2022  02:45 PM           649,728 clrgc.dll

@jkoritzinsky
Copy link
Member

I don't know the criteria, but I think we include anything that's in the runtime pack. I don't think we have an exclusion mechanism for that today.

@Maoni0
Copy link
Member

Maoni0 commented Jul 12, 2022

that's great because I'd like this to be included in SCD :)

@jkoritzinsky
Copy link
Member

Yeah if the file is listed in the framework list (which all files in a runtime pack are), it will automatically be included in the self-contained deployment. There's actually no way today to include files in the runtime pack and not include them in the framework list (we'd have to add a feature to the Shared Framework SDK in Arcade to support that)

@mangod9 mangod9 changed the title [WIP] Add clrgc binary to sharedFx Add clrgc binary to sharedFx Jul 12, 2022
@mangod9
Copy link
Member Author

mangod9 commented Jul 12, 2022

I have verified locally that clrgc.dll makes it into the Microsoft.NETCore.App.Runtime.win-x64.7.0.0-dev package. Merging now.

@mangod9 mangod9 merged commit c5005e0 into dotnet:main Jul 12, 2022
@mangod9 mangod9 deleted the addClrGC branch July 12, 2022 19:47
@mangod9
Copy link
Member Author

mangod9 commented Jul 14, 2022

I have verified on the latest p7 sdk that clrgc.dll is dropped with the sharedFx, and also included for --self-contained deployments.

@ghost ghost locked as resolved and limited conversation to collaborators Sep 7, 2022
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.

5 participants