hls graph runtime engine experiment by soulomoon · Pull Request #4927 · haskell/haskell-language-server

soulomoon · 2026-05-06T13:22:25Z

WIP/experimental

The main idea is to let hls graph runtime have finer control over the runtime threads and better dependency tracking.
reduce the number of times actual thread intrupt is need. It also enable us build a better test infrastructure to inspect the hls graph runtime information that we no longer need to wait unconditionally in testing

…Level0/1M01

…untime-engine # Conflicts: # .github/workflows/bench.yml

…wPercentageDiff

…ook parameter

(cherry picked from commit e4ead4f)

(cherry picked from commit c71527d)

…-runtime-engine

soulomoon · 2026-05-12T14:41:56Z

Here is the main hls-graph changes we talked about. @fendor

hls-graph now has an explicit runtime engine for running actions, tracking runtime dependencies, and restarting selectively.

The benchmark shows the intended tradeoff. Startup improves broadly, mostly around 25-32%, from skipping kills for non-dirty work. For very short single-edit operations, the central thread maintenance cost is visible in userT / totalT, so some “after edit” cases regress. In the typing-burst cases, which better match human editing, that overhead is amortized and is partly offset by avoiding unnecessary non-dirty kills.

crtschin

Could you elaborate on the design and how it relates with the previous setup? This is all quite intricate concurrent code, and there's a lot that's unfinished here with the TODO comments and dead code.

crtschin · 2026-07-21T19:08:28Z

+-- | A no-file rule that triggers the IDE "kick" action
+data Kick = Kick
+    deriving (Eq, Show, Generic)
+instance Hashable Kick
+instance NFData   Kick
+
+type instance RuleResult Kick = ()


Why turn kick into a rule? I thought the reason it existed is because it signified a cache invalidation.

crtschin · 2026-07-21T21:48:24Z

+-- | Create a new key that is guaranteed not to collide with any other key.
+-- This is useful for keys that are not based on user data, e.g., for
+-- tracking temporary actions.
+newDirectKey :: Int -> Key
+newDirectKey i = UnsafeMkKey $ negate (abs i + 1)


I don't quite understand the point of this abstraction and the negate . abs looks like we're encoding some invariant?

Keys already don't collide with each other, unless they are specifically the same key right? newDirectKey n == newDirectKey m iff n == m, which is also true for newKey, no?

crtschin · 2026-07-21T22:02:40Z

+    databaseRuntimeDepRoot             :: SMap.Map Key KeySet,
+
+    databaseRRuntimeDepRoot            :: SMap.Map Key KeySet,
+    databaseRRuntimeDep                :: SMap.Map Key KeySet,
+    -- it is used to compute the transitive reverse deps, so
+    -- if not in any of the transitive reverse deps of a dirty node, it is clean
+    -- we can skip clean the threads.
+    -- this is update right before we query the database for the key result.


The dependencies of a rule is already stored in the rule results, ResultDeps. What does this extra bookkeeping save us complexity-wise? And do we need all of these?

Yes, having two sources of truth is problematic. I think we should consolidate them after this work is complete.
I’m implementing more selective thread cancellation across restarts, so only threads are actually dirty will be cancelled. To do that reliably, we need to track dependencies right in time at the point when a thread is started.
Rule results are produced too late for this purpose: the database values' deps are updated only after the rule finishes computing, leaving a significant gap between starting the thread and recording its dependencies.

crtschin · 2026-07-21T22:09:57Z

+    databaseTransitiveRRuntimeDepCache :: SMap.Map KeySet TransitiveDirtyKeys,
+    -- ^ this is a cache for transitive reverse deps if we have computed it before
+    -- and the databaseRRuntimeDep did not change since last time
+    -- it is very useful for large projects where many files depend on a few common files
+    -- e.g. we do not want to recompute the transitive reverse deps every time we enter a letter
+    -- to a file.


Having KeySet as the domain of the map makes me think that this can explode quite quickly in storage overhead, with too small hit-rate to justify having it.

If a user types a key, the previous setup induced a kick which did cleanup and ensured freshness of results. What's the difference with the new setup?

crtschin · 2026-07-21T22:15:23Z

 viewDirty :: Step -> Status -> Status
-viewDirty currentStep (Running s _ _ re) | currentStep /= s = Dirty re
+-- viewDirty currentStep (Running s re _ _) | currentStep /= s = Dirty re
 viewDirty _ other = other

+
+viewToRun :: Maybe Status -> Status
+-- viewToRun _currentStep (Dirty _) = Nothing
+-- viewToRun currentStep (Running s _re _ _) | currentStep /= s = Nothing
+viewToRun Nothing      = (Dirty Nothing)
+viewToRun (Just other) = other


Both of these commented out bits look very load-bearing. What's the bit that assumes responsibility for this behavior? Or is it not relevant anymore, and why?

If we are doing selective thread cancel during shake restart, we do not need currentStep /= s. dirties is already marked correctly by other parts of the system(e.g. In builderOne', we roll back to make it dirty if the thread is cancelled during its startup).

crtschin · 2026-07-21T22:19:02Z

+    mempty = RunDependenciesSame
+instance Semigroup RunMode where
+    RunDependenciesSame <> b    = b
+    RunDependenciesChanged <> _ = RunDependenciesChanged


Are these instances appropriate? If runMode a == RunDependenciesChanged and runMode b == RunDependenciesSame, I can only assume I want to make independent decisions here and refresh only a, and ignore b, do you ever need a <> b?

crtschin · 2026-07-21T22:36:00Z

+                    modifyTVar' databaseThreads ((deliver, a):)
+                    -- make sure we only start after the restart
+                    putTMVar startBarrier ()
+    a <- asyncWithUnmask $ \restore -> (handler =<< ((restore $ atomically (readTMVar startBarrier) >> (Right <$> asyncBody)) `catch` \e@(SomeException _) -> return (Left e)))


Could you break this line up? It does a lot of things.

asyncWithUnmask so we won't be interrupted after we enter handler. The handler is passed in here to alter the some key states outside to keep invariant of the system intact (e.g. changing databaseValues).

crtschin · 2026-07-21T22:38:56Z

+-- 3. Controlled start coordination via barriers
+-- 4. Exception safety with rollback on registration failure
+-- @ inline
+{-# INLINE spawnAsyncWithDbRegistration #-}


I don't think this inline pragma does much but increase code size at callsites, which also isn't great performance-wise. IIRC inlining IO functions doesn't expose significant GHC optimization opportunities.

crtschin · 2026-07-21T22:44:29Z

+
+mkDelayedAction :: String -> Logger.Priority -> Action a -> IO (DelayedAction a)
+mkDelayedAction s p a = do
+    u <- newUnique


Why mint uniques? Where does the need for global uniqueness come from? Isn't the String label enough, IIRC derived from the rule type-level keys? How does this interact with caching?

After replacing the AIO, we do not have to cancel all the DelayedAction during restart but only the actually dirty ones. So we identify every DelayedAction with unique.

soulomoon added 4 commits May 6, 2026 20:16

hls-graph: isolate runtime engine improvements

23d1e81

Isolate hls-graph runtime engine improvements

b894af1

ghcide: integrate hls-graph runtime engine

bf753af

chore: remove accidental eventlog

f72ca37

soulomoon requested review from fendor and wz1000 as code owners May 6, 2026 13:22

soulomoon marked this pull request as draft May 6, 2026 13:24

soulomoon added the performance Issues about memory consumption, responsiveness, etc. label May 6, 2026

soulomoon changed the title ~~Codex/hls graph runtime engine~~ hls graph runtime engine May 6, 2026

soulomoon changed the title ~~hls graph runtime engine~~ hls graph runtime engine experiment May 6, 2026

soulomoon added 20 commits May 7, 2026 00:47

Confirm reactor shutdown before cleanup

8693fde

Run LSP shutdown after worker teardown

71e697e

Fix shutdown warning helper masking

49f9304

chore: update GHC version in benchmark matrix

8f80b9e

chore: remove GHC 9.8 from benchmark matrix

eecfa90

fix bench 9.12

ed2a709

update bench config

a11fbe4

update bench config

46d5609

update bench config

0f4ea7b

update bench CI to 9.12

5482d79

refactor: update GHC version handling in benchmark workflow

28a6ddd

Enable benchmark CI for GHC 9.14

5a5a837

Drop benchmark SVG generation

8cb6663

refactor: remove unused dependencies for GHC 9.14

cd6ee40

Use Cabal 3.16 for benchmark CI

1de9bab

fix: update Cabal version to 3.16 in benchmark workflow

abb8a0e

Update benchmark examples for GHC 9.14

e9530c2

Fix benchmark target project setup

9786cd7

fix: restore benchmark configurations for MultiLayerModules and Dummy…

399bba3

…Level0/1M01

Merge branch 'master' into fixbench

cb69dd8

soulomoon added 13 commits May 7, 2026 20:20

Merge remote-tracking branch 'origin/fixbench' into codex/hls-graph-r…

04da422

…untime-engine # Conflicts: # .github/workflows/bench.yml

fix: hide uninformative benchmark diffs

2f8a559

Merge branch 'fixbench' into codex/hls-graph-runtime-engine

1aec4e9

fix: remove threshold for insignificant percentage differences in sho…

b7e950a

…wPercentageDiff

refactor: simplify spawnAsyncWithDbRegistration by removing registerH…

2a71e12

…ook parameter

fix: handle unmatched Key cases in fromKey and fromKeyType functions

1bcc1bd

fix: correct newDirectKey implementation to avoid key collisions

6095bcd

Clarify hls-graph runtime restart keys

027d588

Add no-file rule for global kick action and refactor kick usage

cf73b99

(cherry picked from commit e4ead4f)

fix: adjust doKick behavior to always kick during testing

089a781

(cherry picked from commit c71527d)

Merge branch 'codex/extract-dokick-optimization' into codex/hls-graph…

06b68c0

…-runtime-engine

Merge branch 'master' into codex/hls-graph-runtime-engine

a9739b2

Add typing burst benchmark coverage

f8a9250

soulomoon added 3 commits May 16, 2026 07:18

Merge branch 'master' into codex/hls-graph-runtime-engine

c977429

Merge branch 'master' into codex/hls-graph-runtime-engine

d0971fd

Merge branch 'master' into codex/hls-graph-runtime-engine

21b5f26

soulomoon mentioned this pull request Jun 26, 2026

Discard rule results from runs superseded by Shake restarts #4988

Closed

crtschin self-requested a review June 26, 2026 08:37

Merge branch 'master' into codex/hls-graph-runtime-engine

27d218b

soulomoon mentioned this pull request Jul 21, 2026

WIP: Ensure restarts cancel outstanding shake threads #5021

Closed

crtschin reviewed Jul 21, 2026

View reviewed changes

Uh oh!

Uh oh!

Conversation

soulomoon commented May 6, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

soulomoon commented May 12, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

crtschin left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

soulomoon Jul 22, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

soulomoon commented May 6, 2026 •

edited

Loading

soulomoon commented May 12, 2026 •

edited

Loading

soulomoon Jul 22, 2026 •

edited

Loading