Skip to content

Conversation

@KevinRansom
Copy link
Contributor

This fixes

  1. The BuildTools Sku ngens the FSharp binaries, however, the dependencies don't match the ones realized at run time, due to ngen using the VS binding redirects.
    Fixed by configuring ngen to use the fsc.exe.config and fsi.exe.config in the build tools.

  2. FSharp assemblies are not ngen'd for for Enterprise, Professional and Community Skus
    The fix is to add the necessary metadata in the VisualFSharpFull project to the project references to enable ngen.

  3. BuildTools sku failed to deploy the Microsoft.FSharp.Overrides.NetSdk.targets

//cc @jmarolf, @brettfo

@cartermp
Copy link
Contributor

Link #5518

@KevinRansom KevinRansom reopened this Aug 17, 2018
@KevinRansom
Copy link
Contributor Author

Merging to fixngen branch to enable signed build.

@KevinRansom KevinRansom merged commit d31e16f into dotnet:fixngen Aug 17, 2018
folder "InstallDir:Common7\IDE\CommonExtensions\Microsoft\FSharp"
file source="$(BinariesFolder)\net40\bin\fsc.exe" vs.file.ngen=yes

file source=$(BinariesFolder)\net40\bin\fsc.exe vs.file.ngen=yes vs.file.ngenArchitecture=All vs.file.ngenPriority=2 vs.file.ngenApplication="[installDir]\Common7\IDE\CommonExtensions\Microsoft\FSharp\fsc.exe"
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why ngen priority 2 instead of 1?

@KevinRansom
Copy link
Contributor Author

@jmarolf

That is a great question:

<snip: https://docs.microsoft.com/en-us/dotnet/framework/tools/ngen-exe-native-image-generator#Anchor_3 >
2 | Native images are generated and installed without waiting for idle time, but after all priority 1 actions (and their dependencies) have completed.

So F# will be done quickly, but after everything that is appropriately marked "genuinely important to be ngened imediately"

Does that make sense ?

Kevin

@davidglassborow
Copy link
Contributor

Was this fixed in the 15.8.1 release of VS2017 ?

@cartermp
Copy link
Contributor

@davidglassborow No, 15.8.1 was a high-priority bug fix for VS crashing with Web (non-core) projects.

brettfo added a commit that referenced this pull request Sep 6, 2018
@kkm000
Copy link

kkm000 commented Sep 13, 2018

It would be great to get a ping here when the version of VS containing the fix is known. I noticed the compile time increase, but, having come from the C++ land, did not pay much attention, as anything under 30 seconds is still blazing fast. :)

Looks like the F# compile time in 15.8.3 did not improve?

@cartermp
Copy link
Contributor

@kkm000 No, 15.8.3 was unrelated to F#. We're plumbing the fix through to the correct servicing release now that the fix has been thoroughly validated. This one took time because it's very high-impact. Any slip-up on our part could degrade other parts of the product.

@kkm000
Copy link

kkm000 commented Sep 13, 2018

@cartermp, thank you for the response, no worries at all! I am at a loss for words to express how much I appreciate the new quick updating philosophy embraced by the VS team, and your opening your work here on GitHub! In the days past, when a bug in VS11 C++/CLI-emitted metadata hit us, we waited for months until the service pack 1 was released, holding on a fix and clinging to the old DLLs...

As I said, it's really a minor tooling performance issue, and has a known workaround. What I was asking for is just please drop a message to this thread when the fix is assigned a VS version. Just whenever it happens, no rush at all. Keeping a product of a VS complexity churning for literally millions of users, each of whom has their own ideas how to (ab)use it, is certainly not an easy feat. Kudos!

@KevinRansom KevinRansom deleted the ngen_fix branch September 15, 2018 18:10
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.

5 participants