Read PackageDefinitions and PackageDependencies from the cache assets file instead of ResolvePackageDependencies #28057

ocallesp · 2022-09-20T21:18:42Z

We want to avoid reading the assets file because it is a large file, which can cause performance degradation in design-time builds for large projects when executing ResolvePackageDependencies.

To improve this, the assets cache file in ResolvePackageAssets will now keep track of PackageDefinitions and PackageDependencies.

The change:
Output PackageDefinitions and PackageDependencies items from ResolvePackageAssets, and modified the targets so that ResolvePackageDependencies is only called when EmitLegacyAssetsFileItems is true.

All the code related to EmitLegacyAssetsFileItems in ResolvePackageDependencies was removed since there is case for when the value is false.
Unit-tests were modified to remove EmitLegacyAssetsFileItems in ResolvePackageDependencies.

Removing all the conditions that depend on EmitLegacyAssetsFileItems since there is case when this variable is false.

marcin-krystianc · 2022-09-27T10:19:13Z

I've tried this change and I see an amazing speed improvement. Thank you!

marcin-krystianc · 2022-09-27T12:40:55Z

I've tried this change and I see an amazing speed improvement. Thank you!

Actually, I didn't spot the build errors that are happening after 47e09093:

MSBuild version 17.4.0-preview-22474-01+c7d758591 for .NET
D:\workspace\sdk\artifacts\bin\redist\Release\dotnet\sdk\8.0.100-dev\MSBuild.dll -maxcpucount /m:1 -verbosity:m /v:n /bl /clp:Summary /clp:PerformanceSummary /flp:v=n;PerformanceSummary /nr:false /p:AndroidPreserveUserData=True /p:AndroidUseManagedDesignTimeResourceGenerator=True /p:BuildingByReSharper=True /p:BuildingProject=False /p:BuildProjectReferences=False /p:ContinueOnError=ErrorAndContinue /p:DesignTimeBuild=True /p:DesignTimeSilentResolution=False /p:JetBrainsDesignTimeBuild=True /p:ProvideCommandLineArgs=True /p:ResolveAssemblyReferencesSilent=False /p:SkipCompilerExecution=True /p:TargetFramework=net5.0 /t:GetSuggestedWorkloads;_CheckForInvalidConfigurationAndPlatform;ResolveReferences;ResolveProjectReferences;ResolveAssemblyReferences;ResolveComReferences;ResolveNativeReferences;ResolveSdkReferences;ResolveFrameworkReferences;ResolvePackageDependenciesDesignTime;Compile;CoreCompile .\LargeAppWithPrivatePackagesCentralisedNGBVRemoved.sln
Build started 27/09/2022 14:25:34.
Project "D:\workspace\TestSolutions\LargeAppWithPrivatePackagesCentralisedNGBVRemoved\solution\LargeAppWithPrivatePackagesCentralisedNGBVRemoved.sln" on node 1 (GetSuggestedWorkloads;_CheckForInvalidConfigurationAndPlatform;ResolveReferences;ResolveProjectReferences;ResolveAssemblyReferences;ResolveComReferences;ResolveNativeReferences;ResolveSdkReferences;ResolveFrameworkReferences;ResolvePackageDependenciesDesignTime;Compile;CoreCompile target(s)).
ValidateSolutionConfiguration:
  Building solution configuration "Debug|Any CPU".
Project "D:\workspace\TestSolutions\LargeAppWithPrivatePackagesCentralisedNGBVRemoved\solution\LargeAppWithPrivatePackagesCentralisedNGBVRemoved.sln" (1) is building "D:\workspace\TestSolutions\LargeAppWithPrivatePackagesCentralisedNGBVRemoved\solution\Project47\Project47.fsproj" (2:2) on node 1 (_CheckForInvalidConfigurationAndPlatform target(s)).
_CheckForNETCoreSdkIsPreview:
D:\workspace\sdk\artifacts\bin\redist\Release\dotnet\sdk\8.0.100-dev\Sdks\Microsoft.NET.Sdk\targets\Microsoft.NET.RuntimeIdentifierInference.targets(257,5): message NETSDK1057: You are using a preview version of .NET. See: https://aka.ms/dotnet-support-policy [D:\workspace\TestSolutions\LargeAppWithPrivatePackagesCentralisedNGBVRemoved\solution\Project47\Project47.fsproj]
Done Building Project "D:\workspace\TestSolutions\LargeAppWithPrivatePackagesCentralisedNGBVRemoved\solution\Project47\Project47.fsproj" (_CheckForInvalidConfigurationAndPlatform target(s)).
Project "D:\workspace\TestSolutions\LargeAppWithPrivatePackagesCentralisedNGBVRemoved\solution\LargeAppWithPrivatePackagesCentralisedNGBVRemoved.sln" (1) is building "D:\workspace\TestSolutions\LargeAppWithPrivatePackagesCentralisedNGBVRemoved\solution\Project47\Project47.fsproj" (2:3) on node 1 (ResolveReferences target(s)).
D:\workspace\sdk\artifacts\bin\redist\Release\dotnet\sdk\8.0.100-dev\Sdks\Microsoft.NET.Sdk\targets\Microsoft.PackageDependencyResolution.targets(265,5): error MSB4018: The "ResolvePackageAssets" task failed unexpectedly. [D:\workspace\TestSolutions\LargeAppWithPrivatePackagesCentralisedNGBVRemoved\solution\Project47\Project47.fsproj]
D:\workspace\sdk\artifacts\bin\redist\Release\dotnet\sdk\8.0.100-dev\Sdks\Microsoft.NET.Sdk\targets\Microsoft.PackageDependencyResolution.targets(265,5): error MSB4018: System.IndexOutOfRangeException: Index was outside the bounds of the array. [D:\workspace\TestSolutions\LargeAppWithPrivatePackagesCentralisedNGBVRemoved\solution\Project47\Project47.fsproj]
D:\workspace\sdk\artifacts\bin\redist\Release\dotnet\sdk\8.0.100-dev\Sdks\Microsoft.NET.Sdk\targets\Microsoft.PackageDependencyResolution.targets(265,5): error MSB4018:    at Microsoft.NET.Build.Tasks.ResolvePackageAssets.CacheReader.ReadItem() in D:\workspace\sdk\src\Tasks\Microsoft.NET.Build.Tasks\ResolvePackageAssets.cs:line 664 [D:\workspace\TestSolutions\LargeAppWithPrivatePackagesCentralisedNGBVRemoved\solution\Project47\Project47.fsproj]
D:\workspace\sdk\artifacts\bin\redist\Release\dotnet\sdk\8.0.100-dev\Sdks\Microsoft.NET.Sdk\targets\Microsoft.PackageDependencyResolution.targets(265,5): error MSB4018:    at Microsoft.NET.Build.Tasks.ResolvePackageAssets.CacheReader.ReadItemGroup() in D:\workspace\sdk\src\Tasks\Microsoft.NET.Build.Tasks\ResolvePackageAssets.cs:line 656 [D:\workspace\TestSolutions\LargeAppWithPrivatePackagesCentralisedNGBVRemoved\solution\Project47\Project47.fsproj]
D:\workspace\sdk\artifacts\bin\redist\Release\dotnet\sdk\8.0.100-dev\Sdks\Microsoft.NET.Sdk\targets\Microsoft.PackageDependencyResolution.targets(265,5): error MSB4018:    at Microsoft.NET.Build.Tasks.ResolvePackageAssets.ReadItemGroups() in D:\workspace\sdk\src\Tasks\Microsoft.NET.Build.Tasks\ResolvePackageAssets.cs:line 369 [D:\workspace\TestSolutions\LargeAppWithPrivatePackagesCentralisedNGBVRemoved\solution\Project47\Project47.fsproj]
D:\workspace\sdk\artifacts\bin\redist\Release\dotnet\sdk\8.0.100-dev\Sdks\Microsoft.NET.Sdk\targets\Microsoft.PackageDependencyResolution.targets(265,5): error MSB4018:    at Microsoft.NET.Build.Tasks.ResolvePackageAssets.ExecuteCore() in D:\workspace\sdk\src\Tasks\Microsoft.NET.Build.Tasks\ResolvePackageAssets.cs:line 342 [D:\workspace\TestSolutions\LargeAppWithPrivatePackagesCentralisedNGBVRemoved\solution\Project47\Project47.fsproj]
D:\workspace\sdk\artifacts\bin\redist\Release\dotnet\sdk\8.0.100-dev\Sdks\Microsoft.NET.Sdk\targets\Microsoft.PackageDependencyResolution.targets(265,5): error MSB4018:    at Microsoft.NET.Build.Tasks.TaskBase.Execute() in D:\workspace\sdk\src\Tasks\Common\TaskBase.cs:line 52 [D:\workspace\TestSolutions\LargeAppWithPrivatePackagesCentralisedNGBVRemoved\solution\Project47\Project47.fsproj]
D:\workspace\sdk\artifacts\bin\redist\Release\dotnet\sdk\8.0.100-dev\Sdks\Microsoft.NET.Sdk\targets\Microsoft.PackageDependencyResolution.targets(265,5): error MSB4018:    at Microsoft.Build.BackEnd.TaskExecutionHost.Microsoft.Build.BackEnd.ITaskExecutionHost.Execute() [D:\workspace\TestSolutions\LargeAppWithPrivatePackagesCentralisedNGBVRemoved\solution\Project47\Project47.fsproj]
D:\workspace\sdk\artifacts\bin\redist\Release\dotnet\sdk\8.0.100-dev\Sdks\Microsoft.NET.Sdk\targets\Microsoft.PackageDependencyResolution.targets(265,5): error MSB4018:    at Microsoft.Build.BackEnd.TaskBuilder.ExecuteInstantiatedTask(ITaskExecutionHost taskExecutionHost, TaskLoggingContext taskLoggingContext, TaskHost taskHost, ItemBucket bucket, TaskExecutionMode howToExecuteTask) [D:\workspace\TestSolutions\LargeAppWithPrivatePackagesCentralisedNGBVRemoved\solution\Project47\Project47.fsproj]

...

    0 Warning(s)
    276 Error(s)

Time Elapsed 00:00:32.32

I'm trying to debug the problem and implement a fix.

ocallesp · 2022-09-27T18:14:52Z

Revert PR #28212 just in case it is needed.

marcin-krystianc · 2022-09-27T18:31:26Z

Revert PR #28212

I'm not sure if the revert is necessary. After cleaning the solution, running restore and then testing the design time build it worked. I don't know yet what is going on, I'll keep digging tomorrow.

ocallesp · 2022-09-27T18:44:30Z

This pr adds two new properties to the nuget assets cache, so it will invalidate the old cache.

drewnoakes · 2022-09-27T20:57:11Z

Does the cache file have a version number? Changing the file format without advertising that fact will cause errors for users that are hard to diagnose.

ocallesp · 2022-09-27T21:29:38Z

I created this pr #28218 to increase the format version of the header

src/Tasks/Microsoft.NET.Build.Tasks/ResolvePackageAssets.cs

marcin-krystianc · 2022-09-28T07:03:50Z

This pr adds two new properties to the nuget assets cache, so it will invalidate the old cache.

Yes, indeed. Running dotnet restore prior to the design-time build, fixes the System.IndexOutOfRangeException errors.

drewnoakes · 2022-09-28T12:30:45Z

Yes, indeed. Running dotnet restore prior to the design-time build, fixes the System.IndexOutOfRangeException errors.

Thanks for confirming. We can't ship it as is and will fix it.

ocallesp · 2022-09-28T18:57:00Z

It seems that the only missing change is to update the format version.

@drewnoakes if that is the case I can add your suggested changes in the same pr (in main branch)

drewnoakes · 2022-09-29T00:37:34Z

I'm not aware of any other required changes, but the SDK team should weigh in as they know more about the consequences of your original PR.

ocallesp requested a review from dsplaisted September 20, 2022 21:18

ocallesp force-pushed the fix27738 branch from 427a9f9 to 44606e7 Compare September 20, 2022 21:25

ocallesp requested a review from marcpopMSFT September 20, 2022 21:37

ocallesp changed the title ~~Fix27738~~ Avoid reading PackageDefinitions and PackageDependencies from the assets file in ResolvePackageDependencies to improve design-time builds Sep 20, 2022

ocallesp changed the title ~~Avoid reading PackageDefinitions and PackageDependencies from the assets file in ResolvePackageDependencies to improve design-time builds~~ Read PackageDefinitions and PackageDependencies from the cache assets file instead of ResolvePackageDependencies Sep 20, 2022

ocallesp force-pushed the fix27738 branch from 44606e7 to a35a506 Compare September 21, 2022 06:58

dotnet deleted a comment Sep 21, 2022

ocallesp marked this pull request as ready for review September 21, 2022 06:58

ocallesp force-pushed the fix27738 branch from 536d2e5 to a35a506 Compare September 21, 2022 19:08

ocallesp mentioned this pull request Sep 21, 2022

error MSB3277: Found conflicts between different versions of "System.Runtime" that could not be resolved. dotnet/arcade#10965

Closed

2 tasks

ocallesp force-pushed the fix27738 branch 9 times, most recently from 59f88e6 to dab7d17 Compare September 26, 2022 03:14

ocalles added 3 commits September 25, 2022 22:52

Add output PackageDefinitions to ResolvePackageAssets

7f3b7bb

Add output PackageDependenciesBetweenPackages to ResolvePackageAssets

d62e4f4

ResolvePackageDependencies only when EmitLegacyAssetsFileItem is true

00b0220

Removing all the conditions that depend on EmitLegacyAssetsFileItems since there is case when this variable is false.

ocallesp force-pushed the fix27738 branch from dab7d17 to 00b0220 Compare September 26, 2022 05:53

ocallesp merged commit 47e0909 into main Sep 26, 2022

ocallesp deleted the fix27738 branch September 26, 2022 16:34

ocallesp mentioned this pull request Sep 27, 2022

Revert pr28057 #28212

Merged