Parallel TargetFrameworks builds are disabled when using the extras SDK

[jeromelaban]

See this repro: https://github.com/jeromelaban/parallel-msbuild-extras-issue.

The processing of DispatchToInnerBuilds is not done in parallel when using the msbuild sdk extras, but only when using the sdk attribute syntax.

When Sdk="Microsoft.NET.Sdk" is used, multiple msbuild nodes are used:

1>Running in 58180/MSBuild
1>Running in 84868/MSBuild
1>Running in 20368/devenv
1>NoExtrasTest -> ExtrasParallelIssue\NoExtrasTest\bin\Debug\net47\NoExtrasTest.dll
1>NoExtrasTest -> ExtrasParallelIssue\NoExtrasTest\bin\Debug\net46\NoExtrasTest.dll
1>NoExtrasTest -> ExtrasParallelIssue\NoExtrasTest\bin\Debug\netstandard2.0\NoExtrasTest.dll

When Sdk="MSBuild.Sdk.Extras/1.6.65" is used:

1>Running in 58180/MSBuild
1>ExtrasWithSDKTest -> ExtrasParallelIssue\ExtrasWithSDKTest\bin\Debug\netstandard2.0\ExtrasWithSDKTest.dll
1>Running in 58180/MSBuild
1>ExtrasWithSDKTest -> ExtrasParallelIssue\ExtrasWithSDKTest\bin\Debug\net46\ExtrasWithSDKTest.dll
1>Running in 58180/MSBuild
1>ExtrasWithSDKTest -> ExtrasParallelIssue\ExtrasWithSDKTest\bin\Debug\net47\ExtrasWithSDKTest.dll

but when the extras are used with the old PackageReference way:

1>Running in 20368/devenv
1>Running in 84868/MSBuild
1>Running in 58180/MSBuild
1>ExtrasWithPackageRefTest -> ExtrasParallelIssue\ExtrasWithPackageRefTest\bin\Debug\net47\ExtrasWithPackageRefTest.dll
1>ExtrasWithPackageRefTest -> ExtrasParallelIssue\ExtrasWithPackageRefTest\bin\Debug\net46\ExtrasWithPackageRefTest.dll
1>ExtrasWithPackageRefTest -> ExtrasParallelIssue\ExtrasWithPackageRefTest\bin\Debug\netstandard2.0\ExtrasWithPackageRefTest.dll

[clairernovotny]

@davkean @nguerrera I could use some help understanding what's going on here.

The only place I should be affecting inner loop things is here, where I return additional inner builds:

https://github.com/onovotny/MSBuildSdkExtras/blob/8cfb9cd75086419bae47e4cd736ea76cc4fe303b/Source/MSBuild.Sdk.Extras/Build/RIDs.targets#L23-L41

[nguerrera]

I'm not seeing it yet. This looks normal:

And yet, I can repro.

@rainersigwald Ideas?

[nguerrera]

This will cause the MSBuild invocation to batch and serialize:

https://github.com/onovotny/MSBuildSdkExtras/blob/8cfb9cd75086419bae47e4cd736ea76cc4fe303b/Source/MSBuild.Sdk.Extras/Build/RIDs.targets#L30

But that's not the invocation that calls Build, triggering BeforeBuild and the logging of process ID, the screenshot above is.

[clairernovotny]

@nguerrera even for that one, I pass in BuildInParallel, should that work there too? Is there a way to parallelize that call (even though, it's just gathering info, not doing the actual build)

[nguerrera]

It doesn't matter that you pass BuildInParallel there, because with batching, you are passing it 3 separate times.

Update: Commenting out that target fixes the problem. @rainersigwald Does this initial building of another target for (project + global properties) affinitize that combo to a node?

[nguerrera]

This reworking of that target works:

  <Target Name="_ComputeTargetFrameworkItems" Returns="@(InnerOutput)">
    <ItemGroup>
      <_TargetFramework Include="$(TargetFrameworks)" />
    </ItemGroup>

    <ItemGroup>
      <_InnerBuildProjects Include="$(MSBuildProjectFile)">
        <AdditionalProperties>TargetFramework=%(_TargetFramework.Identity)</AdditionalProperties>
      </_InnerBuildProjects>
    </ItemGroup>

    <MSBuild Projects="@(_InnerBuildProjects)"
             BuildInParallel="$(BuildInParallel)"
             Targets="_SdkGetRidsPerTargetFramework">
      <Output ItemName="_SdkTargetsWithRids" TaskParameter="TargetOutputs"  />
    </MSBuild>

    <ItemGroup>
      <_InnerBuildProjects Remove="@(_InnerBuildProjects)" />
      <_InnerBuildProjects Include="$(MSBuildProjectFile)">
        <AdditionalProperties Condition="'%(_SdkTargetsWithRids.Rid)' != ''" >TargetFramework=%(_SdkTargetsWithRids.Identity);RuntimeIdentifier=%(_SdkTargetsWithRids.Rid)</AdditionalProperties>
        <AdditionalProperties Condition="'%(_SdkTargetsWithRids.Rid)' == ''" >TargetFramework=%(_SdkTargetsWithRids.Identity)</AdditionalProperties>
      </_InnerBuildProjects>
    </ItemGroup>
  </Target>

[nguerrera]

(By the way, the overriding of this _Target producing _Items may break in the future.)

[clairernovotny]

Thanks! I understand that the _ targets may break; I don't really have any other option for this feature.

Why does that workaround work?

[nguerrera]

Why does that workaround work?

To MSBuild in parallel, you must pass all the projects in one invocation. As soon as you use %, you are generating multiple invocations that happen serially.

[nguerrera]

The interesting part is the serialization of _SdkGetRidsPerTargetFramework causes later serialization of Build. I just learned this and asked @rainersigwald to confirm what I'm seeing. This node affinity makes sense if that's what's happening, I just never thought about it before. :)

[clairernovotny]

Good to know, never use % to batch within an MSBuild task. Seems like a good tip for some future MSBuild best-practices wiki ;)

[rainersigwald]

Yes, node affinity is to blame here. Once a project instance (path to project + passed-in and inherited global properties) is assigned a node, it can only ever build on that node.

With this reduced repro, the problem is exacerbated: the entry-point (outer) project starts building, then yields to a series of inner builds one at a time. Since only one project is eligible for build at that time (outer, then inner 1, then inner 2, and so on), the engine never needs to spin up another worker node--it can do them all in one. Then, when the parallel inner builds invoke, they're all locked to the (one) node.

Good to know, never use % to batch within an MSBuild task.

That's a pretty good rule, but there are some cases where it makes (some) sense to batch over MSBuild tasks. Perhaps those are all limitations of the task itself that we could remove, though.

Seems like a good tip for some future MSBuild best-practices wiki

I'd go further and hope it could be an opt-in warning. Added https://github.com/Microsoft/msbuild/issues/1330 to the tag we're using to track that sort of thing.