Skip to content

Warn when TargetFrameworks declares a single framework: it evaluates every such project twice #55699

Description

@ViktorHofer

Revised. The wall-clock figures first posted here were measured on a fixture that was not in an
incremental steady state, and overstated the cost by roughly 20x. Thanks to @rainersigwald for
catching it. The numbers below are the corrected ones; the evaluation-count figures are unchanged.

Summary

A project that declares a single target framework in the plural form —
<TargetFrameworks>net10.0</TargetFrameworks> — is treated as a cross-targeting build and is therefore
evaluated twice per build: once as the outer, framework-negotiating build (no TargetFramework
global property) and once as the inner build (TargetFramework=net10.0). Declaring the same single
framework as <TargetFramework>net10.0</TargetFramework> evaluates once.

The outer build is deliberately cheap, and measurement confirms it is: it imports 27% of the XML by
volume and costs about 20% of an inner evaluation. The residual cost is roughly 8–9 ms per project
per build
, which is small but permanent, invisible to the author, and almost never intentional.

This proposes a suppressible, low-severity diagnostic telling authors that a one-entry
TargetFrameworks is costing them an extra evaluation per project per build. It deliberately does
not propose collapsing the behaviour automatically.

The cost

Evaluation counts (from binary logs, unambiguous):

Fixture plural TargetFrameworks singular TargetFramework
1 library + 1 app 3 evaluations 2 evaluations
20 libraries + 1 app 40 evaluations 20 evaluations
OrchardCore, one module's graph 82 evaluations for 41 projects 41

Wall clock, 20 libraries plus an app, identical in every respect except the property name, every
measured build verified incremental (no Csc/CoreCompile in either binlog), alternating
plural/singular to control for machine drift, 8 pairs, MSBuild Server enabled, .NET 11 preview 7 SDK,
8 logical cores:

Library declares Evaluations / build Wall clock median min
TargetFrameworks (plural, one entry) 40 1 526 ms 1 390 ms
TargetFramework (singular) 20 1 351 ms 1 164 ms

175 ms on 20 projects, 1.13x, or ~8.75 ms per project per build.

Why it is that small — the outer build behaves as designed. Same project, with and without the
TargetFramework global property:

evaluation time imported files preprocessed size
outer 5 ms 60 411 KB
inner 25 ms 117 1 539 KB
singular, for comparison 22 ms

The outer build sees 51% of the files but only 27% of the XML by volume, does no item globbing, and
costs about a fifth of an inner evaluation. 5 + 25 against 22 gives a marginal cost of ~8 ms, which
agrees with the wall-clock figure derived independently.

One nuance worth recording, because it cuts against reading the outer build as pure overhead: it runs
first, so on a cold node it absorbs the cold SDK parse and leaves the cache warm for the inner
build. In the OrchardCore binlog several projects show the outer evaluation as the more expensive of
the pair (445 ms outer versus 338 ms inner) purely because of that ordering. Per-evaluation timings
from a parallel binlog are therefore not a sound basis for marginal cost; the controlled fixture is.

At scale: for OrchardCore's 41 projects, ~8 ms per project is on the order of 350 ms per
incremental build. Against a build doing real compilation that is noise. Against a no-op incremental
build of a large repository it is a visible fraction, and it is paid on every build forever.

The shape is extremely common, and for a good reason: repositories set TargetFrameworks from a
shared property so that adding a second framework later is a one-line change. OrchardCore sets
CommonTargetFrameworks to net10.0 and assigns it to <TargetFrameworks> in three
Directory.Build.props files. They then pay for cross-targeting machinery they never use, and there
is currently nothing telling them so.

Why not just fix it silently

Collapsing automatically is the obvious move and I believe it is the wrong one, because a large amount
of code — in this repo, in MSBuild's common targets, and in third-party build logic — detects "this is
the outer build" by testing the TargetFramework / TargetFrameworks properties directly rather than
by consulting IsCrossTargetingBuild. Setting TargetFramework for a one-entry list changes the
answer those conditions give, while $(TargetFrameworks) remains non-empty, so a project would present
as both cross-targeting and single-targeting depending on which idiom a given target happens to use.

Scanning the 256 .props/.targets files in an installed SDK layout (which includes MSBuild's common
targets) gives a sense of the exposure before counting any third-party build logic:

Idiom Occurrences Files
'$(TargetFrameworks)' != '' tested directly 27 14
'$(TargetFramework)' == '' tested directly 8 7
IsCrossTargetingBuild (the intended abstraction) 30 12

So roughly half of the in-box detection sites bypass the abstraction and test the raw properties. Each
one is a place where a silent collapse could change behaviour, and the ecosystem outside these two
repositories is unbounded.

Other things that would change, and that a silent collapse would have to guarantee:

  • Targets that only run in the outer build (DispatchToInnerBuilds, GetTargetFrameworks) stop
    running, so anything hooking them via BeforeTargets/AfterTargets silently stops running too.
  • Packing a single-TFM cross-targeting project goes down a different path.
  • Anything conditioned on '$(IsCrossTargetingBuild)' == 'true' in customer build logic.

For what it is worth, a local patch that sets TargetFramework from a one-entry TargetFrameworks
before IsCrossTargetingBuild is computed did take the unmodified 20-project plural fixture to 20
evaluations, and the project-reference protocol (_GetProjectReferenceTargetFrameworkProperties)
negotiated correctly. But a trivial fixture proves very little here, and the failure mode for the
ecosystem is silent wrong behaviour rather than a build break.

Proposal: tell the author instead

Emit a diagnostic when TargetFrameworks resolves to exactly one entry and TargetFramework is empty,
pointing at the singular property. This keeps the SDK's behaviour unchanged, puts the decision with the
person who owns the project, and makes an invisible cost visible.

Sketch, in the branch that already computes IsCrossTargetingBuild:

<!-- ';' cannot appear literally in a Condition (MSB4090), so the list test is done in a
     property value using the %3B escape and the Condition compares the resulting boolean. -->
<PropertyGroup Condition="'$(TargetFrameworks)' != '' and '$(TargetFramework)' == ''">
  <_SingleTfmCandidate>$(TargetFrameworks.Trim().Trim('%3B').Trim())</_SingleTfmCandidate>
  <_TargetFrameworksHasMultipleEntries>$(_SingleTfmCandidate.Contains('%3B'))</_TargetFrameworksHasMultipleEntries>
</PropertyGroup>

and a diagnostic raised from a target in the outer build when
_TargetFrameworksHasMultipleEntries == 'false', with text along the lines of:

NETSDKxxxx: Project declares a single target framework using TargetFrameworks, which builds it as
a cross-targeting project and evaluates it twice. Use <TargetFramework>net10.0</TargetFramework>
instead, or set <NoWarn>$(NoWarn);NETSDKxxxx</NoWarn> to keep the current behaviour.

Design points worth settling:

  1. Severity should be low. At ~8 ms per project this does not justify a warning that would fire
    across a large part of the ecosystem at once. A new warning is itself a breaking change for
    TreatWarningsAsErrors / WarnAsError builds, and on these numbers that cost plausibly outweighs
    the benefit. A message, or an analyzer-style suggestion that is off unless asked for, fits the
    measured impact better than the warning originally proposed here.
  2. Suppression must be first-class. A dedicated NETSDKxxxx code so NoWarn works per project and
    repo-wide is the minimum. Repos that deliberately use the plural form — because they add and remove
    frameworks often, or because their own build logic depends on the outer build existing — need a
    one-line opt-out that does not involve suppressing anything else.
  3. Fire once per project, from the outer build, so a genuine cross-targeting build with several
    frameworks never sees it and the message is not repeated per inner build.
  4. Say what it costs. "Evaluates this project twice per build" is more actionable than "prefer the
    singular property", and it is the entire reason the diagnostic exists.

Method

Numbers were produced with MSBuild's Microsoft-Build event source markers, binary logs (evaluation
counts and per-evaluation global properties), and -pp for import closures, on dotnet/msbuild at
f08c806268 (18.11.0) with the .NET 11.0.100-preview.7 SDK, Windows, 8 logical cores, Release.

Two measurement cautions for anyone reproducing this, both learned by getting them wrong first:

  • Verify the fixture is genuinely incremental before timing it. My original numbers here came from
    a tree built once and then timed with a different command, so some runs were doing real compilation.
    The raw spread (6 368–26 560 ms) showed it. The corrected runs build twice, warm three times, and are
    confirmed against binlogs to contain no Csc.
  • -profileevaluation reports evaluation summed over all projects and all nodes, so on a parallel
    build it routinely exceeds wall clock — it measures work, not elapsed time.

Metadata

Metadata

Assignees

Projects

No projects

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions