Add an experimental flag-gated in-process compile path for hot reload sessions#20031
Add an experimental flag-gated in-process compile path for hot reload sessions#20031NatElkins wants to merge 47 commits into
Conversation
❗ Release notes requiredYou can open this PR in browser to add release notes: open in github.dev
Warning No PR link found in some release notes, please consider adding it.
|
a4c8d4e to
eec9fa2
Compare
|
Oxpecker performance follow-up (2026-07-09): I reproduced Lanayx's 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 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:
The |
…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*"
…0260717/hotreload-baseline-reader
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.
…/20260717/hotreload-delta-emitter
RoslynBaselineComparisons parses its baseline table with System.Text.Json, which .NET Framework does not have, and FSharp.Compiler.Service.Tests compiles for net472 on every Windows CI job because the solution build covers both target frameworks. Gate the file to .NET Core in the project file like the SurfaceArea and checker tests. The nine comparisons keep running on all net10.0 legs. Verified: net10.0 build clean, RoslynBaselineComparisons 9 passed.
The synthesized-alignment and state-machine-shape tests build their updated sources by String.Replace over triple-quoted baseline literals. Triple-quoted literals inherit the source file's line endings, and the repo does not pin an eol for .fs files, so Windows checkouts hand these baselines CRLF while the replace needles use plain newline escapes. The needle then never matches and Replace silently returns the baseline unchanged, so the tests compile identical source twice: the alignment tests fail with NoChanges and the rejection tests invert because there is nothing left to reject. This broke six tests on the first Windows CI run that reached test execution; the compiler itself is unaffected since names and line shifts key off sequence-point line numbers. Normalize the affected baselines to plain newlines before any splicing so the fixture surgery is line-ending-agnostic on any host. Verified: component HotReload 236 passed 0 failed 2 skipped, fantomas clean on the touched file.
Problem: hot reload sessions still depended on an external rebuild to refresh the output assembly and PDB before delta emission, which left the perf path slower than a plain build and kept line-shift analysis tied to stale sibling PDB reads. Mechanism: add an internal CompileFromCheckedProject path that reuses checked project data, normalizes assembly references through TcImports, runs the TAST-to-IL and binary write in process, parses the emitted bytes back into the read-module representation, and carries the emitted module, DLL bytes, PDB bytes, and token mappings into delta emission. Gating: FSHARP_HOTRELOAD_INPROCESS_COMPILE enables the in-process compile. FSHARP_HOTRELOAD_INCREMENTAL_EMIT stays nested under that path and caches per-file optimized trees by CheckedImplFile identity. Timing, delta dump, and closure-name trace flags remain zero cost when disabled. Semantics: the session fails open to the external path when the flag is off or the in-process compile cannot produce artifacts. Existing fail-closed edit validation is left intact. Emitted artifacts are optional on DeltaEmissionRequest, so external callers keep the old reserialization path. Tests: FCS build, service test build, and component test build passed with 0 warnings and 0 errors. Service HotReload passed 428 of 428. Component HotReload passed 236 with 2 skipped. SurfaceArea passed 1 of 1 with no baseline change. Determinism guard passed 21 of 21 using the live determinism method filter because the requested DeterministicTests class filter matches zero tests in this checkout. Fantomas check passed over all touched source and test directories.
Bring the cumulative in-process hot reload branch to the reviewed post-dotnet#19929 compiler source and test state. This removes duplicate extracted metadata writer copies, keeps the final module placement, carries the latest in-process compile performance fixes, and includes the HotReloadDemo project referenced by FSharp.slnx. Verified: ./.dotnet/dotnet build FSharp.slnx -c Debug -v minimal passed; service HotReload tests 428 passed; component HotReload tests 236 passed, 2 skipped.
3356e70 to
5fa9d32
Compare
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.
github-actions
Bot
added
⚠️ Affects-Compiler-Output
|
🔍 Tooling Safety Check — Affects-Build-Infra, Affects-Compiler-Output
|
T-Gro
left a comment
T-Gro
left a comment
There was a problem hiding this comment.
🤖 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.
| typedImplFiles | ||
| |> List.map (fun implFile -> | ||
| HotReloadIncrementalEmit.getOrAdd implFile (fun () -> | ||
| let file, _, _ = optimizeImplFile optEnv0 SignatureHidingInfo.Empty implFile |
There was a problem hiding this comment.
The cached per-file path optimizes each implFile from a fresh optEnv0 / SignatureHidingInfo.Empty, discarding both the cross-file optimization environment (env') and the accumulated hiding info (hidingInfo') that optimizeWithThreadedEnvironment threads across files. Even with minimalSettings (localOpt/jitOpt/crossAssembly off, inline threshold 0), the threaded SignatureHidingInfo still governs which members are considered hidden/optimizable assembly-wide, so per-file-from-empty can diverge. Equivalence here is only empirically validated (the "INC-0 spike"); if it ever diverges, the emitted baseline DLL differs from fsc's output and the delta is diffed/emitted against a subtly wrong module — a silent miscompilation applied to the live process, which is weaker than the fail-closed→rude-edit guarantee the rest of the design relies on. Consider a determinism guard (e.g. compare/hash the cached result against the threaded-env output before trusting it, at least under a debug/verification flag) or document the precise optimizer invariants that make per-file-from-empty-env sound under minimalSettings.
| if isEnvVarTruthy "FSHARP_HOTRELOAD_INCREMENTAL_EMIT" then | ||
| try | ||
| optimizeWithCachedPerFileTrees () | ||
| with _ -> |
There was a problem hiding this comment.
This catch-all also swallows OperationCanceledException (FCS signals cancellation via exceptions) and asynchronous failures such as OutOfMemoryException, silently retrying the full threaded optimizer. Cancellation should propagate. Suggest: with :? System.OperationCanceledException -> reraise() | _ -> optimizeWithThreadedEnvironment ().
2 participants
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_COMPILEenvironment flag (plus the nestedFSHARP_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 buildper 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 byCheckedImplFilereference 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
ILModuleDefis 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),
SurfaceAreaTestgreen with no baseline change (everything new is internal).Sequencing
This PR is part of splitting the F# hot reload work (#19941) into small, independently reviewable PRs. The planned order:
--test:HotReloadDeltascapture hook).The dotnet-watch integration consuming this API is dotnet/sdk#55128.
Refresh status (2026-07-17)