Improve NuGet.targets perf #1866
Conversation
* Add BuildInParallel to MSBuild tasks * Add SkipNonexistentTarget support * Reduce TargetFramework iteration
emgarten
force-pushed
the
dev-emgarten-targetPerf2
branch
from
cbc76d6 to
5911b69
Compare
emgarten
requested review from
rohit21agrawal,
PatoBeltran,
nkolev92,
zhili1208,
mishra14 and
jainaashish
|
@jainaashish I've opened NuGet/Home#6315 for this |
There was a problem hiding this comment.
Thanks for improving perf, hopefully we can find a way to make this on by default.
I'd also like to investigate if we'd get any perf gains by using IBuildEngine3.BuildProjectsInParallel
| // When BuildInParallel is used with ContinueOnError it does not continue in | ||
| // some scenarios. | ||
| // Allow opt in to msbuild 15.5 behavior with NUGET_RESTORE_MSBUILD_USESKIPNONEXISTENT | ||
| var nonExistFlag = Environment.GetEnvironmentVariable("NUGET_RESTORE_MSBUILD_USESKIPNONEXISTENT"); |
There was a problem hiding this comment.
I'm not in love with the fact that this is entirely opt-in. If there's a smart way for us to detect they're using MSBuild 15, I think it should be on by default and be opt-out. What do you think?
There was a problem hiding this comment.
Do you have any ideas on how to get that? Since this is only for nuget.exe I was planning to fix that in a later PR. MSBuild and dotnet will use the 15.5 features by default.
Ideally it would determine this in the target, but the only property I could find was for 15.0 and it did not include the minor version.
NuGet could potentially probe the msbuild.exe it finds to read the assembly version, it might be worth the cost of this.
| %(_MSBuildProjectReferenceExistent.SetPlatform); | ||
| $(_GenerateRestoreGraphProjectEntryInputProperties)" | ||
| RemoveProperties="%(_MSBuildProjectReferenceExistent.GlobalPropertiesToRemove)"> | ||
| SkipNonexistentTargets="true" |
There was a problem hiding this comment.
@emgarten
Any reason fro not adding SkipNonexistentProjects="true" in this call?
The dg file output from the restore target is identical to before the changes and that is covered by existing tests.
The only behavior change here is that invalid projects will now cause restore to fail since we can exclude projects without restore targets through
SkipNonexistentTargetand no longer need to useContinueOnErrorfor probing projects. I've verified this manually, but adding automated tests for this requires MSBuild 15.5 on the test machines which isn't yet available.Fixes NuGet/Home#6310
Fixes NuGet/Home#6311
Fixes NuGet/Home#6312
Fixes NuGet/Home#6061
//cc @rrelyea @jeffkl @AndyGerlicher @mikeharder