-
Notifications
You must be signed in to change notification settings - Fork 831
Fix ngen of compiler #5525
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
Fix ngen of compiler #5525
Conversation
|
Link #5518 |
|
Merging to fixngen branch to enable signed build. |
| 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" |
There was a problem hiding this comment.
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?
|
That is a great question: <snip: https://docs.microsoft.com/en-us/dotnet/framework/tools/ngen-exe-native-image-generator#Anchor_3 > So F# will be done quickly, but after everything that is appropriately marked "genuinely important to be ngened imediately" Does that make sense ? Kevin |
|
Was this fixed in the 15.8.1 release of VS2017 ? |
|
@davidglassborow No, 15.8.1 was a high-priority bug fix for VS crashing with Web (non-core) projects. |
|
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? |
|
@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. |
|
@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! |
This fixes
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.
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.
BuildTools sku failed to deploy the Microsoft.FSharp.Overrides.NetSdk.targets
//cc @jmarolf, @brettfo