-
Notifications
You must be signed in to change notification settings - Fork 1.4k
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
Avoid double evaluation of all ProjectReferences #1276
Comments
Targeted mitigation for dotnet#1276. VC++ noticed a dramatic regression in solution-load performance related to the time cost of ResolveProjectReferences. That affects all projects, but we can safely assume that a .vcxproj is NOT using cross-targeting, while it's impossible to detect from the outside whether a .csproj is. This commit avoids the regression for C++ projects by just not calling the MSBuild task again for .vcxproj references.
Targeted mitigation for dotnet#1276. VC++ noticed a dramatic regression in solution-load performance related to the time cost of ResolveProjectReferences. That affects all projects, but we can safely assume that a .vcxproj is NOT using cross-targeting, while it's impossible to detect from the outside whether a .csproj is. This commit avoids the regression for C++ projects by just not calling the MSBuild task again for .vcxproj references.
Targeted mitigation for #1276. VC++ noticed a dramatic regression in solution-load performance related to the time cost of ResolveProjectReferences. That affects all projects, but we can safely assume that a .vcxproj is NOT using cross-targeting, while it's impossible to detect from the outside whether a .csproj is. This commit avoids the regression for C++ projects by just not calling the MSBuild task again for .vcxproj references.
@nguerrera Any progress on this? Thanks! |
Update on this: I worked hard on the approach we white boarded and it's doable, but too risky of a change for RTW, I've gotten far enough to determine that it can be done without breaking changes (unless you consider not calling GetTargetFrameworkProperties when we don't have to breaking. I don't.) So we can fix this in an update. Meanwhile #1621 includes some escape hatches to turn it off or just force a selection, so that can be used as a workaround for anyone hitting the perf issue until we arrive with the improved protocol in an update. |
Sounds great. I agree that that wouldn't be a breaking change. |
Just to add some additional data here, looking at trying to reduce build times over on project system, and this one is floating to the top as our slowest targets:
|
Nick gave me his prototype branch; I'm planning to pick this up this week. |
Not sure what you mean? On some up-to-date builds, _GetProjectReferenceTargetFrameworkProperties is 63% of the build time - is that expected? |
Re-reading above, maybe i need a little more guidance I'm not really understanding your conclusion - or what we can do get build time down. |
@davkean can you point me toward a repro of that extreme percentage? I can walk through that specific case and make sure my reasoning still holds. In general, my theory is that: evaluation is fast and |
Before cross-targeting was implemented, a project had a single output that was queried in ResolveProjectReferences by either `GetTargetPath` (in single-project-build scenarios) or the default target (`Build`) in command line builds. A cross-targeting project doesn't have a single output--it has one for each of its `TargetFrameworks`. A ProjectReference, then, has to first figure out which output to ask for--the one that's the best fit for the current project. The initial implementation of this was simple: unconditionally call `GetTargetFrameworkProperties` on every ProjectReference, passing a `ReferringTargetFramework`. Then, if the project cross-targets, specify the resulting TF on future calls to the ProjectReference. If the project has a single TF, do not specify it to avoid building it once (from the solution build) and again (from a reference, explicitly specifying the one TF). Unfortunately, adding a global property for `ReferringTargetFramework` will result in a separate evaluation for the project. This means that almost _every_ project in a solution will be evaluated at least twice--once directly, with no special global properties, and once for each unique `ReferringTargetFramework` that refers to it. This can be particularly onerous for C++ projects (.vcxproj), where evaluation time is nontrivial and doubling it is noticeable. That was mitigated by special-casing vcxproj in dotnet#1278, but that is inelegant and doesn't address the root cause. This change alters the calling pattern for `ProjectReference`s. Instead of asking all references for the correct TF, it breaks the problem into multiple phases: * Ask all references whether they cross-target, and split into cross-targeting and non-cross-targeting lists. * Perform the `GetTargetFrameworkProperties` query only on projects that reported that they cross-targeted. * Update (in place) the existing `_MSBuildProjectReferenceExistent` item to pass or undefine the relevant properties based on whether the reference cross-targets (and what TF is the best match). Fixes dotnet#1276.
Picking this up again. I'm setting a testbed up: Building this Roslyn project (at that version), after having built the whole repo (to restore packages). In a "design-time-like" way, meaning
On my machine with VS 26730.0.d15rel, that takes
The 36 references are each evaluated twice, with some projects with lots of globbed includes taking more than 500 ms total evaluation time. |
Resurrecting dotnet/sdk#993 and #1866 cuts the time dramatically, as hoped (
target diffs in the private MSBuild folderdiff --git a/MSBuild/15.0/Bin/Microsoft.Common.CurrentVersion.targets b/MSBuild/15.0/Bin/Microsoft.Common.CurrentVersion.targets
index 531d63f..36fcd53 100644
--- a/MSBuild/15.0/Bin/Microsoft.Common.CurrentVersion.targets
+++ b/MSBuild/15.0/Bin/Microsoft.Common.CurrentVersion.targets
@@ -910,7 +910,7 @@ Copyright (C) Microsoft Corporation. All rights reserved.
Projects="@(_MSBuildProjectReferenceExistent)"
Targets="BuildGenerateSources"
BuildInParallel="$(BuildInParallel)"
- Properties="%(_MSBuildProjectReferenceExistent.SetConfiguration); %(_MSBuildProjectReferenceExistent.SetPlatform); %(_MSBuildProjectReferenceExistent.SetTargetFramework);"
+ Properties="%(_MSBuildProjectReferenceExistent.SetConfiguration); %(_MSBuildProjectReferenceExistent.SetPlatform); %(_MSBuildProjectReferenceExistent.SetDesiredProperties);"
Condition="'$(BuildPassReferences)' == 'true' and '@(ProjectReferenceWithConfiguration)' != '' and '@(_MSBuildProjectReferenceExistent)' != '' and '%(_MSBuildProjectReferenceExistent.BuildReference)' == 'true'"
ContinueOnError="!$(BuildingProject)"
RemoveProperties="%(_MSBuildProjectReferenceExistent.GlobalPropertiesToRemove)">
@@ -935,7 +935,7 @@ Copyright (C) Microsoft Corporation. All rights reserved.
Projects="@(_MSBuildProjectReferenceExistent)"
Targets="BuildCompile"
BuildInParallel="$(BuildInParallel)"
- Properties="%(_MSBuildProjectReferenceExistent.SetConfiguration); %(_MSBuildProjectReferenceExistent.SetPlatform); %(_MSBuildProjectReferenceExistent.SetTargetFramework)"
+ Properties="%(_MSBuildProjectReferenceExistent.SetConfiguration); %(_MSBuildProjectReferenceExistent.SetPlatform); %(_MSBuildProjectReferenceExistent.SetDesiredProperties)"
Condition="'$(BuildPassReferences)' == 'true' and '@(ProjectReferenceWithConfiguration)' != '' and '@(_MSBuildProjectReferenceExistent)' != '' and '%(_MSBuildProjectReferenceExistent.BuildReference)' == 'true'"
ContinueOnError="!$(BuildingProject)"
RemoveProperties="%(_MSBuildProjectReferenceExistent.GlobalPropertiesToRemove)">
@@ -960,7 +960,7 @@ Copyright (C) Microsoft Corporation. All rights reserved.
Projects="@(_MSBuildProjectReferenceExistent)"
Targets="BuildLink"
BuildInParallel="$(BuildInParallel)"
- Properties="%(_MSBuildProjectReferenceExistent.SetConfiguration); %(_MSBuildProjectReferenceExistent.SetPlatform); %(_MSBuildProjectReferenceExistent.SetTargetFramework)"
+ Properties="%(_MSBuildProjectReferenceExistent.SetConfiguration); %(_MSBuildProjectReferenceExistent.SetPlatform); %(_MSBuildProjectReferenceExistent.SetDesiredProperties)"
Condition="'$(BuildPassReferences)' == 'true' and '@(ProjectReferenceWithConfiguration)' != '' and '@(_MSBuildProjectReferenceExistent)' != '' and '%(_MSBuildProjectReferenceExistent.BuildReference)' == 'true'"
ContinueOnError="!$(BuildingProject)"
RemoveProperties="%(_MSBuildProjectReferenceExistent.GlobalPropertiesToRemove)">
@@ -1511,33 +1511,18 @@ Copyright (C) Microsoft Corporation. All rights reserved.
<!--
====================================================================================
- _GetProjectReferenceTargetFrameworkProperties
+ _GetProjectReferencesThatNeedProperties
- Builds the GetTargetFrameworkProperties target of all existent project references,
- passing $(TargetFrameworkMoniker) as $(ReferringTargetFramework) and sets the
- SetTargetFramework metadata of the project reference to the value that is returned.
+ Builds the ShouldQueryForProperties target of all existent project references,
+ removing any TargetFramework and RuntimeIdentifier properties that are set as
+ global properties in this project.
- This allows a cross-targeting project to select how it should be configured to
- build against the most appropriate target for the referring target framework.
+ This allows a cross-targeting project to indicate that it should be asked
+ for a compatible TargetFramework before building.
======================================================================================
-->
- <Target Name="_GetProjectReferenceTargetFrameworkProperties"
- Outputs="%(_MSBuildProjectReferenceExistent.Identity)">
- <!--
- Honor SkipGetTargetFrameworkProperties=true metadata on project references
- to mean that the project reference is known not to target multiple frameworks
- and the mechanism here for selecting the best one can be skipped as an optimization.
-
- We give this treatment to .vcxproj by default since no .vcxproj can target more
- than one framework.
- -->
- <ItemGroup>
- <_MSBuildProjectReferenceExistent Condition="'%(_MSBuildProjectReferenceExistent.SkipGetTargetFrameworkProperties)' == '' and '%(Extension)' == '.vcxproj'">
- <SkipGetTargetFrameworkProperties>true</SkipGetTargetFrameworkProperties>
- </_MSBuildProjectReferenceExistent>
- </ItemGroup>
-
+ <Target Name="_GetProjectReferencesThatNeedProperties">
<!--
Allow project references to specify which target framework properties to set and their values
without consulting the referenced project. This has two use cases:
@@ -1547,15 +1532,58 @@ Copyright (C) Microsoft Corporation. All rights reserved.
there is also a .NETFramework implementation.
2. As an escape hatch for cases where the compatibility check performed by
- GetTargetFrameworkProperties is faulty.
+ GetDesiredProperties is faulty.
-->
<ItemGroup>
- <_MSBuildProjectReferenceExistent Condition="'%(_MSBuildProjectReferenceExistent.SetTargetFramework)' != ''">
+ <_MSBuildProjectReferenceExistent Condition="'%(_MSBuildProjectReferenceExistent.SetDesiredProperties)' != ''">
<SkipGetTargetFrameworkProperties>true</SkipGetTargetFrameworkProperties>
</_MSBuildProjectReferenceExistent>
</ItemGroup>
<!--
+ Honor SkipGetTargetFrameworkProperties=true metadata on project references
+ to mean that the project reference is known not to target multiple frameworks
+ and the mechanism here for selecting the best one can be skipped as an optimization.
+ -->
+ <MSBuild
+ Projects="@(_MSBuildProjectReferenceExistent)"
+ Targets="ShouldQueryForProperties"
+ BuildInParallel="$(BuildInParallel)"
+ Properties="%(_MSBuildProjectReferenceExistent.SetConfiguration); %(_MSBuildProjectReferenceExistent.SetPlatform)"
+ ContinueOnError="!$(BuildingProject)"
+ RemoveProperties="%(_MSBuildProjectReferenceExistent.GlobalPropertiesToRemove);$(PropertiesToRemoveWhenQueryingForDesiredProperties)"
+ Condition="'%(_MSBuildProjectReferenceExistent.SkipGetTargetFrameworkProperties)' != 'true'">
+
+ <Output TaskParameter="TargetOutputs" ItemName="_MSBuildProjectReferenceWithDesiredPropertiesResults" />
+ </MSBuild>
+
+ <ItemGroup>
+ <!-- Transform the list so that its Identity correlates with that of _MSBuildProjectReferenceExistent. -->
+ <_MSBuildProjectReferenceWithDesiredProperties Include="@(_MSBuildProjectReferenceWithDesiredPropertiesResults->'%(OriginalItemSpec)')" />
+
+ <!-- Update refs to remove any properties that the reference knows (without the referring TF) it doesn't want. -->
+ <_MSBuildProjectReferenceExistent Condition="'@(_MSBuildProjectReferenceWithDesiredProperties)' == '%(Identity)' and '@(_MSBuildProjectReferenceWithDesiredProperties->'%(QueryForProperties)')' == 'false'">
+ <UndefineProperties>@(_MSBuildProjectReferenceExistent->'%(UndefineProperties)');@(_MSBuildProjectReferenceWithDesiredProperties->'%(UndefineProperties)')</UndefineProperties>
+ </_MSBuildProjectReferenceExistent>
+ </ItemGroup>
+ </Target>
+
+ <!--
+ ====================================================================================
+ _GetProjectReferenceTargetFrameworkProperties
+
+ Builds the GetDesiredProperties target of all existent project references,
+ passing $(TargetFrameworkMoniker) as $(ReferringTargetFramework) and sets the
+ SetDesiredProperties metadata of the project reference to the value that is returned.
+
+ This allows a cross-targeting project to select how it should be configured to
+ build against the most appropriate target for the referring target framework.
+
+ ======================================================================================
+ -->
+ <Target Name="_GetProjectReferenceTargetFrameworkProperties"
+ DependsOnTargets="_GetProjectReferencesThatNeedProperties">
+ <!--
Select the moniker to send to each project reference if not already set. NugetTargetMoniker (NTM) is preferred by default over
TargetFrameworkMoniker (TFM) because it is required to disambiguate the UWP case where TFM is fixed at .NETCore,Version=v5.0 and
has floating NTM=UAP,Version=vX.Y.Z. However, in other cases (classic PCLs), NTM contains multiple values and that will cause the MSBuild
@@ -1567,53 +1595,62 @@ Copyright (C) Microsoft Corporation. All rights reserved.
</PropertyGroup>
<MSBuild
- Projects="%(_MSBuildProjectReferenceExistent.Identity)"
- Targets="GetTargetFrameworkProperties"
+ Projects="@(_MSBuildProjectReferenceWithDesiredProperties)"
+ Targets="GetDesiredProperties"
BuildInParallel="$(BuildInParallel)"
- Properties="%(_MSBuildProjectReferenceExistent.SetConfiguration); %(_MSBuildProjectReferenceExistent.SetPlatform); ReferringTargetFramework=$(ReferringTargetFrameworkForProjectReferences)"
+ Properties="%(_MSBuildProjectReferenceWithDesiredProperties.SetConfiguration); %(_MSBuildProjectReferenceWithDesiredProperties.SetPlatform); ReferringTargetFramework=$(ReferringTargetFrameworkForProjectReferences)"
ContinueOnError="!$(BuildingProject)"
- RemoveProperties="%(_MSBuildProjectReferenceExistent.GlobalPropertiesToRemove);TargetFramework;RuntimeIdentifier"
- Condition="'%(_MSBuildProjectReferenceExistent.SkipGetTargetFrameworkProperties)' != 'true'">
+ RemoveProperties="%(_MSBuildProjectReferenceWithDesiredProperties.GlobalPropertiesToRemove);$(PropertiesToRemoveWhenQueryingForDesiredProperties)"
+ Condition="'%(_MSBuildProjectReferenceWithDesiredProperties.SkipGetTargetFrameworkProperties)' != 'true' and '%(_MSBuildProjectReferenceWithDesiredProperties.QueryForProperties)' == 'true'">
- <Output TaskParameter="TargetOutputs" PropertyName="_ProjectReferenceTargetFrameworkProperties" />
+ <Output TaskParameter="TargetOutputs" ItemName="_ProjectReferenceTargetFrameworkProperties" />
</MSBuild>
<ItemGroup>
- <_MSBuildProjectReferenceExistent Condition="'%(_MSBuildProjectReferenceExistent.Identity)' == '%(Identity)' and '$(_ProjectReferenceTargetFrameworkProperties)' != ''">
- <SetTargetFramework>$(_ProjectReferenceTargetFrameworkProperties)</SetTargetFramework>
+ <!-- Build an item that has Identity matching _MSBuildProjectReferenceExistent and metadata for properties to set. -->
+ <_ProjectReferencesWithTargetFrameworkProperties Include="@(_ProjectReferenceTargetFrameworkProperties->'%(OriginalItemSpec)')" />
- <UndefineProperties Condition="$(_ProjectReferenceTargetFrameworkProperties.Contains(`ProjectHasSingleTargetFramework=true`))">%(_MSBuildProjectReferenceExistent.UndefineProperties);TargetFramework;ProjectHasSingleTargetFramework</UndefineProperties>
- <!-- Unconditionally remove the property that was set as a marker to indicate that for this call we should remove TargetFramework -->
- <UndefineProperties Condition="!$(_ProjectReferenceTargetFrameworkProperties.Contains(`ProjectHasSingleTargetFramework=true`))">%(_MSBuildProjectReferenceExistent.UndefineProperties);ProjectHasSingleTargetFramework</UndefineProperties>
+ <!-- Set the project's returned TargetFramework -->
+ <_MSBuildProjectReferenceExistent Condition="'@(_ProjectReferencesWithTargetFrameworkProperties)' == '%(Identity)' and '@(_ProjectReferencesWithTargetFrameworkProperties->'%(SetDesiredProperties)')' != ''">
+ <SetDesiredProperties>@(_ProjectReferencesWithTargetFrameworkProperties->'%(SetDesiredProperties)')</SetDesiredProperties>
</_MSBuildProjectReferenceExistent>
- </ItemGroup>
- <ItemGroup>
- <_MSBuildProjectReferenceExistent Condition="'%(_MSBuildProjectReferenceExistent.Identity)' == '%(Identity)' and '$(_ProjectReferenceTargetFrameworkProperties)' != ''">
- <UndefineProperties Condition="$(_ProjectReferenceTargetFrameworkProperties.Contains(`ProjectIsRidAgnostic=true`))">%(_MSBuildProjectReferenceExistent.UndefineProperties);RuntimeIdentifier;ProjectIsRidAgnostic</UndefineProperties>
- <!-- Unconditionally remove the property that was set as a marker to indicate that for this call we should remove RuntimeIdentifier -->
- <UndefineProperties Condition="!$(_ProjectReferenceTargetFrameworkProperties.Contains(`ProjectIsRidAgnostic=true`))">%(_MSBuildProjectReferenceExistent.UndefineProperties);ProjectIsRidAgnostic</UndefineProperties>
+ <!-- Remove any properties that the reference didn't want. -->
+ <_MSBuildProjectReferenceExistent Condition="'@(_ProjectReferencesWithTargetFrameworkProperties)' == '%(Identity)' and '@(_ProjectReferencesWithTargetFrameworkProperties->'%(UndefineProperties)')' != ''">
+ <UndefineProperties>@(_MSBuildProjectReferenceExistent->'%(UndefineProperties)');@(_ProjectReferencesWithTargetFrameworkProperties->'%(UndefineProperties)')</UndefineProperties>
</_MSBuildProjectReferenceExistent>
</ItemGroup>
-
- <PropertyGroup>
- <_ProjectReferenceTargetFrameworkProperties />
- </PropertyGroup>
</Target>
<!--
============================================================
- GetTargetFrameworkProperties
+ GetDesiredProperties
Overrridden by cross-targeting projects to return the set of
- properties (in the form "key1=value1;...keyN=valueN") needed
- to build it with the best target for the referring project's
- target framework.
+ properties (in the metadatum SetDesiredProperties` in the
+ form "key1=value1;...keyN=valueN") needed to build against
+ the target framework that best matches the referring
+ project's target framework.
+
+ The referring project's $(TargetFrameworkMoniker) is passed
+ in as $(ReferringTargetFramework).
+ -->
+ <Target Name="GetDesiredProperties" />
+
+ <!--
+ ============================================================
+ ShouldQueryForProperties
+
+ Overrridden by cross-targeting projects to return an item
+ with QueryForProperties metadata that is a boolean set to
+ "true" if the project should have its GetDesiredProperties
+ target called with a ReferringTargetFramework.
- The referring project's $(TargetFrameworkMoniker) is passed
- in as $(ReferringTargetFramework)
+ If the returned QueryForProperties is false, the project
+ will be built with the returned UndefineProperties metadata
+ removed from its global properties.
-->
- <Target Name="GetTargetFrameworkProperties" />
+ <Target Name="ShouldQueryForProperties" />
<!--
============================================================
@@ -1626,8 +1663,8 @@ Copyright (C) Microsoft Corporation. All rights reserved.
[OUT]
@(ProjectReferenceWithConfiguration) - Project references with apporpriate metadata
- @(_MSBuildProjectReferenceExistent) - Subset of @(ProjectReferenceWithConfiguration) that exist
- with added SetTargetFramework metadata for cross-targeting
+ @(_MSBuildProjectReferenceExistent) - Subset of @(ProjectReferenceWithConfiguration) that exist
+ with added SetDesiredProperties metadata for cross-targeting
@(_MSBuildProjectReferenceNonExistent) - Subset of @(ProjectReferenceWithConfiguration) that do not exist
============================================================
-->
@@ -1682,7 +1719,7 @@ Copyright (C) Microsoft Corporation. All rights reserved.
Projects="@(_MSBuildProjectReferenceExistent)"
Targets="GetTargetPath"
BuildInParallel="$(BuildInParallel)"
- Properties="%(_MSBuildProjectReferenceExistent.SetConfiguration); %(_MSBuildProjectReferenceExistent.SetPlatform); %(_MSBuildProjectReferenceExistent.SetTargetFramework)"
+ Properties="%(_MSBuildProjectReferenceExistent.SetConfiguration); %(_MSBuildProjectReferenceExistent.SetPlatform); %(_MSBuildProjectReferenceExistent.SetDesiredProperties)"
Condition="'%(_MSBuildProjectReferenceExistent.BuildReference)' == 'true' and '@(ProjectReferenceWithConfiguration)' != '' and ('$(BuildingInsideVisualStudio)' == 'true' or '$(BuildProjectReferences)' != 'true') and '$(VisualStudioVersion)' != '10.0' and '@(_MSBuildProjectReferenceExistent)' != ''"
ContinueOnError="!$(BuildingProject)"
RemoveProperties="%(_MSBuildProjectReferenceExistent.GlobalPropertiesToRemove)">
@@ -1709,7 +1746,7 @@ Copyright (C) Microsoft Corporation. All rights reserved.
Projects="@(_MSBuildProjectReferenceExistent)"
Targets="%(_MSBuildProjectReferenceExistent.Targets);GetTargetPath"
BuildInParallel="$(BuildInParallel)"
- Properties="%(_MSBuildProjectReferenceExistent.SetConfiguration); %(_MSBuildProjectReferenceExistent.SetPlatform); %(_MSBuildProjectReferenceExistent.SetTargetFramework)"
+ Properties="%(_MSBuildProjectReferenceExistent.SetConfiguration); %(_MSBuildProjectReferenceExistent.SetPlatform); %(_MSBuildProjectReferenceExistent.SetDesiredProperties)"
Condition="'%(_MSBuildProjectReferenceExistent.BuildReference)' == 'true' and '@(ProjectReferenceWithConfiguration)' != '' and ('$(BuildingInsideVisualStudio)' == 'true' or '$(BuildProjectReferences)' != 'true') and '$(VisualStudioVersion)' == '10.0' and '@(_MSBuildProjectReferenceExistent)' != ''"
ContinueOnError="!$(BuildingProject)"
RemoveProperties="%(_MSBuildProjectReferenceExistent.GlobalPropertiesToRemove)">
@@ -1726,7 +1763,7 @@ Copyright (C) Microsoft Corporation. All rights reserved.
Projects="@(_MSBuildProjectReferenceExistent)"
Targets="%(_MSBuildProjectReferenceExistent.Targets)"
BuildInParallel="$(BuildInParallel)"
- Properties="%(_MSBuildProjectReferenceExistent.SetConfiguration); %(_MSBuildProjectReferenceExistent.SetPlatform); %(_MSBuildProjectReferenceExistent.SetTargetFramework)"
+ Properties="%(_MSBuildProjectReferenceExistent.SetConfiguration); %(_MSBuildProjectReferenceExistent.SetPlatform); %(_MSBuildProjectReferenceExistent.SetDesiredProperties)"
Condition="'%(_MSBuildProjectReferenceExistent.BuildReference)' == 'true' and '@(ProjectReferenceWithConfiguration)' != '' and '$(BuildingInsideVisualStudio)' != 'true' and '$(BuildProjectReferences)' == 'true' and '@(_MSBuildProjectReferenceExistent)' != ''"
ContinueOnError="$(ContinueOnError)"
RemoveProperties="%(_MSBuildProjectReferenceExistent.GlobalPropertiesToRemove)">
@@ -1743,7 +1780,7 @@ Copyright (C) Microsoft Corporation. All rights reserved.
Projects="@(_MSBuildProjectReferenceExistent)"
Targets="GetNativeManifest"
BuildInParallel="$(BuildInParallel)"
- Properties="%(_MSBuildProjectReferenceExistent.SetConfiguration); %(_MSBuildProjectReferenceExistent.SetPlatform); %(_MSBuildProjectReferenceExistent.SetTargetFramework)"
+ Properties="%(_MSBuildProjectReferenceExistent.SetConfiguration); %(_MSBuildProjectReferenceExistent.SetPlatform); %(_MSBuildProjectReferenceExistent.SetDesiredProperties)"
Condition="'%(_MSBuildProjectReferenceExistent.BuildReference)' == 'true' and '@(ProjectReferenceWithConfiguration)' != '' and '$(BuildingProject)' == 'true' and '@(_MSBuildProjectReferenceExistent)' != ''"
ContinueOnError="$(ContinueOnError)"
RemoveProperties="%(_MSBuildProjectReferenceExistent.GlobalPropertiesToRemove)">
@@ -2318,7 +2355,7 @@ Copyright (C) Microsoft Corporation. All rights reserved.
<Target Name="GetReferenceTargetPlatformMonikers" DependsOnTargets="PrepareProjectReferences">
<MSBuild
Projects="@(_MSBuildProjectReferenceExistent)"
- Properties="%(_MSBuildProjectReferenceExistent.SetConfiguration); %(_MSBuildProjectReferenceExistent.SetPlatform); %(_MSBuildProjectReferenceExistent.SetTargetFramework)"
+ Properties="%(_MSBuildProjectReferenceExistent.SetConfiguration); %(_MSBuildProjectReferenceExistent.SetPlatform); %(_MSBuildProjectReferenceExistent.SetDesiredProperties)"
Targets="GetTargetPathWithTargetPlatformMoniker"
BuildInParallel="$(BuildInParallel)"
ContinueOnError="!$(BuildingProject)"
@@ -4301,7 +4338,7 @@ Copyright (C) Microsoft Corporation. All rights reserved.
Projects="@(_MSBuildProjectReferenceExistent)"
Targets="GetCopyToOutputDirectoryItems"
BuildInParallel="$(BuildInParallel)"
- Properties="%(_MSBuildProjectReferenceExistent.SetConfiguration); %(_MSBuildProjectReferenceExistent.SetPlatform); %(_MSBuildProjectReferenceExistent.SetTargetFramework)"
+ Properties="%(_MSBuildProjectReferenceExistent.SetConfiguration); %(_MSBuildProjectReferenceExistent.SetPlatform); %(_MSBuildProjectReferenceExistent.SetDesiredProperties)"
Condition="'@(_MSBuildProjectReferenceExistent)' != '' and '$(_GetChildProjectCopyToOutputDirectoryItems)' == 'true' and '%(_MSBuildProjectReferenceExistent.Private)' != 'false' and '$(UseCommonOutputDirectory)' != 'true'"
ContinueOnError="$(ContinueOnError)"
RemoveProperties="%(_MSBuildProjectReferenceExistent.GlobalPropertiesToRemove)">
@@ -4864,7 +4901,7 @@ Copyright (C) Microsoft Corporation. All rights reserved.
<MSBuild
Projects="@(_MSBuildProjectReferenceExistent)"
Targets="Clean"
- Properties="%(_MSBuildProjectReferenceExistent.SetConfiguration); %(_MSBuildProjectReferenceExistent.SetPlatform); %(_MSBuildProjectReferenceExistent.SetTargetFramework)"
+ Properties="%(_MSBuildProjectReferenceExistent.SetConfiguration); %(_MSBuildProjectReferenceExistent.SetPlatform); %(_MSBuildProjectReferenceExistent.SetDesiredProperties)"
BuildInParallel="$(BuildInParallel)"
Condition="'$(BuildingInsideVisualStudio)' != 'true' and '$(BuildProjectReferences)' == 'true' and '@(_MSBuildProjectReferenceExistent)' != ''"
ContinueOnError="$(ContinueOnError)"
diff --git a/MSBuild/15.0/Microsoft.Common.props b/MSBuild/15.0/Microsoft.Common.props
index b04ff5b..defd522 100644
--- a/MSBuild/15.0/Microsoft.Common.props
+++ b/MSBuild/15.0/Microsoft.Common.props
@@ -147,6 +147,10 @@ Copyright (C) Microsoft Corporation. All rights reserved.
<WMSJSProjectDirectory Condition="'$(WMSJSProjectDirectory)' == ''">JavaScript</WMSJSProjectDirectory>
</PropertyGroup>
+ <PropertyGroup>
+ <PropertiesToRemoveWhenQueryingForDesiredProperties Condition="'$(PropertiesToRemoveWhenQueryingForDesiredProperties)' == ''">TargetFramework;RuntimeIdentifier</PropertiesToRemoveWhenQueryingForDesiredProperties>
+ </PropertyGroup>
+
<Import Project="$(MSBuildExtensionsPath)\$(MSBuildToolsVersion)\Microsoft.VisualStudioVersion.v*.Common.props" Condition="'$(VisualStudioVersion)' == ''" />
<!--
diff --git a/MSBuild/Sdks/Microsoft.NET.Sdk/build/Microsoft.NET.Sdk.Common.targets b/MSBuild/Sdks/Microsoft.NET.Sdk/build/Microsoft.NET.Sdk.Common.targets
index 1cdc34e..fad922e 100644
--- a/MSBuild/Sdks/Microsoft.NET.Sdk/build/Microsoft.NET.Sdk.Common.targets
+++ b/MSBuild/Sdks/Microsoft.NET.Sdk/build/Microsoft.NET.Sdk.Common.targets
@@ -28,54 +28,127 @@ Copyright (c) .NET Foundation. All rights reserved.
<DefaultImplicitPackages>Microsoft.NETCore.App;NETStandard.Library</DefaultImplicitPackages>
</PropertyGroup>
+ <!--
+ Some versions of Microsoft.NET.Test.Sdk.targets change the OutputType after we've set _IsExecutable and
+ HasRuntimeOutput default in Microsfot.NET.Sdk.BeforeCommon.targets. Refresh these value here for backwards
+ compatibilty with that.
+ -->
+ <PropertyGroup>
+ <_IsExecutable Condition="'$(OutputType)' == 'Exe' or '$(OutputType)'=='WinExe'">true</_IsExecutable>
+ <HasRuntimeOutput Condition="'$(_UsingDefaultForHasRuntimeOutput)' == 'true'">$(_IsExecutable)</HasRuntimeOutput>
+ </PropertyGroup>
+
+ <PropertyGroup Condition="'$(DotnetCliToolTargetFramework)' == '' And '$(BundledNETCoreAppTargetFrameworkVersion)' != ''">
+ <!-- Set the TFM used to restore .NET CLI tools to match the version of .NET Core bundled in the CLI -->
+ <DotnetCliToolTargetFramework>netcoreapp$(BundledNETCoreAppTargetFrameworkVersion)</DotnetCliToolTargetFramework>
+ </PropertyGroup>
<UsingTask TaskName="GetNearestTargetFramework" AssemblyFile="$(MicrosoftNETBuildTasksAssembly)" />
<UsingTask TaskName="NETSdkError" AssemblyFile="$(MicrosoftNETBuildTasksAssembly)" />
-
+
+ <!--
+ ============================================================
+ ShouldQueryForProperties
+
+ Indicate whether this project cross-targets, so before
+ building it, a referring project must call
+ GetDesiredProperties to get the matching TargetFramework to
+ specify.
+
+ If this project only has a single TF and a single RID,
+ return UndefineProperties to allow the referring project
+ to build it directly without a subsequent query.
+ -->
+ <Target Name="ShouldQueryForProperties"
+ Returns="@(_ShouldQueryForPropertiesResult)">
+ <PropertyGroup>
+ <!-- indicate to caller that project is RID agnostic so that a global property RuntimeIdentifier value can be removed -->
+ <_IsRidAgnostic>false</_IsRidAgnostic>
+ <_IsRidAgnostic Condition=" '$(RuntimeIdentifier)' == '' and '$(RuntimeIdentifiers)' == '' ">true</_IsRidAgnostic>
+
+ <_DesiredRemovedProperties Condition="'$(_IsRidAgnostic)' == 'true'">$(_DesiredRemovedProperties);RuntimeIdentifier</_DesiredRemovedProperties>
+
+ <!-- A project can only have more than one output if the current global properties are such that the current build is a cross-targeting one. -->
+ <_HasSingleTargetFramework Condition="'$(IsCrossTargetingBuild)' != 'true'">true</_HasSingleTargetFramework>
+ <_HasSingleTargetFramework Condition="'$(_HasSingleTargetFramework)' == ''">false</_HasSingleTargetFramework>
+
+ <_DesiredRemovedProperties Condition="'$(_HasSingleTargetFramework)' == 'true'">$(_DesiredRemovedProperties);TargetFramework</_DesiredRemovedProperties>
+
+ <_ReferringProjectShouldCallGetDesiredProperties>true</_ReferringProjectShouldCallGetDesiredProperties>
+ <_ReferringProjectShouldCallGetDesiredProperties Condition="$(_HasSingleTargetFramework) and $(_IsRidAgnostic)">false</_ReferringProjectShouldCallGetDesiredProperties>
+ </PropertyGroup>
+
+ <ItemGroup>
+ <_ShouldQueryForPropertiesResult Include="$(MSBuildProjectFullPath)">
+ <QueryForProperties>$(_ReferringProjectShouldCallGetDesiredProperties)</QueryForProperties>
+ <UndefineProperties Condition="'$(_ReferringProjectShouldCallGetDesiredProperties)' == 'false'">$(_DesiredRemovedProperties)</UndefineProperties>
+ </_ShouldQueryForPropertiesResult>
+ </ItemGroup>
+ </Target>
+
<!--
============================================================
- GetTargetFrameworkProperties
+ GetDesiredProperties
- Invoked by common targets to return the set of properties
- (in the form "key1=value1;...keyN=valueN") needed to build
- against the target framework that best matches the referring
- project's target framework.
+ Invoked by common targets to return the set of properties
+ (in the metadatum SetDesiredProperties` in the form
+ "key1=value1;...keyN=valueN") needed to build against the
+ target framework that best matches the referring project's
+ target framework.
- The referring project's $(TargetFrameworkMoniker) is passed
+ The referring project's $(TargetFrameworkMoniker) is passed
in as $(ReferringTargetFramework).
-
- This is in the common targets so that it will apply to both
- cross-targeted and single-targeted projects. It is run
- for single-targeted projects so that an error will be
- generated if the referenced project is not compatible
- with the referencing project's target framework.
============================================================
-->
- <Target Name="GetTargetFrameworkProperties" Returns="TargetFramework=$(NearestTargetFramework);ProjectHasSingleTargetFramework=$(_HasSingleTargetFramework);ProjectIsRidAgnostic=$(_IsRidAgnostic)">
+ <Target Name="GetDesiredProperties"
+ DependsOnTargets="ShouldQueryForProperties"
+ Returns="@(_ProjectBuildInstructions)">
<PropertyGroup>
- <!-- indicate to caller that project is RID agnostic so that a global property RuntimeIdentifier value can be removed -->
- <_IsRidAgnostic>false</_IsRidAgnostic>
- <_IsRidAgnostic Condition=" '$(RuntimeIdentifier)' == '' and '$(RuntimeIdentifiers)' == '' ">true</_IsRidAgnostic>
-
<!-- If a ReferringTargetFramework was not specified, and we only have one TargetFramework, then don't try to check compatibility -->
<_SkipNearestTargetFrameworkResolution Condition="'$(TargetFramework)' != '' and '$(ReferringTargetFramework)' == ''">true</_SkipNearestTargetFrameworkResolution>
<NearestTargetFramework Condition="'$(_SkipNearestTargetFrameworkResolution)' == 'true'">$(TargetFramework)</NearestTargetFramework>
- <!-- A project can only have more than one output if the current global properties are such that the current build is a cross-targeting one. -->
- <_HasSingleTargetFramework Condition="'$(IsCrossTargetingBuild)' != 'true'">true</_HasSingleTargetFramework>
- <_HasSingleTargetFramework Condition="'$(_HasSingleTargetFramework)' == ''">false</_HasSingleTargetFramework>
+ <_PossibleTargetFrameworks Condition="'$(TargetFramework)' != ''">$(TargetFramework)</_PossibleTargetFrameworks>
+ <_PossibleTargetFrameworks Condition="'$(TargetFramework)' == ''">$(TargetFrameworks)</_PossibleTargetFrameworks>
+ </PropertyGroup>
+
+ <GetNearestTargetFramework ReferringTargetFramework="$(ReferringTargetFramework)"
+ PossibleTargetFrameworks="$(_PossibleTargetFrameworks)"
+ ProjectFilePath="$(MSBuildProjectFullPath)"
+ Condition="'$(_SkipNearestTargetFrameworkResolution)' != 'true'">
+ <Output PropertyName="NearestTargetFramework" TaskParameter="NearestTargetFramework" />
+ </GetNearestTargetFramework>
+
+ <ItemGroup>
+ <_ProjectBuildInstructions Include="$(MSBuildProjectFullPath)">
+ <SetDesiredProperties>TargetFramework=$(NearestTargetFramework)</SetDesiredProperties>
+ <UndefineProperties>$(_DesiredRemovedProperties)</UndefineProperties>
+ </_ProjectBuildInstructions>
+ </ItemGroup>
+ </Target>
+
+ <!-- This target is a compat shim, allowing the SDK to continue to function with older common targets that haven't switched to GetDesiredProperties.
+
+ TODO: After the SDK has picked up an MSBuild with https://github.com/Microsoft/msbuild/pull/1866, it should be deleted. -->
+ <Target Name="GetTargetFrameworkProperties" Returns="TargetFramework=$(NearestTargetFramework);ProjectHasSingleTargetFramework=$(_HasSingleTargetFramework);ProjectIsRidAgnostic=$(_IsRidAgnostic)"
+ DependsOnTargets="ShouldQueryForProperties">
+
+ <PropertyGroup>
+ <!-- If a ReferringTargetFramework was not specified, and we only have one TargetFramework, then don't try to check compatibility -->
+ <_SkipNearestTargetFrameworkResolution Condition="'$(TargetFramework)' != '' and '$(ReferringTargetFramework)' == ''">true</_SkipNearestTargetFrameworkResolution>
+ <NearestTargetFramework Condition="'$(_SkipNearestTargetFrameworkResolution)' == 'true'">$(TargetFramework)</NearestTargetFramework>
<_PossibleTargetFrameworks Condition="'$(TargetFramework)' != ''">$(TargetFramework)</_PossibleTargetFrameworks>
<_PossibleTargetFrameworks Condition="'$(TargetFramework)' == ''">$(TargetFrameworks)</_PossibleTargetFrameworks>
</PropertyGroup>
- <GetNearestTargetFramework ReferringTargetFramework="$(ReferringTargetFramework)"
+ <GetNearestTargetFramework ReferringTargetFramework="$(ReferringTargetFramework)"
PossibleTargetFrameworks="$(_PossibleTargetFrameworks)"
ProjectFilePath="$(MSBuildProjectFullPath)"
Condition="'$(_SkipNearestTargetFrameworkResolution)' != 'true'">
<Output PropertyName="NearestTargetFramework" TaskParameter="NearestTargetFramework" />
</GetNearestTargetFramework>
</Target>
-
+
</Project> |
I wrote up some detailed notes about the problem and options for solving it: https://gist.github.com/rainersigwald/fdf2be017223bf90850abf06ffa32169. I talked to @nguerrera @davkean and @terrajobst about it a while ago and settled on the “Query for TFs; Choose in Consumer” approach as the option that looked the best. In particular, the improved error messages available from consumption-side TF selection are a big win. Going down that road requires
Formalizing multitargetingRight now multitargeting is defined in MSBuild (crosstargeting.targets and the existing ask-project-what-TF-to-build code is in common.targets), and in the SDK. Since any project type can reference a multitargeted project, we should just admit that it's a core MSBuild concept now. That means taking ownership of the definitions of multitargetable dimensions (currently that's just I don't think anything really has to change for this, just mindset. Elevating TargetFramework compatibilityDeciding what TF from a list is most compatible with the a given TF is currently handled by NuGet.Frameworks, which is an optional add-in to MSBuild. It’s delivered in the SDK (that's how the current give-me-your-TF-properties code works) but isn’t available in some MSBuild scenarios. Ideally, the compat matrix would be represented as data, and we could query it with a Framework method. At the moment, that doesn't appear likely to happen. I propose that we add a reference to NuGet.Frameworks from Microsoft.Build.Tasks.Core.dll, and add a new target to perform the select-best-match-for-reference-or-error functionality. This is conceptually similar to the existing I've prototyped that in #2472. It does not include nice error messages yet. Changing the ProjectReference protocolThis is tricky because it is necessarily a breaking change . . . but only for projects that manually implement the ProjectReference protocol and don't include common.targets. If that’s too breaking for a minor release of VS, we could add an an engine feature (#2471) to avoid failing on ProjectReferences to projects that don't have a I have prototyped the change to the protocol in a way that's backward-compatible (as long as you import common.targets/crosstargeting.targets) in #2472. Getting rid of double evaluation for multitargeted projectsThis isn't possible in general, because with no knowlege of a project we can't know whether it multitargets or what TF to request, so we have to evaluate it in some context to figure out the right context to evaluate it in for real. BUT! The list of supported
When resolving P's references with a cache:
This allows us to maintain correctness if the TFs supported by a reference change (we still ask what it supports) but save an evaluation in the (common) case that the best-match TF is the same as it was last build. |
Add GetReferenceNearestTargetFrameworkTask Related: dotnet/msbuild#1276
This was merged in: #2595. |
As of #1018, the
ResolveProjectReferences
flow (which is executed during solution load in VS and is therefore very perf-sensitive) is two-phase:_GetProjectReferenceTargetFrameworkProperties
to ask the reference "What TFM of yours should I build to match my current TFM?"ResolveProjectReferences
to ask the reference "What is your primary output for this TFM?"Those evaluations have different sets of global properties (the first sets
ReferringTargetFramework
and the latter setsSetTargetFramework
), so they must be evaluated separately. Neither called target takes very long, but the evaluation is not cheap. This means that time toResolveProjectReferences
has roughly doubled in cost for all projects.Cost of crosstargeting should be on a pay-for-play basis as much as possible. Existing csproj -> existing csproj (ditto vcxproj) shouldn't be slowed down by this.
/cc @nguerrera
The text was updated successfully, but these errors were encountered: