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

Make FSharp.Core a normal component (makes optdata/sigdata files unnecessary when recent compiler and FSharp.Core can be assumed) #2884

Merged
merged 5 commits into from
Apr 19, 2017

Conversation

dsyme
Copy link
Contributor

@dsyme dsyme commented Apr 19, 2017

F# DLLs contain special resources for F#-specific data. For whatever reasons, in the past FSharp.Core split these out as separate files called FSharp.Core.sigdata and FSharp.Core.optdata.

This PR reduces the complexity of F# engineering by incorporating the sigdata/optdata information into FSharp.Core just like any other normal F# DLL. This makes the sigdata/optdata files unnecessary when using more recent FSharp.Core DLLs.

The PR doesn't require a bump in FSharp.Core version (though a new signed FSharp.Core 4.4.1.0 is produced). My belief is that it can be integrated as soon as practical.

Links:

Old-compiler compat

The PR is sensitive to the fact that older F# compilers may end up referencing new-style FSharp.Core. For older F# compilers, the sigdata/optdata files are still required - their presence is ignored by newer F# compilers. This means we can choose to continue to ship the sigdata/optdata files (e.g. in the FSharp.Core nuget package) for any future FSharp.Core.dll, for as long as we believe that older compilers may end up referencing these new FSharp.Core.

There are thus no setup changes in this PR. The optdata/sigdaata files are still produced. They just become unnecessary whenever a newer F# compiler and newer FSharp.Core can be assumed. This is precisely the case for most uses of FSharp.Formatting and FSharp.Compiler.Service to analyze or run scripts.

Old compiler compat is implemented by using an alternative name for the relevant resources. I tested old compiler compat manually by doing this using the VS2017 fsc.exe.

fsc --noframework -r:"C:\GitHub\dsyme\visualfsharp\release\net40\bin\FSharp.Core.dll" -r:"C:\Program Files (x86)\Reference Assemblies\Microsoft\Framework\.NETFramework\v4.6\mscorlib.dll" a.fs

Background and Rationale

The aim of the sigdata/optdata files was to reduce the "minimum download size"of F# apps for mobile app programming. The decision was made on size assumptions valid circa 2008 - approx 9 years ago. The split has no other benefit (e.g. no performance gain, since the pages containing the data are not loaded). The size reduction for the compressed .NET 4.x FSharp.Core.dll was ~250KB.

The pain inflicted on the broader F# ecosystem by doing this split has frankly been horrific. For example, the split induces downstream complexity on anyone trying to do F# compilation or type checking analysis using FSharp.Compiler.Service (see bugs related to sigdata), or code formatting using FSharp.Formatting (see issues related to sigdata ), or just accidentally referencing an FSharp.Core where the files had not been copied with the component, or forgetting to add these files to a project - again and again we have seen really bad bugs and great community frustration at what should be one of the simplest parts of the F# ecosystem.

Splitting out these files was just a mistake. It isn't done for any other F# DLL. The tiny gain is just not worth it: F# is rarely used in Windows Store Apps (the original motivation). It is used in Xamarin apps, though in that context we have advice that shaving a couple of hundred KB off app download size is not a significant gain (approx quote: "these days splash screens are 4MB")

Notes

  • The capability to either create sigdata/optdata files or omit the data altogether using explicit command line arguments is still maintained. But these flags are not used for FSharp.Core.
  • If we ever want to revisit size issues, there would be other ways to reduce size, e.g. by compressing the sigdata/optdata, or by reducing the use of size-inducing constructs in the implementation of FSharp.Core

@KevinRansom KevinRansom merged commit 4fcfb8b into dotnet:master Apr 19, 2017
@7sharp9
Copy link
Contributor

7sharp9 commented Apr 19, 2017

@dsyme Does the opt/sig data have any interesting information that could be used by consumed by editors for instance?

@dsyme
Copy link
Contributor Author

dsyme commented Apr 19, 2017

@7sharp9 the sigdara resource contains the F# TAST data already made available via Symbols in FCS. The optdata contains the inlining expressions for "let inline" and other inlining data, neither yet available from FCS though not too hard to make available (though harder to make good use of)

@vasily-kirichenko
Copy link
Contributor

@dsyme it would be super interesting to make the code that saved sigdata to save typecheck results to disk in order to populate caches on startup.

@dsyme
Copy link
Contributor Author

dsyme commented Apr 19, 2017

@dsyme it would be super interesting to make the code that saved sigdata to save typecheck results to disk in order to populate caches on startup.

It's probably not too hard to pickle the state of the IncrementalBuilder partial build - I think that's the primary thing that would be needed. A fair number of different data types involved but you would just follow the model of TastPickle.fs, the code is simple enough.

@cloudRoutine
Copy link
Contributor

Dealing with this issue and FSharp.Core binding redirects has been a maddening process that has wasted so much time working on F# tooling and infrastructure projects. I'm ecstatic to see one of them finally go away. 🎊

@forki
Copy link
Contributor

forki commented Apr 20, 2017

Xoxo

@vasily-kirichenko
Copy link
Contributor

@forki are you learning Russian these days? :)

@forki
Copy link
Contributor

forki commented Apr 20, 2017

@ImaginaryDevelopment
Copy link

so this is merged, how can I track/find out when a new dll with this feature in it is released?

@cartermp
Copy link
Contributor

cartermp commented Apr 27, 2017

@ImaginaryDevelopment AFAIK this won't be available until a new release of the F# 4.1 compiler SDK is released. Unfortunately, nightlies only have tooling updates - no compiler or FSI updates. I'd like to have nightlies come with updates to everything, but right now nobody as the time to set up that process.

You'll likely see this improvement in the next update of FSharp.Compiler.Tools.

@ImaginaryDevelopment
Copy link

ImaginaryDevelopment commented Apr 27, 2017 via email

@forki
Copy link
Contributor

forki commented Apr 27, 2017 via email

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

9 participants