Activate Server mode for .NET Core compiler

[dsyme]

Adjust GC settings based on analysis in #13739

• Turn on Server GC mode for .NET 6/7 tools
• Turn off Batch mode for .NET 6/7 fsc.dll and .NET Framework fsc.exe, fscAnyCpu.exe etc.

Turning off Batch mode also decouples us from the impact of dotnet/runtime#74286 (except for needing the temporary workaround #13736 until these changes flow into an SDK compiler we can consume back here)

There are likely other GC tuning settings we could look into, see https://docs.microsoft.com/en-us/dotnet/core/runtime-config/garbage-collector

[vzarytovskii]

Curious - what is the difference on your machine after the change?

[vzarytovskii]

Nvm, found it on another issue

[dsyme]

I'm doing some more validation here

[dsyme]

I've put additional end-to-end build time validation in #13739. My analysis was incorrect - one multicore is taken into account, Server GC is a win.

This means we should instead turn on server GC for .NET 6/7

[dsyme]

@vzarytovskii You mentioned that you thought arcade would handle server mode - it looks like it needs to go in this config file??

[dsyme]

However I'm sceptical that this is the runtimeconfig.template.json used for the final fsc.dll put into the .NET SDK, because this setting rollForwardOnNoCandidateFx doesn't seem to appear in the SDK fsc.runtimeconfig.json, which currently looks like this:

C:\Program Files\dotnet\sdk\7.0.100-preview.7.22377.5\FSharp\fsc.runtimeconfig.json

{
  "runtimeOptions": {
    "tfm": "net7.0",
    "framework": {
      "name": "Microsoft.NETCore.App",
      "version": "7.0.0-preview.7.22375.6"
    },
    "configProperties": {
      "System.Reflection.Metadata.MetadataUpdater.IsSupported": false
    }
  }
}

[dsyme]

The place where Arcade specifies the runtimeconfig.template.json to use is here:

<Project>
  <PropertyGroup>
    <!--
      When building a .NET Core exe make sure to include the template runtimeconfig.json file
      which has all of our global settings
    -->
    <UserRuntimeConfig Condition="'$(TargetFrameworkIdentifier)' == '.NETCoreApp' AND '$(OutputType)' == 'Exe'">$(RepositoryEngineeringDir)config\runtimeconfig.template.json</UserRuntimeConfig>
  </PropertyGroup>
</Project>

Presumably this config changes when building the actual .NET SDK. I'm concerned that Arcade is not giving us any way to insert the settings we need for fsc

[baronfel]

all of the 'layout' portion of the SDK with respect to F# happens in this project file.

This is used to publish the F# folder by calling the Publish target here.

So I'd look for a way to make that project take the already-templated outputs from dotnet/fsharp if possible?

[dsyme]

Yeah somehow fsc.runtimeconfig.json and fsi.runtimeconfif.json are being generated, I'll leave the details for others to work out. In short we want Server GC mode enabled for the compiler :)

[dsyme]

I validated that when using these changes (including for bootstrap), the wall time for .\build -noVisualStudio drops from 123sec to 112sec, a 9% gain.

Using -noVisualStudio is important as it causes the bootstrap compiler to run with .NET 6 (not .NET Framework).

[dsyme]

Unfortunately, activating Server GC across all compononents means we hit the problem where fsyacc gives an exception, at least in CI

FSYACC : error FSY000: Object reference not set to an instance of an object. [D:\a\_work\1\s\src\Compiler\FSharp.Compiler.Service.fsproj]

Until we sort that out we should just enable it for fsc

[vzarytovskii]

Got the stand-alone repro for the NRE in fsyacc:
https://github.com/vzarytovskii/fsyacc-issue-repro

[dsyme]

@vzarytovskii In principle this can go in. But someone needs to own it to check it's how we should be doing it. And make sure that the necessary config setting is actually used in the .NET SDK

[safesparrow]

FWIW At my company we've been manually setting Server GC when starting up Rider, which then gets propagated to all child processes, including Dotnet/MSBuild/FSC. Nice to see that this might become the default 👍

[vzarytovskii]

We probably need to do something on the SDK side, since those file aren't being propagated when we generate those. @baronfel what would be the easiest way for us to have servergc enabled in sdk?

[baronfel]

@vzarytovskii fsc is put into the SDK via the tool_fsc.csproj project, which copies files from the transport packages. I believe that project is what's generating the runtimeconfig.json.

I then see that runtimeconfig.json copied in this target for fsc and fsi - so perhaps adding the project properties above would be all that's needed?

@dsplaisted/@joeloff is this the correct interpretation of the tool runtimeconfig generation process?

[dsyme]

@vzarytovskii I updated this based on @dsplaisted's feedback, which makes it much simpler

Could I have you and the Microsoft team take this over? I do think it's a highly significant perf win for the compiler.

[dsplaisted]

Once this goes in and is merged into the .NET SDK, someone should verify that the updated setting actually makes it into the runtimeconfig that the F# compiler uses there.

[kerams]

Will this make it into .NET 7 RTM next week?

[vzarytovskii]

Will this make it into .NET 7 RTM next week?

no, .NET 7 freeze happened month or so ago, when 17.5 got branched.

[vzarytovskii]

Oh, I remember why server gc was not merged, rc1, which is used in 17.5 branch, has a GC bug with fslexyacc crashing, rc2 has a fix, but is not supported by signing gate, and was failing our signed builds.

[vzarytovskii]

This will likely need to be reverted, till when rc2+ will be supported in signed builds.

[vzarytovskii]

Once this goes in and is merged into the .NET SDK, someone should verify that the updated setting actually makes it into the runtimeconfig that the F# compiler uses there.

OK. With help from @baronfel, we figured out that it doesn't end up in runtimeconfig.
We should either pack those files, or add the msbuild property to the corresponding csproj in sdk repo

https://github.com/dotnet/sdk/blob/a50339473c041a9099a27b808b09e1faf506920b/src/Layout/redist/targets/GenerateLayout.targets#L237-L271