Repository navigation
Retention graph: draw each root by its label, layered from roots to targets - #14
Merged
Merged
Conversation
An array or a framework type has no source location ([PROF-DOTNET-SOURCE]), and the spec keeps such a site in the UI with no diagnostic. The wire adapter turned its absent file and line into "" and line 0, so the dashboard listed a 0th line and decorations tried to paint a file named "". MemoryAllocation's file and line are now optional: an unlocated site lists by name and paints no line.
The objects step summarised only the twenty largest types, so a type too small to rank (an F# union case such as OrderStatus+Paid, or the one list that holds a leak) had no row agreeing with its totals. The summary now appends that type's row after the ranked ones, keeping largest-first order.
Each test crate compiles tests/support on its own, and only the F# suite drives a fixture's line protocol, so Fixture::send and the stdout reader it reads were dead code in the C# suite's crate, which denies dead_code. Both now carry a documented per-item allow.
The fixture serialised its workload and its line protocol with one lock wrapped around Hot.spin, so the CPU profile no longer put Hot.spin on the busy thread's stack for most ticks and the F# CPU suite failed. The workload now runs lock-free and flags its own busy state, and idle work keeps allocating so a snapshot's induced collection can run. The FSharp.Core anchor test reads the order count from the reply, whatever its prefix.
…to targets The retention graph ignored is_root: decodeRefGraph dropped it, and the canvas coloured nodes by depth and drew only their types. A root's label, such as an F# module value's `static sessions`, never reached the screen, and roots were not distinct, against [PROF-UI-REFGRAPH]. The random force layout also turned every graph with many targets into an unreadable knot. Roots now survive decoding, are drawn in their own colour and named by their label above their type. Rows follow each node's depth: roots on top, targets along the bottom, and a row too tight to label names its type once with a count.
MelbourneDeveloper
added a commit
to Nimblesite/SharpLsp
that referenced
this pull request
Oct 18, 2026
…F# programs (#385) ## TLDR Fixes the three findings from the v0.22.0 static review that were still open on `main`, and closes the review's CI gap for Dependabot PRs. Closes #382, closes #383, closes #384. It also proves the profiler on real F# programs: two F# fixtures, nine new VS Code suites, and the lspkit and host fixes they forced (Nimblesite/lspkit#12–#15). It adds a narrated **Profiling F#** docs page, and the film kit that shoots it. ## Details ### v0.22.0 review findings - **#382: C# framework contexts** (`Workspace/TargetFrameworks.cs`). `FrameworkOf` used to treat any final `(…)` in `Project.Name` as a target framework. It now reads only the `(tfm)` that `MSBuildWorkspace` appends after the project file's stem, using `NativePaths.StemOf`. A single-target `Utility(net48).csproj` answers `{ active: null, available: [] }`, as [NETFX-CONTEXT] requires. - **#383: switch order** (`target_framework.rs`, `main.rs`, `diagnostics.rs`). - `handle_set` no longer announces. It returns a `Switched` value. - `main.rs` clears `nav_cache`, then calls the new `diagnostics::publish_now`, and only then calls `Switched::announce`, which sends `sharplsp/targetFrameworkChanged`, the declared refreshes and the response. - `publish_now` fetches and publishes every open document's diagnostics concurrently under the push gate, and waits for each first answer. A failed or provisional fetch goes on as an ordinary background push. - `fetch_and_publish_gated` and `publish_now` share one new `publish_once` attempt. `main.rs`'s `push_target` replaces the sidecar/path lookup that `trigger_diagnostics` duplicated. - **#384: test runner** (`src/test/suite/index.ts`). `resolveSuiteFiles` is exported and takes its globs and platform. Off Windows, a pattern whose matches are all `windowsOnly` suites now throws, naming the pattern and [NETFX-SCOPE]. The new `test-runner-selection.test.ts` is registered in the `workspace` chunk. - **Dependabot gate** (`.github/workflows/ci.yml`). The whole pipeline skips Dependabot, and the terminal `CI` job used to pass that empty run. A security bump that `dependabot-automerge.yml` failed to sweep therefore showed a green required check and could merge into `main` untested. A new step, `Fail a Dependabot PR to main, which ran no job`, fails it. That enforces [DIST-CI-DEPENDABOT]'s rule that the tested `dependabot-upgrades → main` consolidation PR is the only route into `main`. - **Specs.** - DOTNET-FRAMEWORK-SPEC [NETFX-CONTEXT] now covers the parenthesized-name rule and states that the host waits for each document's first publication. - DISTRIBUTION-SPEC gains the invariant "A selection never runs nothing". [DIST-CI-PROTECTION] rule 2 and [DIST-CI-DEPENDABOT] now say a Dependabot PR fails `CI`. - **F# sidecar** (`FSharpCSharpReferences.fs`). The in-memory compile of a referenced C# project now runs that project's source generators (e.g. PolySharp's `IsExternalInit` polyfill), so GitReader's net48 F# projects no longer read an unbuilt DLL (FS0078/FS0084). [SHARPLSP-ARCHITECTURE-PROJECTS-FSHARP-CSHARP-REFERENCES] says so; `FSharpCSharpReferenceTests` › "a C# project its source generator completes for .NET Framework is read in memory" failed first with FS0078/FS0084/FS0039. - **Real-repo setup** (`real-repo-helpers.ts`, `real-repo-kit.ts`). The clone and `dotnet restore` of a real repository now run asynchronously, in a setup hook with a budget of its own. A cold restore of a large multi-target solution (once about 10 minutes on a Windows runner, against 2 minutes usually) used to freeze the extension host and spend the whole 600 s budget that the solution load and warm-up also need. No assertion changes. - **Review doc.** `docs/reviews/V0.22.0-STATIC-REVIEW.md` gains a Resolution table. The Run/F5 `samePath` finding was already fixed on `main`. The Dependabot skip stays, but its terminal-gate pass is fixed. ### F# profiling This PR also adds the F# profiler test bed ([PROFILER-SHARED-LIBRARY-VSCODE]), and moves `lspkit` onto the profiler fixes those tests exposed. - **Fixtures** (`src/sharplsp/tests/fixtures/fsharp-profiling/`). Both are real F# programs driven over a stdin line protocol: - **`FsPricingEngine`** is the CPU workload. It runs CSV parsing, recursive discount rules, fuzzy search, route planning and reports, each on demand, on one workload thread. - **`FsOrderLedger`** is the memory workload. Every leak in it has its own F# module-value root: - a never-removed event handler; - a memoize cache; - a mailbox audit trail; - an undo history of immutable `Map`s; - rejection capture; - a blob store. `release`, `audit pause` and `burn on` give every test a deterministic heap and CPU profile. - **VS Code suites** (`src/editors/vscode/src/test/suite/`): - Through the UI: - `profiler-fsharp-cpu-e2e` (new), and `profiler-fsharp-e2e` (extended with an F# heap zoo): profile from the process row, F# hot functions and lines in their own source files, the results panel, exports. - `-memory-e2e`: track, snapshot, diff, leak confidence, decorations on the constructing line. - `-retention-e2e`: retention graphs rooted at module values by their source names, objects-by-type, GC. - `-corner-e2e`: concurrent sessions, refusals and error codes, heap profile, generation bytes. - `-soak-e2e` and `-soak-cpu-e2e`: long runs, many snapshots, coherence of the store and the UI. - Over the wire, with no UI: `profiler-lsp-protocol-e2e`, `profiler-lsp-cpu-e2e` and `profiler-lsp-memory-e2e`. - Shared helpers live in `profiler-fsharp-kit`, `profiler-fsharp-soak-kit`, `profiler-fsharp-artifacts` and `profiler-fixtures`. `profiler-fixtures` holds the fixture locations and Release builds, moved out of `profiler-kit`. - `profiler-ui-debuggee` takes a `DebuggeeSource`, so the debug-launched suites can build an F# debuggee. - All the new suites fall under the `profiler*.test.js` chunk glob. - **lspkit** moves from `031bfc8` to lspkit main's `6ea8dce`, which carries the F# profiler fixes: - **Nimblesite/lspkit#12:** - idle samples no longer count as active; - a frame every thread shares counts once per tick; - a named runtime on a busy PID is `UNSUPPORTED_RUNTIME`; - an array (`Order[]`) is never located on the line that builds an `Order`; - a retention graph says it is truncated when instances past the targets' quarter of the node budget are left out; - a memory step on an exited program is `PROCESS_NOT_FOUND`; - each diff replaces the last diff's leak badges. - **Nimblesite/lspkit#13:** - a memory site with no location stays unlocated in the UI; - a type whose PDB names a build server's checkout, as FSharp.Core's does, gets no location instead of a path that does not exist on this machine; - `objectsByType` keeps the requested type's own summary row when it is too small to rank among the twenty largest. An F# union case such as `OrderStatus+Paid` is an example. - **Nimblesite/lspkit#14:** the retention graph draws every root with its kind and label, such as an F# module value's `static sessions`, in a layered layout from roots down to targets. - **Nimblesite/lspkit#15:** - the timeline's peak is the session's peak, not each snapshot's heap size; - a heap snapshot taken during a CPU session names every type, because the CPU session now enables only the sampler; - a hot function with no location stays unlocated in the UI instead of reaching it as file `""`, line 0; - the UI's Stop sends `memory.stop`, so the server session and its graphs end with it; - a timed CPU session leaves `cpu.list` once its duration elapses, and its report is kept for the next `stop`. - **Host** (`src/sharplsp/src/profiler/host.rs`). A profiler command whose arguments are not exactly one params object is `INVALID_PARAMS` ([PROF-WIRE-METHODS]); an absent argument list is still empty params. Every production caller already sends one object (the lsp-profiler-ui library's 13 calls). Three older suite call sites sent `cpu.list` bare; they now pass `{}`, with no assertion changes. - **Plan.** `docs/plans/PROFILER-PLAN.md` records the F# test bed and the 16 defects it found. - **Website.** - A new **Profiling F#** page (en, ja, zh) shows what the profiler does with F#'s compiled code: - closures and pipeline stages by name; - records and union cases painted on the line that builds them; - module values named as written in the retention graph. The profiler, F# and memory-leak pages link to it. - The F# pages play `profiler-fsharp`, a narrated and captioned film of `FsStorefront`'s CPU and memory. - `{% video %}` plays a narrated film unmuted with caption tracks; autoplaying loops stay muted. - **Film tools** (`src/website/tools/profiler-film/`). They become a reusable kit: - a film is one file in `films/`, shot with `make _film FILM=<film>`; - demos, including the new F# `FsStorefront`, live in `demos/`; - `kit/` holds the stage, camera, overlay and locally synthesised voice and score; - the README says how to add a film. ## How Do The Automated Tests Prove It Works? ### v0.22.0 review findings Each test was written first and confirmed to fail for its bug before the fix: - **`WorkspaceManagerTargetFrameworkTests.A_single_target_project_has_no_framework_to_choose`** is now a theory over `Single` and `Utility(net48)`, on real restored projects. - Before the fix: `Assert.Null() Failure … Actual: "net48"`. - After: both cases answer null/empty, keep the right `Project`, and refuse a switch. - The new `A_multi_targeted_project_named_like_a_framework_answers_its_declared_frameworks` proves `Tool(net10.0).csproj` still answers `[net48, net10.0]` and switches. All 9 tests in the class pass. - **`test_full_stack_target_framework_switch`** (host e2e, real Roslyn sidecar). The fixture gained a `#else` member with an unused local, so only `net10.0` reports `CS0168`. The new `assert_switch_order` asserts that the publication carrying `CS0168` (at 8:37) arrives before `sharplsp/targetFrameworkChanged`, which arrives before each refresh. - Before the fix: the announcement and both refreshes arrived with no re-published diagnostics. - After: passes. - **`test-runner-selection.test.ts`** checks a real `windowsOnly` name from `test-chunks.json`. - Off Windows, `netfx-language.test.js` and `netfx-*.test.js` throw with the platform reason, while `*.test.js` runs only the non-Windows suite. - On `win32`, the same patterns select their suites, and a pattern matching nothing still fails. - With the guard disabled it fails with `Missing expected exception`; with the guard: 2 passing. - **`security-gates.test.mjs` › the terminal gate fails a Dependabot PR to main, whose pipeline ran nothing.** It parses the real `ci.yml` and asserts: - CI only sees PRs to `main`, and `detect-changes` skips Dependabot. - A `github.actor == 'dependabot[bot]'` step fails with `exit 1` before `Report`, naming `dependabot-upgrades`. - Before the fix: `AssertionError: a Dependabot PR must not report green`. After: all 35 `tools/ci` contract tests pass, and actionlint is clean. ### F# profiling Each fix was proven by a test that failed before it. - **Host:** `profiler::host::tests::anything_but_one_params_object_is_malformed` and `one_params_object_or_no_argument_list_is_well_formed`. The VS Code test `profiler-lsp-protocol-e2e` › "all seventeen requests enforce their typed arguments" failed before the fix with `sharplsp.lspProfiler.capabilities with [] should have failed`. - **lspkit:** each fix in #12–#15 lands with a failing-first engine or UI test. For example: - `dotnet_e2e` › `objects_of_a_small_type_keep_its_own_summary_row` failed with no summary row for `List<LeakedPayload>`. - `a_duration_caps_the_recorded_time` now also requires that `cpu.list` omits a session past its duration; it failed with the session still listed. lspkit's `make ci` passes 412/412 and its UI suite passes 161. - **VS Code, run locally on macOS** at lspkit `6ea8dce`, covering `profiler-fsharp-*`, `profiler-lsp-*` and `memory-flows`. Before the fixes: 24 passing, 8 failing. After: every test passes. One run gave 50 passing; the last, "the profiler tells the truth about four sustained F# workloads", passes (7/7 in its suite) since its Stopwatch window opens once samples flow ([PROF-AGGREGATION-LOGIC]). Before that, the window counted the profiler's unsampled attach time. - The eight failures were the fall of the timeline's peak, a stopped session still answering, a stage's sampled share, a hot framework frame with an invented file, leak types named by bare address, the undo history's closed generic, a timed session that never ended, and `capabilities` accepting `[]`. - **The rest of the pipeline, run locally:** - Rust shards: 292/292 and 261/261. - .NET sidecars: 125, 568 and 437 passing. - Tooling: 74 passing. - Analyse: Rust, Zed, csharpier, Prettier, ESLint, `tsc` and citations all clean; Deslop at 6.59%.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
TLDR
The retention graph draws every root with its kind and label, in a layered layout from roots down to targets.
Details
isRoot, so a root such as an F# module value'sstatic sessionswas never labelled and roots looked like any holder.memory-ref-graph.tsnow uses a layered layout: a node's row is itsdepth(its distance from the targets), so roots sit on top and targets along the bottom, and each root is drawn with its kind and label.How Do The Automated Tests Prove It Works?
profiler-webviews.test.ts"retention graph names each root by its label, above the holders and the targets they keep alive": each root's label is drawn, and roots sit above holders and targets. It failed before the fix.profiler-webviews.test.ts"a decoded retention graph keeps each root, its kind and its label": decoding keepsisRoot,rootKindand the label. It failed before the fix.