Skip to content

[MicroPerf] Avoid redundant FreeVars record allocation for local vals - #20354

Open
T-Gro wants to merge 2 commits into
dotnet:mainfrom
T-Gro:t-gro-freevar-perf-spike
Open

[MicroPerf] Avoid redundant FreeVars record allocation for local vals#20354
T-Gro wants to merge 2 commits into
dotnet:mainfrom
T-Gro:t-gro-freevar-perf-spike

Conversation

@T-Gro

@T-Gro T-Gro commented Aug 25, 2026

Copy link
Copy Markdown
Member

Folds the two FreeVars allocations in accFreeLocalVal/boundLocalVal — free-type-variable accumulation and the FreeLocals update — into one record. Safe because the type walk (accFreeInVal) writes only FreeTyvars, never FreeLocals, so the updates are independent.

Self-compile of src/Compiler, 65,880 LOC / 120 files, net11.0 Release, --optimize+:

FreeVars allocation, 1 compile before after Δ
record-type total (gc-verbose trace) 207.3 MB 196.1 MB −11.2 MB (−5.4%)
redundant intermediate records 177,250 0 −177,250

Compiler-output identity: assemblies emitted by the patched compiler are byte-for-byte identical to those from unmodified main — each of the 120 files compiled --deterministic+ to a fixed output path, SHA-256 equal on both. Corroborated by an in-process check comparing the old and new FreeVars at every call across one compile: 3,727,347 calls, 0 differences.

@T-Gro
T-Gro requested a review from a team as a code owner August 25, 2026 15:45
@T-Gro T-Gro added the NO_RELEASE_NOTES Label for pull requests which signals, that user opted-out of providing release notes label Aug 25, 2026
@github-actions

github-actions Bot commented Aug 25, 2026

Copy link
Copy Markdown
Contributor

⚠️ Release notes required, but author opted out

Warning

Author opted out of release notes, check is disabled for this pull request.
cc @dotnet/fsharp-team-msft

@github-actions github-actions Bot added the AI-Tooling-Check-Scanned-Clean Tooling check: diff analyzed, no interesting infrastructure files label Aug 25, 2026
@T-Gro
T-Gro marked this pull request as draft August 26, 2026 08:01
accFreeLocalVal and boundLocalVal accumulated the free type variables of
the val's type and then separately added/removed the val from FreeLocals,
allocating two FreeVars records whenever the type contributed a free type
variable. Fold both into a single record update, factoring the shared
type-variable accumulation into `accFreeTyvarsInVal` (used by both) and
dropping the now-unused accFreevarsInVal helper.

Output is byte-for-byte identical: accFreeInVal only accumulates into
FreeTyvars, never FreeLocals, so computing the type contribution before
the FreeLocals membership check is order-independent.

Measured on a 65,880-LOC / 120-file compile (net11.0, Release):
177,250 redundant FreeVars records eliminated per compile (~11 MB);
FreeVars record allocation down 5.4% in a gc-verbose trace. Byte identity
confirmed by comparing per-file deterministic (--deterministic+) output
hashes across all 120 files (manifest hash unchanged vs HEAD).

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
@T-Gro
T-Gro force-pushed the t-gro-freevar-perf-spike branch from a07eac8 to 96d41b1 Compare August 26, 2026 08:20
@T-Gro
T-Gro marked this pull request as ready for review August 26, 2026 09:12
@T-Gro
T-Gro requested a review from abonie August 26, 2026 09:12
@T-Gro
T-Gro enabled auto-merge (squash) August 26, 2026 09:12
@T-Gro T-Gro changed the title Avoid redundant FreeVars record allocation for local vals [MicroPerf] Avoid redundant FreeVars record allocation for local vals Aug 27, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

AI-Tooling-Check-Scanned-Clean Tooling check: diff analyzed, no interesting infrastructure files NO_RELEASE_NOTES Label for pull requests which signals, that user opted-out of providing release notes Theme-Performance

Projects

Status: New

Development

Successfully merging this pull request may close these issues.

1 participant