ghcide: stop accumulating equivalent values across early-cutoff rebuilds#4942
Draft
parsonsmatt wants to merge 11 commits into
Draft
ghcide: stop accumulating equivalent values across early-cutoff rebuilds#4942parsonsmatt wants to merge 11 commits into
parsonsmatt wants to merge 11 commits into
Conversation
- shake-bench: parse GC pause time, max live data, productivity, major/minor GC counts from +RTS -s; emit .hp.csv top-cost-centre summary alongside each .hp file - ghcide-bench: capture per-sample latencies; emit p50/p95/p99/stddev CSV columns; add memory-pressure experiment - bench: smoke phony target for fast local iteration; parameterise MultiLayerModules.sh with --depth/--width; add MultiLayerModulesXL (200x50 = 10k modules) for production-scale stress Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
defineEarlyCutoff' reuses the previously-cached Succeeded value when the cutoff fingerprint matches the prior. Without this, every cutoff-matched rerun stored a freshly allocated (but structurally equivalent) value, and downstream rules whose deps had not changed continued to hold pointers to the prior value. Multiple equivalent copies of large shared values accumulated across rebuilds. On a 10k-module MLM-XL project, this cut NodeKey_Module heap growth by ~3000x (241 KB/s -> 81 B/s) and overall heap leak by ~50% (1.9 MB/s -> 0.95 MB/s). Largest source of remaining leak is HPT/UniqDFM churn via GhcSessionDeps; tracked separately. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
GhcSessionDeps previously used defineNoDiagnostics (no early-cutoff
fingerprint), so the prior pointer-identity preservation patch on
defineEarlyCutoff' could not engage for it. As a result, every re-run
of GhcSessionDeps allocated a fresh HscEnvEq even when the resulting
value was structurally equivalent — and downstream rules whose deps
had not changed kept pointing at the prior version, accumulating
generations of equivalent HscEnv/HPT.
Switch GhcSessionDeps to defineEarlyCutoff and derive a cutoff
fingerprint from the inputs that fully determine HscEnvEq identity:
- the file's own source hash (covers pragma/dflags effects via
msrHscEnv's initializePlugins step)
- the file's import-level mod summary fingerprint
- the trans-deps fingerprint (covers import-graph changes anywhere
in the file's transitive cone)
- the iface fingerprint of each direct dep (covers signature
changes that would actually affect typecheck)
When all four match the prior run, the new defineEarlyCutoff' code
path reuses the cached HscEnvEq pointer — eliminating accumulation of
ModuleGraph-internal data structures (NodeKey_Module, GWIB, Set.Bin
all drop out of the retainer set entirely on MLM-XL) and cutting
overall heap at fixed wall-clock by ~36%.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This reverts commit 1db55f2.
Hlint flagged ws !! i in secondsAt. Use safe drop/pattern-match instead. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
- Restore samples: 50 in config.yaml (was temporarily 10 for iteration) - Drop config_overnight.yaml (RTS-flag exploration artifact, not used by CI) - Trim verbose comments in Main.hs smoke target and MultiLayerModules.sh Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
After haskell-actions/setup@v2.10.3 finishes, the active GHC is in an "unset" state. With --unset, `ghcup gc` then deletes the active GHC's binary, and `cabal build` fails with Cabal-7620 (ghc not found). All Ubuntu test/flags jobs have been failing this way for new PRs. Drop the --unset flag. The other gc flags (--share-dir --hls-no-ghc --cache --tmpdirs) still free the disk space the gc step exists for. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The unconditional pointer-identity preservation in defineEarlyCutoff' broke rules whose action body has value-dependent side effects. In particular, GetLinkable registers the freshly built linkable and unloads the prior; returning the cached value to downstream then left consumers (e.g. the eval plugin's :info command) pointing at bytecode the action had just unloaded. Make value reuse opt-in via a new flag on defineEarlyCutoff', and a parallel wrapper defineEarlyCutOffNoFileReuseValue. Apply only to GetModuleGraph, which is pure and accounts for nearly all of the heap savings (NodeKey_Module / GWIB / Set.Bin retainers). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Code-review follow-ups on the GetModuleGraph value-reuse change and the
benchmark tooling it ships with.
ghcide:
- defineEarlyCutoff' only reuses a clean prior value (Stale Nothing); a
deletion-marked Stale (Just _) is no longer resurrected as Succeeded.
- Replace the hand-inlined addRule copy in defineEarlyCutOffNoFileReuseValue
with a RuleNoDiagnosticsReuseValue constructor + shared addNoDiagnosticsRule
helper, and factor the empty-path guard into noFileBody.
bench/shake-bench:
- run_pass2_costcentre.sh: drop --lsp-config (it made ghcide-bench read empty
stdin from /dev/null and error out, masked by `|| echo`).
- MultiLayerModules.sh: require a value for --depth/--width (${2:?...}) instead
of looping forever when it is missing.
- summarizeHpProfile: foldl' + forced max (no thunk buildup); skip the .hp.csv
summary under NoProfiling instead of rewriting the dummy each build.
- parseRTSStats: whitespace-normalise label matching (robust to GHC -s spacing).
- Collapse showMB/showSeconds/showDouble/showInt to a single max-0 clamp.
- quantileSorted: sort each sample list once per row instead of 6x.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The comment justified skipping `Stale (Just _)` values by calling them deletion/GC markers. That is inaccurate: `Stale`'s first field is a `Maybe PositionDelta`, written only by the persistent-rule fallback in lastValueIO (a value loaded from disk, not produced by the rule's action this run). State the real reason — such a value is not a trustworthy prior to reuse — so future readers don't misread the Value lifecycle. Comment-only; no behavior change. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
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.
Summary
Cuts steady-state heap retention by ~34% on a 10k-module synthetic project (MLM-XL).
defineEarlyCutoff'pointer-identity preservation (ghcide/src/Development/IDE/Core/Shake.hs)When an early-cutoff rule's fingerprint matches the prior run, reuse the previously cached
Succeededvalue instead of storing the freshly-computed (structurally equivalent) one. Downstream rules whose own deps didn't change continue to hold a pointer to the prior generation — so without this fix, every rebuild stacks another generation in memory.Worst offender:
GetModuleGraph, whosemgTransDeps :: Map NodeKey (Set NodeKey)is ~50M entries at the 10k-module scale.Validation
Bench: 10k modules (
depth 200,width 50),ghcide-bench --select 'memory pressure' --samples 25.Retainer growth rate (
+RTS -hT -i1):NodeKey_ModuleGWIBSet.Internal.BinTHUNK_1_0Total heap @ t≈2140s: 6,225 MB → 4,113 MB (−34%).
Risk
Changes core
defineEarlyCutoff'semantics: on a fingerprint match the cache storesoldVinstead of the freshly-computed equivalent. Behavior change for any caller that compared by reference rather than by structural equality — none should, but worth a review.Residual work (not in this PR)
Word64Map.Bin+TaggedVal(UniqDFM internals, likely NameCache or HPT contents) still grow ~475 kB/s after this fix. Source needs retainer-attribution profile (-hr) to pinpoint; followup PR.🤖 Generated with Claude Code