Improve hls runtime keep async only databse keys downsweep skip non dirties 1#33
Conversation
restartSession to avoid stale GhcSession results and lost diagnostics
… into removeDupTest
…anup - Replace ad-hoc AIO with structured concurrency (TVar + async registry); builder returns results directly; remove lazy splitIO/unsafePerformIO - Reduce redundant thread creation; use per-key builderOne and STM retry instead of spawning; fewer races - Add AsyncParentKill (ThreadId, Step) and treat it as async; use cancelWith from Shake to scope cancellation to the current session - Mask critical sections and do uninterruptible cleanup on exception (mark Dirty) to avoid stuck Running and hangs - Adjust types/wiring (Running payload, runAIO takes Step, compute/refresh signatures); minor tweaks in ghcide Shake/Plugin.Test Fixes haskell#4718
before releasing the serialized queue
2. spawns only if need when building a key, see `builderOneCoroutine`
This reverts commit 7bf6fde.
ⓘ You've reached your Qodo monthly free-tier limit. Reviews pause until next month — upgrade your plan to continue now, or link your paid account if you already have one. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: da810c7a13
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| (Dirty _prev) -> retry | ||
| (Clean r) -> return (key, r) | ||
| (Running _step _s) | ||
| | memberStack key stack -> throw $ StackException stack | ||
| | otherwise -> retry |
There was a problem hiding this comment.
Handle failed refreshes instead of waiting forever
builderOneFinal now waits by retry for both Dirty and Running, but there is no longer a consumed wait handle for spawned refreshes; the new barrier is written but never read. If a refresh thread throws a non-async exception, the status may never transition to Clean, so this STM transaction can block indefinitely and hang the build/restart path instead of surfacing the failure.
Useful? React with 👍 / 👎.
| -- actionQueue should also be empty, otherwise there might be more diagnostics to publish | ||
| isEmpty2 <- isActionQueueEmpty actionQueue | ||
| unless (isEmpty && isEmpty1 && isEmpty2) retry |
There was a problem hiding this comment.
Avoid self-deadlock on action queue emptiness check
This adds isActionQueueEmpty actionQueue as a hard precondition before publishing diagnostics, but waitUntilDiagnosticsPublished is called from kickSignal inside delayed actions. While an action is running it remains in inProgress, so the queue can never be observed empty by that same action, causing test-mode runs that use runWithSignal/runWithSignalAction to block on themselves.
Useful? React with 👍 / 👎.
5a6492b to
da810c7
Compare
…-keep-async-only-databse-keys-downsweep-skip-non-dirties-1
…bse-keys-downsweep-skip-non-dirties-1 # Conflicts: # bench/config.yaml # cabal.project # shake-bench/shake-bench.cabal
No description provided.