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

Ensure that FSharp.Core is trimmable. #12819

Closed
Tracked by #13398
teo-tsirpanis opened this issue Mar 9, 2022 · 11 comments
Closed
Tracked by #13398

Ensure that FSharp.Core is trimmable. #12819

teo-tsirpanis opened this issue Mar 9, 2022 · 11 comments
Assignees
Labels
Area-Library Issues for FSharp.Core not covered elsewhere Feature Request Impact-Medium (Internal MS Team use only) Describes an issue with moderate impact on existing code.
Milestone

Comments

@teo-tsirpanis
Copy link
Contributor

teo-tsirpanis commented Mar 9, 2022

Is your feature request related to a problem? Please describe.

When I trim a simple F# project, the size of FSharp.Core is unchanged. When I manually enable trimming it and after removing the optimization and signature data, its size shrunk by five, from 1.09 MB to 197 KB.

Describe the solution you'd like

Providing these size savings out of the box and in a safe way would be great. The following things need to happen:

  • Ensure that the optimization and signature data are removed by the trimmer. This can happen very easily by embedding an ILLink.Substitutions.xml file to the library. If these files are indeed only used at compile time, I can submit a PR.
  • The library's usage of reflection has to be audited, and the appropriate attributes must be applied to avoid any unpleasant surprises. If the trimmer does not recignize these attributes by name, we would have to multi-target FSharp.Core to .NET 5+.
    • After that, adding [<assembly: AssemblyMetadata("IsTrimmable", "True")>] will ensure that FSharp.Core will get trimmed.

I am not sure whether the first bullet point requires the second.

Describe alternatives you've considered

N/A

Additional context

This issue is not about making F# in general friendly to trimming or AOT, but about the minimum required work to make just FSharp.Core trimmable.

@vzarytovskii
Copy link
Member

@teo-tsirpanis curious - how does trimmer decide what to get rid of? Is there a spec for it? Should (can?) we be marking explicitly what's safely trimmable?

@teo-tsirpanis
Copy link
Contributor Author

To the best of my knowledge it starts from the entry point, recursively marks the types and methods it references, and removes everything else. The problem is reflection, because we can't statically determine with certainty what code is used and the trimmer warns us about it. So we add special attributes to these methods to help it figure out, or suppress warnings if it can't but we are sure it can't break anything. You can learn more about customizing trimming in https://devblogs.microsoft.com/dotnet/customizing-trimming-in-net-core-5/.

But in general, we don't mark what is trimmable, but what is not, or what is difficult to trim by default.

@KevinRansom
Copy link
Member

@teo-tsirpanis , It's not yet a scenario for us. But it will be ...

@KevinRansom KevinRansom added Plan Impact-Medium (Internal MS Team use only) Describes an issue with moderate impact on existing code. and removed Plan labels Mar 14, 2022
@dsyme dsyme mentioned this issue Apr 20, 2022
3 tasks
@dsyme dsyme added the Area-Library Issues for FSharp.Core not covered elsewhere label Apr 20, 2022
@vzarytovskii vzarytovskii moved this to Not Planned in F# Compiler and Tooling Jun 17, 2022
@vzarytovskii vzarytovskii added this to the August-2022 milestone Aug 2, 2022
@psfinaki psfinaki self-assigned this Aug 4, 2022
@vzarytovskii vzarytovskii assigned KevinRansom and unassigned psfinaki Aug 4, 2022
@vzarytovskii vzarytovskii modified the milestones: December-2022, Backlog Jan 5, 2023
@KevinRansom
Copy link
Member

I believe this is now complete, the final step %A support will be saitisfied by this PR:
#15079

There are no known remaining issues.

@github-project-automation github-project-automation bot moved this from Not Planned to Done in F# Compiler and Tooling Apr 17, 2023
@teo-tsirpanis
Copy link
Contributor Author

@KevinRansom FSharp.Core is still not annotated; APIs that do reflection might produce warnings even if they are actually linker-friendly. I had started work on it in https://github.com/teo-tsirpanis/fsharp/tree/fscore-trimming.

@KevinRansom
Copy link
Member

I will take a look thanks.

@KevinRansom
Copy link
Member

@teo-tsirpanis , I took a look at the PR. and will see if there is something we can do for the scenarios you have identified. My preference is to handle this by having bugs containing repros of trim failures, so that we can evaluate the failure and identify how we want to proceed on a case by case basis.

As for annotations, I suppose there are some Apis that obviously won't work, but for now, I would prefer to see bugs where developers are arguing for why the Api should be available and doesn't work.

For example the work for the --reflectionfree completely disabled apis using sprintf "%A" which was probably not the best solution, and I expect that the recent submissions have reversed that requirement.

@teo-tsirpanis
Copy link
Contributor Author

Makes sense, annotating absolutely everything is very hard. I might bring some of these annotations (the easy ones) in a PR in the future, but don't otherwise bother.

@KevinRansom
Copy link
Member

@teo-tsirpanis excellent.

Thanks

@KevinRansom
Copy link
Member

@teo-tsirpanis , hey I just chatted with @dsyme about some of the edits you propose:
for example:
2053eb1#diff-06bb7bec1ea92205ec6f4e69f53cdf3aa53e3431db58ca15a30c7d426ab6f24bR2747

We discussed this and we believe they are legacy APIs that shouldn't occur when compiled with a recent F# compiler unless witness passing is disabled.

If you have an examples of these APIs failing with trimming we would love to take a look and see what it would take to ensure they work.

Thanks Kevin

@charlesroddie
Copy link
Contributor

@KevinRansom FSharp.Core is still not annotated; APIs that do reflection might produce warnings even if they are actually linker-friendly. I had started work on it in https://github.com/teo-tsirpanis/fsharp/tree/fscore-trimming.

I created #15980 to track trimming annotations

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area-Library Issues for FSharp.Core not covered elsewhere Feature Request Impact-Medium (Internal MS Team use only) Describes an issue with moderate impact on existing code.
Projects
Archived in project
Development

No branches or pull requests

6 participants