[Performance] Run refresh work in the existing force runner

[soulomoon]

Run refresh work directly in the existing force runner, removing the redundant asyncWithCleanUp layer and its extra async registration and wait. Notice we already have data Wait to handle possible thread spawning. We have 5% perf improvement with this simple deletion

[soulomoon]

here is the perf report from codex gather from the pipeline:

Performance report

Benchmarked exact head b2037b82f1 against master 16bf046629.

• View diff
• Benchmark run

Metric	Result versus master
Geometric-mean totalT	−6.11%
Summed totalT	−4.78%
Median totalT	−4.42%
Faster benchmark cases	78 of 85

Results by configuration

Configuration	Geometric-mean totalT	Summed totalT	Median
Cabal, GHC 9.12	−4.17%	−3.95%	−2.27%
Cabal, GHC 9.14	−9.40%	−7.97%	−10.34%
lsp-types, GHC 9.12	−6.63%	−4.29%	−5.46%
lsp-types, GHC 9.14	−4.26%	−3.60%	−4.03%

code actions after cradle edit

Configuration	Result versus master
GHC 9.12	−6.30%
GHC 9.14	−4.96%

Conclusion: b2037b82f1 is faster than master across the benchmark suite, improving 78 of 85 successful benchmark cases.

[crtschin]

Nice! The code looks correct to me, but this is a bit subtle so it would be nice to improve the docs a bit while we're here.

[crtschin]

The comment above refreshDeps still refers to this spawn, mind updating the comment?

[soulomoon]

The builder would create a new thread for its deps ?

perhaps change it to Otherwise, new threads would be created to refresh the dirty deps (if any) and then compute the key itself in current thread ? WDYT

[crtschin]

Looks good!

[crtschin]

Could you elaborate this comment a bit, maybe write a Note about the ownership model?

See if you agree with how I read this currently:

• A force runner refers to the first thread that evaluates the rule thunk in the database.
• Refreshing refers to checking whether any dependencies changed and the key needs to be recomputed.
• If a session restart occurs while a thread is refreshing a key, this is safe as the shake database increment invalidates outdated values as well.

[soulomoon]

1. By design, builder find out the rule is still Dirty and wrap it into Spawn(If Running, just wrap it into Wait) and then waitConcurrently_ create its force runner and run it.
2. yes
3. If session restart interrupt the proccess, since the step incre and the next run would consider it as Dirty again even though the Runningstate would not be altered. see viewDirty.

[crtschin]

Do we have an existing note that captures this? If not, could you write one that explains how invalidation is structured and the invariants that hold?

[soulomoon]

added. feel free to update the note if there is any problem there

[crtschin]

Thanks a lot!