Skip to content

Add an experimental flag-gated in-process compile path for hot reload sessions#20031

Open
NatElkins wants to merge 58 commits into
dotnet:mainfrom
NatElkins:hotreload-inprocess-compile
Open

Add an experimental flag-gated in-process compile path for hot reload sessions#20031
NatElkins wants to merge 58 commits into
dotnet:mainfrom
NatElkins:hotreload-inprocess-compile

Conversation

@NatElkins

@NatElkins NatElkins commented Jul 3, 2026

Copy link
Copy Markdown
Contributor

Adds the flag-gated in-process compile performance path for hot reload sessions. This is the last slice of the train and it is explicitly experimental: everything here is behind the FSHARP_HOTRELOAD_INPROCESS_COMPILE environment flag (plus the nested FSHARP_HOTRELOAD_INCREMENTAL_EMIT), and with the flags unset the session behaves exactly as in #20030 (one env read, otherwise zero cost).

What the flag does: instead of waiting for an external dotnet build per edit, the session compiles the edited project in process from the already-checked FCS project (CompileFromCheckedProject), serializes the assembly once in memory, parses the emitted bytes back, and feeds the delta emitter from those in-memory artifacts. The nested incremental flag additionally caches per-file optimized trees (keyed by CheckedImplFile reference identity) so only the edited file re-optimizes. Reference-equality gates scope the per-edit diff and request-side walks to changed files, failing open to the full walk whenever anything looks unusual (duplicate keys, added/removed/renamed files, missing baseline). Trace plumbing (FSHARP_HOTRELOAD_TRACE_TIMING, FSHARP_HOTRELOAD_DUMP_DELTAS, FSHARP_HOTRELOAD_TRACE_CLOSURENAMES) is included for diagnosing this path and is zero cost when off.

Why it exists: with the external build flow a hot reload edit can never be faster than the build it waits on. With this path, measured on the integration branch with a Release FCS and real dotnet watch: a 72-file app went from ~1.9-2.6s per plain build to ~1.6-2.1s per applied edit at 1s polling granularity (median ~0.5s once the polling tick was tightened to 200ms), and on Giraffe's EndpointRouting sample (real multi-project code) body edits applied in ~0.2s and a line-adding edit above the endpoints list in ~0.8s, all in place, no restarts. Caveats: small sample sizes, a light benchmark app, and the Release-synced fsc plain build measuring slower than the Debug one for reasons not yet pinned down (likely missing crossgen). The numbers support "faster than the build it replaces", not more than that.

Fail-closed framing is unchanged: this path only produces deltas the same emitter would produce; anything it cannot prove safe still surfaces as a rude edit and falls back to the external flow. A writer-side module is never handed to the symbol matcher; the emitted bytes are always parsed back first (a writer-side ILModuleDef is not matcher-equivalent to a read one).

Per earlier maintainer guidance (T-Gro), building from compiler caches cannot be a general productized feature; this slice keeps it hot-reload-scoped, internal, and flag-gated, and this PR is where the productization conversation (determinism scrutiny: flags, invocation order, state flow parity) should happen.

Tests: 353 passed, 0 failed service HotReload (includes the flag ON/OFF contract test), 241 passed, 0 failed component HotReload (2 expected skips), incremental-emit equivalence pinned as a regression test plus a cross-generation splice test, determinism guards 22 passed (this slice touches naming state install), SurfaceAreaTest green with no baseline change (everything new is internal).

Stacked PR: based on #20030 (which stacks on #20027, #20026, #20025, #20024, #20019, #20018, #20017). The only commit that belongs to this PR, viewable as a single diff:

Everything else in the commit list arrives from the dependency PRs via merges. The same slice rendered against its base branch: compare hotreload-session...hotreload-inprocess-compile.

Sequencing

This PR is part of splitting the F# hot reload work (#19941) into small, independently reviewable PRs. The planned order:

  1. Wave 1 (independent): Add ResetCompilerGeneratedNameState to compiler-generated name generators #20017, Add Roslyn-format EnC CustomDebugInformation codec and portable PDB method CDI emission #20018, Add ECMA-335 EnC metadata delta writer #20019, Add stable synthesized-name replay infrastructure for hot reload #20024, Add typed-tree differ and edit classification for hot reload #20025.
  2. Wave 2: Add hot reload baseline reading and recorded EnC state #20026 (baseline reading and recorded EnC state).
  3. Wave 3: Add the hot reload delta emitter and symbol matcher #20027 (the delta emitter and symbol matcher).
  4. Wave 4: Add the F# hot reload session and FCS service surface #20030 (the hot reload session, FCS surface, and the --test:HotReloadDeltas capture hook).
  5. Wave 5 (this PR, last, explicitly experimental): the flag-gated in-process compile perf path.

The dotnet-watch integration consuming this API is dotnet/sdk#55128.

Refresh status (2026-07-17)

  • Refreshed against dotnet/fsharp main at 5928e91; the current reviewed head is 7e8cf28.
  • A dedicated review pass completed for this PR, its findings were fixed in the lowest owning slice, and the PR has no unresolved review threads.
  • The downstream session, in-process compiler, umbrella, and SDK branches were restacked after the fixes, so this slice remains part of the decomposed review train.
  • The complete compiler stack passed the 11-step hot reload verifier, 456 service tests, 243 component tests with 2 expected skips, and 1411 EmittedIL tests with 3 expected skips. Replacement CI passed on this exact head.

Refresh status (2026-07-22)

  • Current head: 54eba2f, refreshed against dotnet/fsharp main 69fca7f.
  • Stack position: final experimental in-process slice on Add the F# hot reload session and FCS service surface #20030; also included in umbrella F# hot reload: Edit-and-Continue delta emission behind --test:HotReloadDeltas #19941.
  • Both July 21 findings are fixed and resolved. Incremental optimization now threads real state through a safe reusable prefix, and cancellation/fatal exceptions propagate. Recorded-slot synthesized alias recovery remains limited to complete whole-module in-process artifacts after injective filtering.
  • Local verification: full Debug build passed with 0 warnings/errors; service HotReload passed 365/365; component HotReload passed 244 with 2 documented manual-host skips; the safe-prefix test reused 11 of 12 files.
  • Exact-head Azure compiler CI completed with 45/46 jobs passing. The sole failure was an external IcedTasks restore-sentinel file-lock race before compiler invocation; a privileged rerun has been requested. The base-branch check_release_notes workflow still fails before analysis because pull_request_target refuses fork checkout.

@github-actions

github-actions Bot commented Jul 3, 2026

Copy link
Copy Markdown
Contributor

❗ Release notes required

You can open this PR in browser to add release notes: open in github.dev


✅ Found changes and release notes in following paths:

Warning

No PR link found in some release notes, please consider adding it.

Change path Release notes path Description
src/Compiler docs/release-notes/.FSharp.Compiler.Service/11.0.100.md No current pull request URL (#20031) found, please consider adding it

@NatElkins

Copy link
Copy Markdown
Contributor Author

Oxpecker performance follow-up (2026-07-09):

I reproduced Lanayx's examples/ContactApp edit against the preview SDK. On the default external-compile path, representative edits took 4.45s and 4.04s, while FCS tracing accounted for only 0.79s and 0.59s. The external build was the dominant cost.

Enabling the in-process path exposed a correctness bug in whole-module emit. Regenerated computation-expression helpers from unchanged files could be offered a baseline alias that was already paired with a different fresh TypeDef. That produced a duplicate baseline-token mapping and forced the session to fail closed.

Commit 3356e7040f makes that fresh-to-baseline relation injective. The filtering is deliberately limited to whole-module in-process artifacts backed by a recorded synthesized-name snapshot. External compilation and legacy reconstructed snapshots keep their existing conservative behavior.

After the fix, the first ContactApp edit took 1.18s and warm edits took 529ms, 505ms, and 507ms, without generation-over-generation growth.

Local verification:

  • Full FSharp.slnx Debug build: 0 warnings, 0 errors
  • HotReload service filter: 435 passed
  • HotReload component filter: 236 passed, 2 expected manual-host skips
  • New unit regression for alias injectivity
  • New end-to-end multi-file regression with unrelated computation-expression helpers

The C# and Razor changes applied console text is a generic managed-update label. Oxpecker's cited files are F# ViewEngine computation expressions, not .razor or .cshtml files. I also ran a literal Razor Pages control test by editing Pages/Index.cshtml; it applied in 658ms first and 41ms warm.

NatElkins added 13 commits July 17, 2026 12:27
…ethod CDI emission

Adds an internal AbstractIL module implementing, byte for byte, the three Portable PDB
CustomDebugInformation blob formats Roslyn persists per method for Edit and Continue
(EnC Local Slot Map, EnC Lambda and Closure Map, EnC State Machine State Map), with
serializers, deserializers, a portable PDB read-back helper, and an occurrence-key
packing helper for deterministic syntax-offset slots.

Plumbs an optional methodCustomDebugInfoRows side channel through the IL binary writer
options into the portable PDB generator so a compilation can attach CDI rows to named
methods. Names that do not identify exactly one method row are dropped. All existing
writer call sites pass an empty map, so emitted PDBs are byte-identical to before.

No in-tree caller populates the map yet; the consumer is the F# hot reload work in
dotnet#19941, following the same pattern as dotnet#20017 (land isolated, test-covered
infrastructure first, wire the feature later).

Tests: blob round-trips, Roslyn golden-byte encodings, cross-validation against
CDI blobs emitted by a real Roslyn compilation, fail-closed occurrence-key packing
(including an int32-overflow regression where a wrapped negative key previously
escaped the bound check), and end-to-end synthetic PDB emission proving correct
MethodDef parenting, zero rows for an empty map, and no rows for absent or
ambiguous names.
ProcessStartInfo.ArgumentList does not exist on net472, which the component tests
also target on Windows CI. Build the quoted argument string by hand instead.
xunit 3.2.2 no longer discovers internal test classes, so the module was
silently skipped after rebasing onto current main (pre-existing internal test
modules like CompilerService.Caches are likewise undiscovered there). Public
visibility restores discovery; 17 tests run and pass.
Add internal generated-name normalization and synthesized-name map replay support as a standalone slice. The new map state is side-channel based, all new compiler modules remain internal, and CompilerGlobalState preserves the existing no-map counter path while checking an accessor captured once per compiler state.

Route existing IlxGen generated-name allocations through inert helper wrappers, add pure name-map and normalizer tests, add a normal compilation determinism guard over emitted generated names, and document the extracted seams in P5_REPORT.md.

Verification: built FSharp.Compiler.Service, FSharp.Compiler.Service.Tests, FSharp.Compiler.ComponentTests, and FSharpSuite.Tests in Release; ran the migrated service test classes, the component determinism class, FSharpSuite DeterministicTests, and the FCS SurfaceArea class successfully.
Adds an internal, standalone ECMA-335 Edit-and-Continue metadata delta writer to
AbstractIL: delta #- table stream and heap construction (DeltaMetadataTables,
DeltaMetadataSerializer, DeltaTableLayout, DeltaIndexSizing), ECMA-335 II.24.2.6
coded-index encoding (DeltaMetadataEncoding), EncLog/EncMap emission, generation GUID
chaining, user-string and standalone-signature token calculators (IlxDeltaStreams),
and the coordinating writer (FSharpDeltaMetadataWriter) over a plain row-description
input model (DeltaMetadataTypes, ILDeltaHandles, ILMetadataHeaps).

The writer's inputs are row records (names, tokens, signatures, RVAs) plus heap
offsets; it has no dependency on any semantic diffing or session machinery. It
compiles with no in-tree consumer by design: the consumer is the F# hot reload work
in dotnet#19941, following the same upstreaming pattern as dotnet#20017 and dotnet#20018
(land isolated, test-covered infrastructure first, wire the feature in a later PR).

One line of ilwrite.fsi is touched to expose the pre-existing markerForUnicodeBytes
so the delta writer reuses the exact string-marker logic of the full writer. No
behavior change for any existing code path.

Tests (130): coded-index encodings asserted against the production definitions and
ECMA-335 II.24.2.6 order, System.Reflection.Metadata reader parity over emitted
deltas, EncLog/EncMap correctness, stream layout, heap and index sizing,
multi-generation heap-offset chaining asserted against computed expected values,
standalone-signature rows asserted at baseline+1 from a real seeded baseline, and
serializer failure paths.
…CI images

The Roslyn cross-validation test shells out to dotnet build to produce a
real Roslyn PDB. The process launch had two problems. It computed the
host path by hand from __SOURCE_DIRECTORY__, which misses on CI images
that carry no repo-local .dotnet at that depth, failing with
Win32Exception before the build starts. It also left UseShellExecute at
its default, which is true on net472 and rejects redirected streams, so
every Desktop test leg failed deterministically with
InvalidOperationException.

Resolve the host like the rest of the test framework via
TestFramework.initialConfig.DotNetExe, which prefers the repo-local
.dotnet and falls back to PATH, and set UseShellExecute to false
explicitly.

Verified: FSharp.Compiler.ComponentTests builds clean;
EncMethodDebugInformationTests 17 passed, 0 failed (net10.0); fantomas
clean on the touched file.
Add an internal TypedTreeDiff module that snapshots CheckedImplFile bindings and entities, then classifies body, signature, inline, declaration add/remove, and type layout changes without depending on hot reload sessions, runtime capability negotiation, EnC capability names, baseline state, or delta emission.

Add focused FCS tests for unchanged/reference-equal files, body edits, signature edits, additions, deletions, layout changes, and logical-name arity handling. Wire the module and tests into compile order, add a release note, and include P6_REPORT.md.

Verification:

- ./.dotnet/dotnet build src/Compiler/FSharp.Compiler.Service.fsproj -c Debug /p:BUILDING_USING_DOTNET=true

- ./.dotnet/dotnet test tests/FSharp.Compiler.Service.Tests/FSharp.Compiler.Service.Tests.fsproj -c Debug /p:BUILDING_USING_DOTNET=true -- --filter-class "*TypedTreeDiffTests*"

- ./.dotnet/dotnet test tests/FSharp.Compiler.Service.Tests/FSharp.Compiler.Service.Tests.fsproj -c Debug /p:BUILDING_USING_DOTNET=true -- --filter-class "*SurfaceAreaTest*"
Add standalone baseline PE and portable PDB readers for hot reload, including token maps and MVID/PDB table snapshots.

Carry the F# synthesized-name snapshot module CDI codec and portable PDB read path, with recorded snapshots taking precedence over IL reconstruction.

Add focused component tests for snapshot round-trip, direct module CDI reading, baseline token maps, recorded fallback behavior, and EnC closure-name reconstruction.
…erload

CI compiles the snapshot round-trip and baseline reader tests with
FS0193: the explicit Assert.Equal<string> type application commits
overload resolution to the scalar Equal(T, T) shape while both
arguments are string arrays. Use Assert.Equal<string[]>, the form the
neighboring name-map tests already use, so resolution lands on the
structural array comparison everywhere.

Verified: FSharp.Compiler.ComponentTests builds clean;
EncMethodDebugInformationTests 22 passed, 0 failed (net10.0); fantomas
clean on the touched file.
@NatElkins
NatElkins force-pushed the hotreload-inprocess-compile branch from 3356e70 to 5fa9d32 Compare July 17, 2026 23:11
Verified the repository-wide Fantomas check, the service test project build with zero warnings and errors, and all 134 DeltaMetadata tests.
Carries lower-slice formatting into the in-process branch and verifies the complete repository with Fantomas.
@NatElkins
NatElkins marked this pull request as ready for review July 18, 2026 01:42
@NatElkins
NatElkins requested a review from a team as a code owner July 18, 2026 01:42
@github-actions github-actions Bot added ⚠️ Affects-Compiler-Output Tooling check: PR touches IL emission or codegen ⚠️ Affects-Build-Infra Tooling check: PR touches build infrastructure labels Jul 21, 2026
@github-actions

Copy link
Copy Markdown
Contributor

🔍 Tooling Safety Check — Affects-Build-Infra, Affects-Compiler-Output
Affects-Build-Infra: modifies FSharp.slnx (adds new projects)
Affects-Compiler-Output: modifies ilwrite.fs, ilwritepdb.fs, IlxGen.fs

Generated by PR Tooling Safety Check · opus46 5.4M ·

@T-Gro T-Gro left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🤖 This review was generated by AI (@expert-reviewer agent). Findings may contain inaccuracies — please verify independently.

Reviewed the actual PR head (7e8cf285c4) — specifically the in-process compile slice (CompileFromCheckedProject, the session EmitDelta plumbing, and the shared ilwritepdb/CompilerImports/SynthesizedTypeMaps edits). The default (flags-unset) path is well-isolated and the outer in-process path fails closed to the external build. Two concerns below are confined to the innermost experimental FSHARP_HOTRELOAD_INCREMENTAL_EMIT path, where the fail-closed guarantee is weaker (a wrong optimized tree produces a wrong baseline DLL and hence a wrong applied delta, rather than a rude edit). Non-blocking.

Comment thread src/Compiler/Service/service.fs Outdated
Comment thread src/Compiler/Service/service.fs Outdated
@T-Gro T-Gro added the AI-reviewed PR reviewed by AI review council label Jul 21, 2026
@T-Gro
T-Gro self-requested a review July 21, 2026 11:05
@NatElkins

Copy link
Copy Markdown
Contributor Author

The only Azure failure in this run was IcedTasks_Test_Release. The external IcedTasks build failed before compiler invocation because concurrent restores raced to create build/obj/dotnet-tool-restore-10.0.100-Windows_NT; MSBuild reported MSB3371 that another process was using the file. All other 45 jobs passed. Could someone with pipeline permissions rerun the failed job?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

⚠️ Affects-Build-Infra Tooling check: PR touches build infrastructure ⚠️ Affects-Compiler-Output Tooling check: PR touches IL emission or codegen AI-reviewed PR reviewed by AI review council

Projects

Status: New

Development

Successfully merging this pull request may close these issues.

2 participants