[Opt-in] Parallelize Targets when building a solution by yuehuang010 · Pull Request #7512 · dotnet/msbuild

yuehuang010 · 2022-04-01T21:45:09Z

Fixes

#5072 (comment)

Context

When building a SLN, a metaproj is used to represent the build behavior. When there are multiple targets (ex clean;build), the current behavior is to run all of first Target in the projects, then run second Target. To improve the parallelism, the solution can pass both target to the project. Each project can start the second target without waiting for all of the first Target to finish.
When the feature is enabled via environment variable, MSBuildSolutionBatchTargets, Solution Generator will create a "SlnProjectResolveProjectReference" target to build all the project/targets. All targets will depend on this new target.
Add support for "SkipNonexistentProjects" as a metadata in MSBuild task. This allow the removal of it as a parameter during solution generation.

Testing

Add unit tests.

Before:

After:

…ct property in MSBuild task.

…huang/main_solution

src/Build.UnitTests/Construction/SolutionProjectGenerator_Tests.cs

src/Build/Construction/Solution/SolutionProjectGenerator.cs

src/Build/BackEnd/Components/RequestBuilder/IntrinsicTasks/MSBuild.cs

Forgind · 2022-04-05T17:01:00Z

src/Tasks/MSBuild.cs


-                if (FileSystems.Default.FileExists(projectPath) || (_skipNonexistentProjects == SkipNonexistentProjectsBehavior.Build))
+                // Try to get the behavior from metadata if it is undefined.
+                var skipNonexistPropjects = _skipNonexistentProjects;


It looks like this was essentially copied from SolutionProjectGenerator—is it possible to merge them? Are there major differences? Why are they separate in the first place?

I don't know why there are two copies of MSBuild.cs and MSBuild_Tests.cs.

When tests are running, they resolve to BackEnd.

The story here is that long ago, the MSBuild task was just a task that used the IBuildEngine APIs like any other task would be able to do. At some later time, as a performance optimization, the MSBuild task was pulled into the engine itself, and task resolution short-circuits to prefer that version.

The one in Tasks exists only because it was a public non-sealed type, so user types might have inherited from it. I generally wouldn't worry about adding new features to it, since it should be almost entirely dead code.

rainersigwald

When we talked about this problem, I expected something along the lines of synthesizing a target like

<MSBuild Projects="@(Projects)" Targets="Foo;Bar" />

Instead of the existing mechanism of synthesizing a Foo that calls all Foo and a Bar that calls all bar, then calling them in sequence.

Is that viable?

yuehuang010 · 2022-04-18T15:51:47Z

When we talked about this problem, I expected something along the lines of synthesizing a target like
<MSBuild Projects="@(Projects)" Targets="Foo;Bar" />
Instead of the existing mechanism of synthesizing a Foo that calls all Foo and a Bar that calls all bar, then calling them in sequence.

Is that viable?

How would the target interface with the entry point work? @rainersigwald

…huang/main_solution

rainersigwald · 2022-04-26T15:09:48Z

How would the target interface with the entry point work?

Sorry, missed this. I don't think I understand the question.

I was thinking of something like synthesizing a new target based on the targetNames passed to the SolutionProjectGenerator constructor, and having it be, basically,

  <Target Name="Build_A_B_C">
    <MSBuild BuildInParallel="True"
             SkipNonexistentProjects="%(ProjectReference.SkipNonexistentProjects)"
             Projects="@(ProjectReference)"
             Targets="A;B;C"
             Properties="BuildingSolutionFile=true; CurrentSolutionConfigurationContents=$(CurrentSolutionConfigurationContents); SolutionDir=$(SolutionDir); SolutionExt=$(SolutionExt); SolutionFileName=$(SolutionFileName); SolutionName=$(SolutionName); SolutionPath=$(SolutionPath)" />
  </Target>

yuehuang010 · 2022-04-26T15:23:08Z

How would the target interface with the entry point work?

Sorry, missed this. I don't think I understand the question.

I was thinking of something like synthesizing a new target based on the targetNames passed to the SolutionProjectGenerator constructor, and having it be, basically,
  <Target Name="Build_A_B_C">
    <MSBuild BuildInParallel="True"
             SkipNonexistentProjects="%(ProjectReference.SkipNonexistentProjects)"
             Projects="@(ProjectReference)"
             Targets="A;B;C"
             Properties="BuildingSolutionFile=true; CurrentSolutionConfigurationContents=$(CurrentSolutionConfigurationContents); SolutionDir=$(SolutionDir); SolutionExt=$(SolutionExt); SolutionFileName=$(SolutionFileName); SolutionName=$(SolutionName); SolutionPath=$(SolutionPath)" />
  </Target>

@rainersigwald, it is doing creating a new single target with the MSBuild which invokes all targets. The question is how would you expected the initial targets to invoke this new target?

rainersigwald

Sorry for the delay on this. You're right that the general approach matches what I was thinking--not sure what confused me there.

Can you elaborate on how metadata-SkipNonexistentProjects is tied to the solution-batching stuff?
Looking at the metaproject, I see

  <Target Name="foo" DependsOnTargets="SlnProjectResolveProjectReference" />
  <Target Name="bar" DependsOnTargets="SlnProjectResolveProjectReference" />
  <Target Name="SlnProjectResolveProjectReference">
    <MSBuild ToolsVersion="$(ProjectToolsVersion)" BuildInParallel="True" Projects="@(ProjectReference)" Targets="foo;bar" />
  </Target>

Replacing

  <Target Name="foo">
    <MSBuild BuildInParallel="True" Projects="@(ProjectReference)" Targets="foo" Properties="BuildingSolutionFile=true; CurrentSolutionConfigurationContents=$(CurrentSolutionConfigurationContents); SolutionDir=$(SolutionDir); SolutionExt=$(SolutionExt); SolutionFileName=$(SolutionFileName); SolutionName=$(SolutionName); SolutionPath=$(SolutionPath)" />
  </Target>
  <Target Name="bar">
    <MSBuild BuildInParallel="True" Projects="@(ProjectReference)" Targets="bar" Properties="BuildingSolutionFile=true; CurrentSolutionConfigurationContents=$(CurrentSolutionConfigurationContents); SolutionDir=$(SolutionDir); SolutionExt=$(SolutionExt); SolutionFileName=$(SolutionFileName); SolutionName=$(SolutionName); SolutionPath=$(SolutionPath)" />
  </Target>

Can you pass along the properties? Otherwise I think it'll go wrong in Build on non-default solution configurations.

rainersigwald · 2022-05-09T14:55:14Z

src/Tasks/MSBuild.cs


-                if (FileSystems.Default.FileExists(projectPath) || (_skipNonexistentProjects == SkipNonexistentProjectsBehavior.Build))
+                // Try to get the behavior from metadata if it is undefined.
+                var skipNonexistPropjects = _skipNonexistentProjects;


The story here is that long ago, the MSBuild task was just a task that used the IBuildEngine APIs like any other task would be able to do. At some later time, as a performance optimization, the MSBuild task was pulled into the engine itself, and task resolution short-circuits to prefer that version.

The one in Tasks exists only because it was a public non-sealed type, so user types might have inherited from it. I generally wouldn't worry about adding new features to it, since it should be almost entirely dead code.

…huang/main_solution

This reverts commit f563816.

yuehuang010 · 2022-05-09T22:26:09Z

Can you elaborate on how metadata-SkipNonexistentProjects is tied to the solution-batching stuff?

It is related as the SkipNonexistentProjects is splitting the build into two batches. This batch is serializing them which this will optimize away.

Looking at the metaproject,

This isn't actually needed as ProjectReference item already has those Properties as metadata.

yuehuang010 · 2022-05-24T00:08:11Z

@rainersigwald Ping.

rainersigwald

Sorry for the delay!

It is related as the SkipNonexistentProjects is splitting the build into two batches. This batch is serializing them which this will optimize away.

Ah, and this is happening inside solution builds because anything with a solution build dependency generates a metaproject that gets SkipNonexistent.

This isn't actually needed as ProjectReference item already has those Properties as metadata.

I'd really like to leave this in case a user injects a target that changes the properties, since it's been done that way for the normal build targets for a very long time.

src/Build/BackEnd/Components/RequestBuilder/IntrinsicTasks/MSBuild.cs

src/Build/Construction/Solution/SolutionProjectGenerator.cs

src/Build/BackEnd/Components/RequestBuilder/IntrinsicTasks/MSBuild.cs

yuehuang010 · 2022-07-18T19:22:25Z

@rainersigwald Ping.

Forgind

Tested this out, and it works!

Forgind · 2022-07-18T15:13:19Z

src/Build/Construction/Solution/SolutionProjectGenerator.cs

+            }
+            else
+            {
+                task.SetParameter("Properties", SolutionProperties);


This feels like taking something global and making it local to every project. Is that wrong? I would've imagined properties would flow automatically from the metaproj down to each individual project, but its absence here makes me wonder if we should continue to leave it empty.

@rainersigwald suggested to add it.

This isn't actually needed as ProjectReference item already has those Properties as metadata.

I'd really like to leave this in case a user injects a target that changes the properties, since it's been done that way for the normal build targets for a very long time.

yuehuang010 added 2 commits March 29, 2022 15:57

Add SolutionBatchTargets. Add metadata pickup to SkipNonexistingProje…

68e7509

…ct property in MSBuild task.

Merge branch 'main' of https://github.com/dotnet/msbuild into dev/yue…

430b926

…huang/main_solution

Forgind reviewed Apr 5, 2022

View reviewed changes

Move env to traits. Added a negative case for SkipNonexistentProject

c5c7a79

rainersigwald reviewed Apr 18, 2022

View reviewed changes

Merge branch 'main' of https://github.com/dotnet/msbuild into dev/yue…

72bbb81

…huang/main_solution

rainersigwald reviewed May 9, 2022

View reviewed changes

yuehuang010 added 3 commits May 9, 2022 14:03

Merge branch 'main' of https://github.com/dotnet/msbuild into dev/yue…

a461c1a

…huang/main_solution

Add solution properties to MSBuild task call.

f563816

Revert "Add solution properties to MSBuild task call."

9e72ede

This reverts commit f563816.

rainersigwald reviewed Jun 6, 2022

View reviewed changes

src/Build/BackEnd/Components/RequestBuilder/IntrinsicTasks/MSBuild.cs Outdated Show resolved Hide resolved

Address feedbacks

955cc15

rainersigwald approved these changes Jun 23, 2022

View reviewed changes

benvillalobos reviewed Jun 23, 2022

View reviewed changes

src/Build/Construction/Solution/SolutionProjectGenerator.cs Show resolved Hide resolved

Nirmal4G reviewed Jun 26, 2022

View reviewed changes

src/Build/BackEnd/Components/RequestBuilder/IntrinsicTasks/MSBuild.cs Outdated Show resolved Hide resolved

Renamed Nonexist to NonExist

704ffb5

Nirmal4G approved these changes Jun 29, 2022

View reviewed changes

Forgind approved these changes Jul 18, 2022

View reviewed changes

Forgind added the merge-when-branch-open PRs that are approved, except that there is a problem that means we are not merging stuff right now. label Jul 18, 2022

Forgind merged commit c849248 into dotnet:main Jul 18, 2022

yuehuang010 mentioned this pull request Jul 21, 2022

Slow design-time builds for large solution dotnet/sdk#27738

Closed

JanKrivanek mentioned this pull request Nov 28, 2023

Graph: Mimic quirk for sln-based builds with target name containing semicolon #9390

Closed

yuehuang010 deleted the dev/yuehuang/main_solution branch February 22, 2024 22:28

Conversation

yuehuang010 commented Apr 1, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Fixes

Context

Testing

Before:

After:

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Forgind Apr 5, 2022

Choose a reason for hiding this comment

Uh oh!

yuehuang010 Apr 5, 2022

Choose a reason for hiding this comment

Uh oh!

rainersigwald May 9, 2022

Choose a reason for hiding this comment

Uh oh!

rainersigwald left a comment

Choose a reason for hiding this comment

Uh oh!

yuehuang010 commented Apr 18, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

rainersigwald commented Apr 26, 2022

Uh oh!

yuehuang010 commented Apr 26, 2022

Uh oh!

rainersigwald left a comment

Choose a reason for hiding this comment

Uh oh!

rainersigwald May 9, 2022

Choose a reason for hiding this comment

Uh oh!

yuehuang010 commented May 9, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

yuehuang010 commented May 24, 2022

Uh oh!

rainersigwald left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

yuehuang010 commented Jul 18, 2022

Uh oh!

Forgind left a comment

Choose a reason for hiding this comment

Uh oh!

Forgind Jul 18, 2022

Choose a reason for hiding this comment

Uh oh!

yuehuang010 Jul 18, 2022

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

yuehuang010 commented Apr 1, 2022 •

edited

Loading

yuehuang010 commented Apr 18, 2022 •

edited

Loading

yuehuang010 commented May 9, 2022 •

edited

Loading