Optimize SDK resolution in dotnet build

[ladipro]

Fixes #9506

Context

In dotnet build, resolving in-box Sdk's has suboptimal performance. Resolver assemblies are loaded into the process even in cases where the project asks for Sdk's like Microsoft.NET.Sdk, which can be looked up trivially. And in fact, they are looked up trivially but only after resolver assemblies have been loaded and invoked.

The inefficiency is shown by the recently added logging:

There is also a subtle and confusing functional difference between MSBuild.exe and dotnet build where a NuGet package may take precedence over an in-box Sdk in dotnet build, but the same is not possible in MSBuild.exe.

Changes Made

Made the default resolver handle the Sdk resolution first, before external resolvers are consulted. This is better perf-wise and aligns with MSBuild.exe, please see the large code comment for details.

This eliminates all but the last log event: The SDK "Microsoft.NET.Sdk" was successfully resolved by the "DefaultSdkResolver" resolver to location "C:\Program Files\dotnet\sdk\9.0.100-alpha.1.24067.2\Sdks\Microsoft.NET.Sdk\Sdk" and version "".

Testing

Existing and new unit tests, manual testing of various builds.

Perf testing: dotnet build that builds a project depending on in-box Sdks is ~10 ms faster and loads 5 fewer assemblies, as long as it's built with MSBuildEnableWorkloadResolver=false (see below).

Notes

While we don't have to ask the MSBuildWorkloadSdkResolver and NuGetSdkResolver to resolve in-box Sdk's anymore, this change is unfortunately not avoiding these resolvers in most builds. They are still loaded to resolve the special Microsoft.NET.SDK.WorkloadAutoImportPropsLocator Sdk, unless workloads are disabled.

[ladipro]

While we don't have to ask the MSBuildWorkloadSdkResolver and NuGetSdkResolver to resolve in-box Sdk's anymore, this change is unfortunately not avoiding these resolvers in most builds. They are still loaded to resolve the special Microsoft.NET.SDK.WorkloadAutoImportPropsLocator Sdk, unless workloads are disabled.

@dsplaisted, do you think we can avoid resolving Microsoft.NET.SDK.WorkloadAutoImportPropsLocator when evaluating projects that don't use workloads? It would save 5 assembly loads + running some code, in hot scenarios measured at ~10 ms of overhead.

[dsplaisted]

While we don't have to ask the MSBuildWorkloadSdkResolver and NuGetSdkResolver to resolve in-box Sdk's anymore, this change is unfortunately not avoiding these resolvers in most builds. They are still loaded to resolve the special Microsoft.NET.SDK.WorkloadAutoImportPropsLocator Sdk, unless workloads are disabled.

@dsplaisted, do you think we can avoid resolving Microsoft.NET.SDK.WorkloadAutoImportPropsLocator when evaluating projects that don't use workloads? It would save 5 assembly loads + running some code, in hot scenarios measured at ~10 ms of overhead.

There's not an easy way to do this, because that SDK is imported before we know whether workloads are going to be used. Even if we got rid of that import, we would still then import Microsoft.NET.SDK.WorkloadManifestTargetsLocator, which we need to import to determine if a workload is used, and probably would have the same perf characteristics as the PropsLocator (both of them need to figure out what workload manifests to load, so right now the PropsLocator is the one that takes that hit, but TargetsLocator would have to do basically the same thing if PropsLocator hadn't done it).

I think to speed this up we would need to speed up the resolver itself, or find a way to cache its data better.

[ladipro]

@dsplaisted, do you think we can avoid resolving Microsoft.NET.SDK.WorkloadAutoImportPropsLocator when evaluating projects that don't use workloads? It would save 5 assembly loads + running some code, in hot scenarios measured at ~10 ms of overhead.

There's not an easy way to do this, because that SDK is imported before we know whether workloads are going to be used. Even if we got rid of that import, we would still then import Microsoft.NET.SDK.WorkloadManifestTargetsLocator, which we need to import to determine if a workload is used, and probably would have the same perf characteristics as the PropsLocator (both of them need to figure out what workload manifests to load, so right now the PropsLocator is the one that takes that hit, but TargetsLocator would have to do basically the same thing if PropsLocator hadn't done it).

I think to speed this up we would need to speed up the resolver itself, or find a way to cache its data better.

Thank you. Can we realistically defer these imports until WorkloadMSBuildSdkResolver is invoked to resolve a "real" Sdk? I may not be understanding the full complexity but it looks as though if all (user-specified) Sdk's are resolved by simply finding the matching directory under Sdks, then we know for sure no workload is used. Only if we end up actually calling WorkloadMSBuildSdkResolver, there is a possibility it is a workload (or it can be a NuGet-delivered Sdk - we have to bite the bullet either way).

[dsplaisted]

Thank you. Can we realistically defer these imports until WorkloadMSBuildSdkResolver is invoked to resolve a "real" Sdk? I may not be understanding the full complexity but it looks as though if all (user-specified) Sdk's are resolved by simply finding the matching directory under Sdks, then we know for sure no workload is used. Only if we end up actually calling WorkloadMSBuildSdkResolver, there is a possibility it is a workload (or it can be a NuGet-delivered Sdk - we have to bite the bullet either way).

@ladipro A project doesn't explicitly specify an MSBuild SDK import to depend on a workload. Workloads can be enabled by any MSBuild property, and that mapping is defined in the WorkloadManifest.targets file of each workload manifest. For example, if the target platform is Android, then the Android workload will be required, and the RunAOTCompilation property enables the wasm-tools workload for Blazor projects. How this works is covered in this design doc: https://github.com/dotnet/designs/blob/main/accepted/2020/workloads/workload-resolvers.md.

Basically we can't know whether a project uses a workload or not without first running the workloads resolver and processing the .targets files that come from the workload manifests.

[ladipro]

@ladipro A project doesn't explicitly specify an MSBuild SDK import to depend on a workload. Workloads can be enabled by any MSBuild property, and that mapping is defined in the WorkloadManifest.targets file of each workload manifest. For example, if the target platform is Android, then the Android workload will be required, and the RunAOTCompilation property enables the wasm-tools workload for Blazor projects. How this works is covered in this design doc: https://github.com/dotnet/designs/blob/main/accepted/2020/workloads/workload-resolvers.md.

Basically we can't know whether a project uses a workload or not without first running the workloads resolver and processing the .targets files that come from the workload manifests.

Ah, I was completely off base then. Ok, so the next best thing we can do is have resolvers statically declare their priority so at least the NuGet resolver is pay-for-play - loaded and called only when actually needed. That should be mostly MSBuild work. Anyway, thank you!