Warn when TargetFrameworks declares a single framework: it evaluates every such project twice
- Dominant language
- C#
- Stars
- 3.2k
- Forks
- 1.3k
- PR merge metrics
- PR metrics pending
Description
> **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 —
`net10.0` — 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 `net10.0` 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](https://github.com/OrchardCMS/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 `` 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`:
```xml
<_SingleTfmCandidate>$(TargetFrameworks.Trim().Trim('%3B').Trim())
<_TargetFrameworksHasMultipleEntries>$(_SingleTfmCandidate.Contains('%3B'))
```
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 `net10.0`
> instead, or set `$(NoWarn);NETSDKxxxx` 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.
Contributor guide
No contributing guide indexed for this repository
Assessment
This issue has not been assessed yet.