WIP: DO NOT REVIEW — ExpressionShredder weak-interning experiment (negative result, do not merge)

[artl93]

I am running an experiment - full analysis to follow. Contact @artl93

---

Status: DRAFT — WIP — DO NOT REVIEW. This PR is an experiment container and will remain a draft permanently. Please do not review or merge.

TL;DR — decisive result: no build-scale benefit; recommend withdrawal

The change works exactly as designed (it deduplicates repeated capture strings — see the new reference-identity test), but at realistic evaluation/build scale it produces no measurable allocation or time improvement, and it adds a small permanent retained-memory cost. Evidence below is now end-to-end and BenchmarkDotNet-based, superseding the earlier isolated-probe numbers.

What this changes

src/Build/Evaluation/ExpressionShredder.cs — four edits, each replacing expression.Substring(start, len) with the existing span-based overload Strings.WeakIntern(expression.AsSpan(start, len)), reusing the identical (start, length) indices:

• quoted item-transform text (capture Value),
• item-vector separator,
• transform FunctionName,
• function subexpression text (capture Value).

The method already weak-interned item names and top-level captures via the same overload; this extended that pattern to the four remaining Substring sites. Nullable/absent function and separator branches are untouched. Product diff: 4 insertions(+), 4 deletions(-).

Evidence (A = exact parent 37d66e704, B = this change)

A/B performed by swapping only Microsoft.Build.dll (the only assembly that differs); DLL sha256 verified before and after every run. Env: macOS 26.5.2, Apple M5 Pro arm64, .NET SDK 10.0.300 / runtime 10.0.8, Workstation GC, BenchmarkDotNet v0.13.12. A f3814cad…, B cceebe85….

1. End-to-end evaluation scale (representative) — the headline

MSBuildLocator-hosted harness evaluating a large generated project 30×/process (50 source items + 2000 item-vector transforms with separators + 200 properties). cardinality = number of distinct transform texts.

distinct transforms	A alloc/eval	B alloc/eval	Δ alloc	A retained	B retained	Δ retained
1 (all identical)	6,175,272	6,175,272	0 (identical to the byte)	1,041,232	1,041,776	+544 B
8	6,176,544	6,176,544	0	1,047,392	1,049,392	+2,000 B
2000 (all unique)	6,442,024	6,442,024	0	2,623,160	3,039,448	+416,288 B

Allocation per evaluation is identical to the byte in every case; wall-time is within noise. The transform/separator substrings are a negligible fraction of the ~6 MB allocated per evaluation, so interning them changes nothing measurable at scale. The only measurable effect is increased retained memory that grows with the number of distinct transform texts.

2. BenchmarkDotNet micro (in-tree ExpanderBenchmark, [MemoryDiagnoser], ShortRun)

ItemList_WithTransform / ItemList_WithSeparator: Allocated identical A vs B in every stable case (1.16/9.43 KB transform, 1.86/9.95 KB separator); times noise-dominated. (The Expander caches the shred result within an operation, so the substrings aren't re-allocated in the measured loop.)

The earlier "cold isolated probe" numbers (−31% to −99% on repeated cases) measured substring allocation in isolation, which the E2E result shows is irrelevant at build scale.

Why no benefit + a real cost (post-review, corrected)

• No global shred cache, but tiny absolute cost: GetReferencedItemExpressions runs fresh per Expander call, yet the captured substrings are a negligible share of evaluation allocation, so removing them is unmeasurable at scale.
• Retention is a strong reference, not weak: in WeakStringCache, strings ≤ 500 chars (all transforms/separators/function names) are stored in a ConcurrentDictionary<int,string> as a strong entry (one per hashcode, overwritten on collision). They are not weakly held or scavenged — they persist until Strings.ClearCachedStrings() / build-engine shutdown. In evaluation-only, long-lived hosts this is effectively process-lifetime retention. Bounded (per distinct hashcode, not per occurrence) — no unbounded leak — but a real, permanent cost with no offsetting benefit.
• CPU/lock: each call adds an FNV hash + ConcurrentDictionary lookup/compare; misses still allocate the string (like Substring) plus bookkeeping.

Recommendation

Withdraw (kept here as a documented negative result). The change is correct and behavior-preserving, but delivers no measurable build-scale allocation or time win while adding permanent retained memory for varied/unique transforms. A skeptical senior memory/lifetime review reached the same conclusion.

Correctness / compatibility (verified)

WeakIntern(ReadOnlySpan<char>) returns text byte-identical to Substring (ordinal SequenceCompareTo; ExpensiveConvertToString = span.ToString(); hits content-verified, collisions recreate). All downstream consumers of Value/Separator/FunctionName are ordinal (no ReferenceEquals/identity-keyed maps). Null/empty branches untouched. Short-string path is lock-free and already exercised by the pre-existing interning in the same method.

Tests

• dotnet test src/Build.UnitTests/Microsoft.Build.Engine.UnitTests.csproj -c Release --filter-method "*ExpressionShredder*" → 52/52 pass.
• New WeakInternedCaptures_ReturnSharedInstanceAcrossDistinctExpressions asserts that identical transform/separator/function-name text in two distinct backing strings returns the same interned instance (Assert.Same) — documenting the dedup behavior and proving the interning path is exercised (the pre-change Substring would fail this). This is the falsification check that validates the A/B evidence.

Provenance / artifacts

A/B DLL hashes, E2E harness + raw CSV, BenchmarkDotNet reports, and the skeptical review are retained as experiment artifacts (hashes recorded). CI on the product+test commit is green on Linux/macOS/Source-Build; Windows legs validated after removing the net472-incompatible probe harness from the branch.

[artl93]

WITHDRAWN — closing this draft (NO-GO). Not for review or merge.

Final decision after full A/B validation. I re-ran this as an end-to-end MSBuild evaluation A/B plus BenchmarkDotNet, swapping only Microsoft.Build.dll (sha256 verified before/after) against the exact parent commit 37d66e704.

Decisive result: no build-scale benefit.

• Allocation per evaluation is identical to the byte A vs B across all transform cardinalities (1 / 8 / 2000 distinct); time within noise.
• The transform/separator/function-name substrings are a negligible fraction of the ~6 MB allocated per evaluation, so interning them changes nothing measurable at scale.
• The only measurable effect is a permanent retained-memory increase (+416 KB at 2000 distinct transforms). Capture strings ≤500 chars are held by a strong reference in WeakStringCache (_stringsByHashCode) until engine shutdown — not weakly scavenged.

A skeptical memory/lifetime review concurred and corrected two earlier claims (short strings are strongly retained; there is no global Expander shred cache). No plausible no-regression redesign exists — even the isolated −99.5% separator win is eval-scale-negligible.

The change is correct and behavior-preserving (verified; new WeakInternedCaptures_ReturnSharedInstanceAcrossDistinctExpressions test proves the interning path and dedup), but it delivers no measurable win and adds permanent retention for varied/unique transforms. Closing as a documented negative result. The branch and evidence remain available on my fork for reference.

[artl93]

Closing this WIP experiment draft as WITHDRAWN / NO-GO — decisive A/B evidence shows no build-scale allocation or time benefit and a small permanent retained-memory cost. Documented negative result; see final analysis comment above. Not for review or merge.