fsharp icon indicating copy to clipboard operation
fsharp copied to clipboard

Ensure that FSharp.Core is trimmable.

Open teo-tsirpanis opened this issue 3 years ago • 3 comments

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.

teo-tsirpanis avatar Mar 09 '22 16:03 teo-tsirpanis

@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?

vzarytovskii avatar Mar 09 '22 16:03 vzarytovskii

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.

teo-tsirpanis avatar Mar 09 '22 16:03 teo-tsirpanis

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

KevinRansom avatar Mar 09 '22 18:03 KevinRansom

I believe this is now complete, the final step %A support will be saitisfied by this PR: https://github.com/dotnet/fsharp/pull/15079

There are no known remaining issues.

KevinRansom avatar Apr 17 '23 16:04 KevinRansom

@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.

teo-tsirpanis avatar Apr 17 '23 16:04 teo-tsirpanis

I will take a look thanks.

KevinRansom avatar Apr 17 '23 17:04 KevinRansom

@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.

KevinRansom avatar Apr 18 '23 20:04 KevinRansom

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.

teo-tsirpanis avatar Apr 18 '23 20:04 teo-tsirpanis

@teo-tsirpanis excellent.

Thanks

KevinRansom avatar Apr 18 '23 20:04 KevinRansom

@teo-tsirpanis , hey I just chatted with @dsyme about some of the edits you propose: for example: https://github.com/dotnet/fsharp/commit/2053eb1f6b0dab2c0487526b042d0f7344c66798#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

KevinRansom avatar Apr 20 '23 17:04 KevinRansom

@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 https://github.com/dotnet/fsharp/issues/15980 to track trimming annotations

charlesroddie avatar Sep 14 '23 21:09 charlesroddie