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

Runtime packs don't use trimmed binaries #48655

Open
marek-safar opened this issue Feb 23, 2021 · 8 comments
Open

Runtime packs don't use trimmed binaries #48655

marek-safar opened this issue Feb 23, 2021 · 8 comments

Comments

@marek-safar
Copy link
Contributor

The runtime packs build as part of libraries build don't use output produced by ILLinker even if it's run during the build. The ILLink produces fully trimmed runtime packs into /artifacts/bin/ILLinkTrimAssembly/{version}/trimmed-runtimepack/. This is not optimal for few reasons

  • They are bigger than they need to be (although they diff is small)
  • They ship with additional unneeded dependencies which makes it harder to work with in some scenarios (e.g. AOT)
  • They don't match the shared framework build which AFAIK ships trimmed libraries

@ViktorHofer @eerhardt

@dotnet-issue-labeler
Copy link

I couldn't figure out the best area label to add to this issue. If you have write-permissions please help me learn by adding exactly one area label.

@dotnet-issue-labeler dotnet-issue-labeler bot added the untriaged New issue has not been triaged by the area owner label Feb 23, 2021
@ghost
Copy link

ghost commented Feb 23, 2021

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

Issue Details

The runtime packs build as part of libraries build don't use output produced by ILLinker even if it's run during the build. The ILLink produces fully trimmed runtime packs into /artifacts/bin/ILLinkTrimAssembly/{version}/trimmed-runtimepack/. This is not optimal for few reasons

  • They are bigger than they need to be (although they diff is small)
  • They ship with additional unneeded dependencies which makes it harder to work with in some scenarios (e.g. AOT)
  • They don't match the shared framework build which AFAIK ships trimmed libraries

@ViktorHofer @eerhardt

Author: marek-safar
Assignees: -
Labels:

area-Infrastructure-libraries, untriaged

Milestone: -

@ViktorHofer
Copy link
Member

They don't match the shared framework build which AFAIK ships trimmed libraries

I don't know about if the shared framework packages contain trimmed or untrimmed assets but we don't even test on crossgend bits today in libraries. The required work to fix the overall issue is to depend on the Microsoft.NETCore.App bundle that lives in the installer partition instead of depending on the fake shared framework that we currently generated in libraries.

That will make sure that we test with the "exact" same bits that flow into dotnet/sdk.

@ViktorHofer
Copy link
Member

cc @jkoritzinsky @jkotas

@eerhardt
Copy link
Member

Isn't this the same discussion that was rejected in #31712?

The libraries in the runtime packs get ILLink run on them in isolation as part of their build and that output is placed in the runtimepack. We discard the output of ILLink that sees the complete shared framework because of the reasons outlined in #31712 (comment) - the effect this has on the inner dev loop.

cc @stephentoub

@jkotas
Copy link
Member

jkotas commented Feb 23, 2021

This was also discussed in #487. As @ViktorHofer said, the libraries tests do not run against shipping bits today that is a glaring test hole.

The inner dev loop can continue to use non-trimmed non-crossgened bits like it does today. #31712 (comment) . I understand that there are concerns some types of failures may not get detected in the inner dev loop if we do this; but I do not see any other better solution to fix the test hole.

@ViktorHofer ViktorHofer added good first issue Issue should be easy to implement, good for first-time contributors and removed good first issue Issue should be easy to implement, good for first-time contributors labels Feb 23, 2021
@marek-safar
Copy link
Contributor Author

We discard the output of ILLink that sees the complete shared framework

Isn't that quite wasteful? We run the whole trimming process on runtime pack and generate 160 libraries for 40 or so runtime packs to achieve what exactly?

Does this also mean we cannot use linker in libraries build to produce platform or architecture-specific assemblies?

@eerhardt
Copy link
Member

We run the whole trimming process on runtime pack and generate 160 libraries for 40 or so runtime packs to achieve what exactly?

We get the ILLink warnings from it and baseline the existing warnings. See #38033.

We are then burning down the warnings in #45623.

Does this also mean we cannot use linker in libraries build to produce platform or architecture-specific assemblies?

For the libraries outside of System.Private.CoreLib, this is correct. That was also discussed in #31712.

@ViktorHofer ViktorHofer removed the untriaged New issue has not been triaged by the area owner label Mar 24, 2021
@ViktorHofer ViktorHofer added this to the Future milestone Mar 24, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants