haskell
Fix hls-graph: phantom dependencies invoke in branching deps (resolve #3423)
phantom depencies is invoke becase dependencies have preconditions in rules, see #3423. This pr is intend to fix that. This might also fix some of the flaky tests.
Introduction a new ResultDepsTree to ResultDeps which contains a treelike minimap of dependencies for a key.
It provides the branching semantic when we reflesh a key. Key in depsTreeNodeCond is responsible for computing the branching condition. It equipped a continuation depsTreeContinuation, to decided what dependencies are needed after that.
Pros: we have correct semantic for the rules with branching dependencies.
cons:It requires us to manually annotate the rule by providing the ResultDepsTree.<\del>
In favor of @wz1000 appoach of running deps linearly. It modify the deps result from KeySet to [KeySet] to make sure the result is sorted
we initialy thought it would have performance impact on the build system. But it turns out instead of performance lost, we actaully have performance gain since it avoid building the phantom depencies.
Overall things have been done:
- Fix up hls-graph phantom depencies issue by reflesh linear deps in a linear manner.
- Fix the bench for macos and added semantic tokens bench mark.
- add test to hls-graph to ensure phantom depencies would not be invoke.
Result: Now no more phantom dependencies would be invoked in hls-graph, gaining correctness, less runtime and less mem usage at the some time.
First bench mark
| version | configuration | name | success | samples | startup | setup | userT | delayedT | 1stBuildT | avgPerRespT | totalT | rulesBuilt | rulesChanged | rulesVisited | rulesTotal | ruleEdges | ghcRebuilds | maxResidency | allocatedBytes |
| -------- | ------------- | ------------------------------ | ------- | ------- | ------- | ----- | ------ | -------- | --------- | ----------- | ------ | ---------- | ------------ | ------------ | ---------- | --------- | ----------- | ------------ | -------------- |
| upstream | All | edit-header | True | 50 | 2.00 | 0.00 | 30.51 | 0.05 | 2.84 | 0.28 | 30.57 | 14 | 12 | 1363 | 5910 | 31261 | 51 | 290MB | 136737MB |
| HEAD | All | edit-header | True | 50 | 1.97 | 0.00 | 36.75 | 0.07 | 2.81 | 0.35 | 36.82 | 17 | 14 | 1367 | 5910 | 31261 | 51 | 298MB | 160490MB |
| upstream | All | edit | True | 50 | 2.39 | 0.00 | 30.74 | 0.03 | 2.82 | 0.28 | 30.77 | 14 | 12 | 1363 | 5909 | 31255 | 51 | 296MB | 134882MB |
| HEAD | All | edit | True | 50 | 2.04 | 0.00 | 35.39 | 0.02 | 2.81 | 0.33 | 35.42 | 17 | 14 | 1367 | 5910 | 31261 | 51 | 297MB | 156089MB |
| upstream | All | hover | True | 50 | 2.02 | 0.00 | 2.85 | 0.01 | 2.81 | 0.00 | 2.86 | 5094 | 5093 | 5094 | 5912 | 31267 | 2 | 164MB | 15418MB |
| HEAD | All | hover | True | 50 | 1.85 | 0.00 | 2.82 | 0.01 | 2.78 | 0.00 | 2.83 | 5126 | 5125 | 5126 | 5912 | 31267 | 2 | 127MB | 15332MB |
| upstream | All | semanticTokens | True | 50 | 1.97 | 0.00 | 66.98 | 1.28 | 3.65 | 0.65 | 68.27 | 12 | 10 | 1364 | 5914 | 31271 | 52 | 298MB | 289880MB |
| HEAD | All | semanticTokens | True | 50 | 1.94 | 0.00 | 67.98 | 1.11 | 3.43 | 0.66 | 69.10 | 12 | 10 | 1364 | 5914 | 31271 | 52 | 300MB | 301805MB |
| upstream | All | hover after edit | True | 50 | 1.92 | 0.00 | 3.08 | 30.96 | 2.94 | 0.00 | 34.04 | 21 | 18 | 1371 | 5912 | 31267 | 51 | 305MB | 146999MB |
| HEAD | All | hover after edit | True | 50 | 2.01 | 0.00 | 3.06 | 34.95 | 2.93 | 0.00 | 38.01 | 21 | 18 | 1371 | 5912 | 31267 | 51 | 308MB | 162980MB |
| upstream | All | getDefinition | False | 50 | 1.99 | 0.00 | 0.00 | 0.00 | 0.00 | 0.00 | 2.87 | 5125 | 5124 | 5125 | 5907 | 31262 | 2 | 158MB | 15198MB |
| HEAD | All | getDefinition | False | 50 | 1.95 | 0.00 | 0.00 | 0.00 | 0.00 | 0.00 | 2.68 | 5122 | 5121 | 5122 | 5907 | 31262 | 2 | 189MB | 15108MB |
| upstream | All | getDefinition after edit | False | 50 | 2.17 | 0.00 | 0.00 | 0.00 | 0.00 | 0.00 | 3.30 | 5127 | 5125 | 5127 | 5907 | 31262 | 2 | 134MB | 15222MB |
| HEAD | All | getDefinition after edit | False | 50 | 1.87 | 0.00 | 0.00 | 0.00 | 0.00 | 0.00 | 2.87 | 5128 | 5126 | 5128 | 5907 | 31262 | 2 | 164MB | 15264MB |
| upstream | All | completions | True | 50 | 1.98 | 0.00 | 5.01 | 0.01 | 3.00 | 0.02 | 5.03 | 5124 | 5123 | 5124 | 5914 | 31265 | 2 | 244MB | 20874MB |
| HEAD | All | completions | True | 50 | 1.92 | 0.00 | 4.81 | 0.01 | 3.06 | 0.02 | 4.82 | 5124 | 5123 | 5124 | 5914 | 31265 | 2 | 221MB | 20876MB |
| upstream | All | completions after edit | True | 50 | 1.98 | 0.00 | 12.07 | 22.16 | 3.16 | 0.09 | 34.24 | 23 | 20 | 1373 | 5914 | 31265 | 51 | 314MB | 146901MB |
| HEAD | All | completions after edit | True | 50 | 1.90 | 0.00 | 12.24 | 25.71 | 3.16 | 0.09 | 37.96 | 23 | 20 | 1373 | 5914 | 31265 | 51 | 324MB | 164263MB |
| upstream | All | code actions | True | 50 | 1.98 | 0.00 | 4.62 | 0.02 | 3.07 | 0.02 | 4.65 | 5184 | 5183 | 5184 | 5965 | 31638 | 2 | 213MB | 23574MB |
| HEAD | All | code actions | True | 50 | 1.89 | 0.00 | 4.37 | 0.02 | 2.72 | 0.02 | 4.40 | 5176 | 5175 | 5176 | 5965 | 31638 | 2 | 209MB | 23598MB |
| upstream | All | code actions after edit | True | 50 | 1.97 | 0.00 | 33.59 | 0.02 | 3.10 | 0.31 | 33.62 | 19 | 17 | 1370 | 5965 | 31638 | 51 | 290MB | 144401MB |
| HEAD | All | code actions after edit | True | 50 | 2.00 | 0.00 | 38.98 | 0.03 | 3.01 | 0.37 | 39.02 | 18 | 16 | 1369 | 5965 | 31638 | 51 | 317MB | 164821MB |
| upstream | All | code actions after cradle edit | True | 50 | 2.13 | 0.00 | 210.18 | 0.08 | 5.25 | 2.09 | 210.27 | 1299 | 803 | 2538 | 5965 | 31393 | 100 | 506MB | 368322MB |
| HEAD | All | code actions after cradle edit | True | 50 | 1.86 | 0.00 | 159.57 | 0.17 | 4.52 | 1.58 | 159.74 | 1299 | 803 | 2538 | 5965 | 31393 | 100 | 505MB | 308065MB |
| upstream | All | documentSymbols after edit | True | 50 | 1.97 | 0.00 | 5.73 | 3.37 | 3.00 | 0.06 | 9.10 | 14 | 11 | 1362 | 5905 | 31249 | 3 | 233MB | 44703MB |
| HEAD | All | documentSymbols after edit | True | 50 | 1.87 | 0.00 | 6.00 | 2.88 | 2.78 | 0.06 | 8.89 | 13 | 11 | 1360 | 5905 | 31249 | 2 | 248MB | 44301MB |
| upstream | All | hole fit suggestions | True | 50 | 1.99 | 0.00 | 39.23 | 0.03 | 2.95 | 0.37 | 39.27 | 19 | 16 | 1369 | 5910 | 31261 | 51 | 325MB | 162837MB |
| HEAD | All | hole fit suggestions | True | 50 | 2.08 | 0.00 | 41.86 | 0.02 | 3.29 | 0.39 | 41.88 | 19 | 16 | 1369 | 5910 | 31261 | 51 | 327MB | 186180MB |
| | | | | | | | | | | | | | | | | | | | |
The result is generally better with large rule changes cases. But worse for no change rule visited.
Since we are waiting for the conditional rule to yield a result when refreshing the dependency. No change visit would have a degeneration.
But with rule change, the unneeded rule won't run, thus cancel out the performance degeneration.
Would love to hear you guys opinion on this @pepeiborra @wz1000
Otherthing still quit a mess, the importance commit at https://github.com/haskell/haskell-language-server/pull/4087/commits/8a088638a8be4daec622275c6bf6b5a4e35bcbea
Some good news, I have observed less failure in this branch than the master branch.
I don't think we need to complicate the API by introducing side conditions - we just need to try evaluating dependencies in order. This means instead of maintaining a KeySet of dependencies, we need to maintain something like a [KeySet]. When building a rule, we pop the first element of the [KeySet], and try building it in parallel - if that succeeds we proceed with the other elements, but if those fail we give up on rebuilding previous dependencies and kick off a fresh build.
I don't think we need to complicate the API by introducing side conditions - we just need to try evaluating dependencies in order. This means instead of maintaining a
KeySetof dependencies, we need to maintain something like a[KeySet]. When building a rule, we pop the first element of the[KeySet], and try building it in parallel - if that succeeds we proceed with the other elements, but if those fail we give up on rebuilding previous dependencies and kick off a fresh build.
On the above description, it is not clear to me how evaluating dependencies in order can avoid some unwanted Key(branch out) in the latter key set? We need to know which key we are branching on and kick a fresh rebuild on branch condition changed. We still need some explicit annotation?
rules can be evaluated in 2 modes - if the rule was previously run, then we build all of its dependencies from the previous run and then run (or skip) the rule according to its recomputation flags (is it always rerun etc..).
If the rule was not previously run, then we build it without building its previous dependencies (we don't have any!). The rule might request some dependencies to be built, and we build those on demand by suspending the evaluation of the current rule when this happens.
I'm proposing in case 1 (the rule was previously run), instead of building all dependencies immediately in parallel, we build them in order, and if any of them change, we skip building the subsequent dependencies and just build the rule itself, as in the second case.
Crucially, if an earlier dependency changes, we skip speculatively building later dependencies. This prevents us from getting into conditions where rules are built when their preconditions are not met because those preconditions should be captured by a change in an earlier dependency.
Oh i see how it goes, it would means kicking up deps linearly. It does solve the problem in a more direct way, but I am a bit worried on the performance impact on this linearity.
For a concrete example, consider the GetLinkable rule, which should only be executed when the NeedsCompilation rule returns a Just value. I think the bug is that in certain cases, initially NeedsCompilation returns a Just value, but then it changes to return Nothing. However, when we evaluate a rule for the second time, we see that it depended on NeedsCompilation and GetLinkable the first time, so we try to build both before proceeding with the rule. This is wrong! If NeedsCompilation changed, we do not want to build GetLinkable! That is the behaviour we need.
but I am a bit worried on the performance impact on this linearity.
Yes that was the concern, but we could still build all rules depended on simultaneously using the uses family of functions as well as the applicative combinators (<*>) in parallel. It's just that all the parallelism would have to be explicit in the rule definition.
If the performance impact is still too great, we could try doing "true" speculative builds, where we ignore errors and diagnostics from speculative builds of dependencies.
Let try a linear approach and do some benchmark to see how much we lost.
But I doubt the speculative builds would be better than annotating the dependencies with an explicit branching dependency tree.
I guess we could also just throw exceptions when preconditions are violated, and have these exceptions be caught and ignored by hls-graph.
Instead of
error $ "called GetLinkable for a file without a linkable: " ++ show f
throwIO $ PreconditionViolated $ "called GetLinkable for a file without a linkable: " ++ show f
I guess we could also just throw exceptions when preconditions are violated, and have these exceptions be caught and ignored by hls-graph.
Instead of
error $ "called GetLinkable for a file without a linkable: " ++ show fthrowIO $ PreconditionViolated $ "called GetLinkable for a file without a linkable: " ++ show f
This is indeed a good add on, and we can benefit from it also collecting the precondition violation statitstic in bench. But overall all I believe it is long term beneficial for us to improve hls-graph to a point that such violation can be avoided as much as possible when defining a rule but without sabotage the performance too much.
Yes that was the concern, but we could still build all rules depended on simultaneously using the uses family of functions as well as the applicative combinators (<*>) in parallel. It's just that all the parallelism would have to be explicit in the rule definition.
It sounds good.
linear deps in https://github.com/soulomoon/haskell-language-server/tree/linearity-deps. I am running some bench to see how it goes.
Good news @wz1000 looks good on linearity deps:
| version | configuration | name | success | samples | startup | setup | userT | delayedT | 1stBuildT | avgPerRespT | totalT | rulesBuilt | rulesChanged | rulesVisited | rulesTotal | ruleEdges | ghcRebuilds | maxResidency | allocatedBytes |
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| upstream | All | edit-header | True | 50 | 1.96 | 0.00 | 31.75 | 0.03 | 2.89 | 0.29 | 31.78 | 17 | 14 | 1367 | 5910 | 31261 | 51 | 297MB | 140843MB |
| HEAD | All | edit-header | True | 50 | 2.12 | 0.00 | 28.59 | 0.03 | 3.27 | 0.26 | 28.62 | 17 | 14 | 1367 | 5910 | 31261 | 51 | 233MB | 127732MB |
| upstream | All | edit | True | 50 | 1.95 | 0.00 | 30.02 | 0.06 | 2.97 | 0.28 | 30.08 | 17 | 14 | 1367 | 5910 | 31261 | 51 | 298MB | 131515MB |
| HEAD | All | edit | True | 50 | 2.10 | 0.00 | 24.37 | 2.88 | 2.92 | 0.25 | 27.26 | 17 | 14 | 1367 | 5910 | 31261 | 51 | 248MB | 125506MB |
| upstream | All | hover | True | 50 | 2.16 | 0.00 | 3.35 | 0.01 | 3.30 | 0.00 | 3.37 | 5122 | 5121 | 5122 | 5912 | 31267 | 2 | 212MB | 20368MB |
| HEAD | All | hover | True | 50 | 2.01 | 0.00 | 2.99 | 0.01 | 2.91 | 0.00 | 3.00 | 5125 | 5124 | 5125 | 5912 | 31267 | 2 | 129MB | 15482MB |
| upstream | All | semanticTokens | True | 50 | 1.98 | 0.00 | 67.75 | 2.63 | 3.41 | 0.66 | 70.39 | 12 | 10 | 1364 | 5914 | 31271 | 52 | 290MB | 293708MB |
| HEAD | All | semanticTokens | True | 50 | 2.06 | 0.00 | 56.48 | 0.98 | 3.64 | 0.54 | 57.47 | 12 | 10 | 1364 | 5914 | 31271 | 52 | 252MB | 257866MB |
| upstream | All | hover after edit | True | 50 | 1.95 | 0.00 | 3.25 | 29.37 | 2.87 | 0.00 | 32.63 | 21 | 18 | 1371 | 5912 | 31267 | 51 | 301MB | 144811MB |
| HEAD | All | hover after edit | False | 50 | 0.00 | 0.00 | 0.00 | 0.00 | 0.00 | 0.00 | 0.00 | 0 | 0 | 0 | 0 | 0 | 0 | 19MB | 1287MB |
| upstream | All | getDefinition | True | 50 | 1.93 | 0.00 | 2.88 | 0.01 | 2.75 | 0.00 | 2.89 | 5080 | 5079 | 5080 | 5918 | 31281 | 2 | 126MB | 15430MB |
| HEAD | All | getDefinition | True | 50 | 2.23 | 0.00 | 2.48 | 0.01 | 2.21 | 0.00 | 2.50 | 5116 | 5115 | 5116 | 5917 | 31275 | 2 | 210MB | 14893MB |
| upstream | All | getDefinition after edit | True | 50 | 2.19 | 0.00 | 6.45 | 44.89 | 5.47 | 0.01 | 51.35 | 19 | 16 | 1375 | 5918 | 31281 | 51 | 294MB | 157145MB |
| HEAD | All | getDefinition after edit | True | 50 | 1.93 | 0.00 | 3.48 | 20.31 | 3.02 | 0.00 | 23.79 | 19 | 16 | 1375 | 5918 | 31281 | 51 | 258MB | 110402MB |
| upstream | All | completions | True | 50 | 1.92 | 0.00 | 4.24 | 0.01 | 3.00 | 0.01 | 4.26 | 5129 | 5128 | 5129 | 5914 | 31265 | 2 | 224MB | 19468MB |
| HEAD | All | completions | True | 50 | 1.90 | 0.00 | 4.05 | 0.22 | 2.87 | 0.01 | 4.27 | 5094 | 5093 | 5094 | 5914 | 31265 | 2 | 242MB | 19314MB |
| upstream | All | completions after edit | True | 50 | 1.91 | 0.00 | 12.11 | 20.96 | 3.10 | 0.09 | 33.08 | 23 | 20 | 1373 | 5914 | 31265 | 51 | 301MB | 145745MB |
| HEAD | All | completions after edit | True | 50 | 2.04 | 0.00 | 11.99 | 14.85 | 3.12 | 0.09 | 26.84 | 23 | 20 | 1373 | 5914 | 31265 | 51 | 272MB | 124685MB |
| upstream | All | code actions | True | 50 | 1.92 | 0.00 | 4.44 | 0.02 | 2.88 | 0.02 | 4.47 | 5179 | 5178 | 5179 | 5965 | 31638 | 2 | 207MB | 23168MB |
| HEAD | All | code actions | True | 50 | 2.06 | 0.00 | 4.19 | 0.07 | 2.70 | 0.02 | 4.27 | 5178 | 5177 | 5178 | 5965 | 31638 | 2 | 212MB | 22953MB |
| upstream | All | code actions after edit | True | 50 | 2.23 | 0.00 | 34.51 | 0.02 | 3.25 | 0.32 | 34.54 | 19 | 17 | 1370 | 5965 | 31638 | 51 | 299MB | 148467MB |
| HEAD | All | code actions after edit | True | 50 | 1.98 | 0.00 | 28.74 | 0.02 | 3.06 | 0.26 | 28.76 | 21 | 18 | 1373 | 5965 | 31638 | 51 | 240MB | 134670MB |
| upstream | All | code actions after cradle edit | True | 50 | 2.19 | 0.00 | 212.70 | 0.03 | 4.82 | 2.12 | 212.74 | 1299 | 803 | 2537 | 5964 | 31387 | 100 | 527MB | 476889MB |
| HEAD | All | code actions after cradle edit | True | 50 | 2.09 | 0.00 | 224.64 | 0.03 | 4.76 | 2.24 | 224.68 | 1299 | 803 | 2537 | 5964 | 31387 | 100 | 439MB | 482834MB |
| upstream | All | documentSymbols after edit | True | 50 | 1.95 | 0.00 | 5.63 | 3.11 | 2.79 | 0.06 | 8.74 | 12 | 10 | 1362 | 5905 | 31249 | 3 | 227MB | 44218MB |
| HEAD | All | documentSymbols after edit | True | 50 | 2.03 | 0.00 | 8.62 | 3.09 | 2.87 | 0.09 | 11.72 | 17 | 14 | 1349 | 5905 | 31249 | 3 | 384MB | 56790MB |
| upstream | All | hole fit suggestions | True | 50 | 2.09 | 0.00 | 38.85 | 0.02 | 3.07 | 0.37 | 38.87 | 19 | 16 | 1369 | 5910 | 31261 | 51 | 323MB | 166320MB |
| HEAD | All | hole fit suggestions | True | 50 | 1.97 | 0.00 | 32.92 | 0.02 | 2.94 | 0.31 | 32.95 | 19 | 16 | 1369 | 5910 | 31261 | 51 | 262MB | 147606MB |
Another run the linearity approach, I guess stopping the extra dep does helps a great deal.
| version | configuration | name | success | samples | startup | setup | userT | delayedT | 1stBuildT | avgPerRespT | totalT | rulesBuilt | rulesChanged | rulesVisited | rulesTotal | ruleEdges | ghcRebuilds | maxResidency | allocatedBytes |
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| upstream | All | edit-header | False | 50 | 0.00 | 0.00 | 0.00 | 0.00 | 0.00 | 0.00 | 0.00 | 0 | 0 | 0 | 0 | 0 | 0 | 19MB | 1287MB |
| HEAD | All | edit-header | True | 50 | 1.98 | 0.00 | 26.23 | 0.05 | 2.89 | 0.24 | 26.29 | 17 | 14 | 1367 | 5910 | 31261 | 51 | 241MB | 124011MB |
| upstream | All | edit | True | 50 | 1.98 | 0.00 | 31.74 | 0.07 | 3.01 | 0.29 | 31.81 | 17 | 14 | 1367 | 5910 | 31261 | 51 | 304MB | 140039MB |
| HEAD | All | edit | True | 50 | 1.98 | 0.00 | 26.50 | 0.02 | 2.86 | 0.24 | 26.53 | 17 | 14 | 1367 | 5910 | 31261 | 51 | 234MB | 125801MB |
| upstream | All | hover | True | 50 | 2.07 | 0.00 | 2.81 | 0.01 | 2.72 | 0.00 | 2.82 | 5125 | 5124 | 5125 | 5912 | 31267 | 2 | 148MB | 15478MB |
| HEAD | All | hover | True | 50 | 2.10 | 0.00 | 2.79 | 0.01 | 2.74 | 0.00 | 2.80 | 5090 | 5089 | 5090 | 5912 | 31267 | 2 | 118MB | 15530MB |
| upstream | All | semanticTokens | True | 50 | 1.97 | 0.00 | 67.96 | 0.68 | 3.63 | 0.66 | 68.64 | 12 | 10 | 1364 | 5914 | 31271 | 52 | 302MB | 291938MB |
| HEAD | All | semanticTokens | True | 50 | 2.06 | 0.00 | 54.54 | 1.08 | 3.37 | 0.52 | 55.62 | 12 | 10 | 1364 | 5914 | 31271 | 52 | 250MB | 255018MB |
| upstream | All | hover after edit | True | 50 | 1.99 | 0.00 | 3.35 | 30.41 | 2.94 | 0.00 | 33.77 | 21 | 18 | 1371 | 5912 | 31267 | 51 | 312MB | 145779MB |
| HEAD | All | hover after edit | True | 50 | 1.98 | 0.00 | 3.10 | 21.60 | 2.91 | 0.00 | 24.71 | 21 | 18 | 1371 | 5912 | 31267 | 51 | 253MB | 122298MB |
| upstream | All | getDefinition | True | 50 | 1.90 | 0.00 | 2.84 | 0.01 | 2.70 | 0.00 | 2.85 | 5132 | 5131 | 5132 | 5918 | 31281 | 2 | 122MB | 15385MB |
| HEAD | All | getDefinition | True | 50 | 2.08 | 0.00 | 2.87 | 0.01 | 2.61 | 0.00 | 2.88 | 5055 | 5054 | 5055 | 5917 | 31281 | 2 | 214MB | 15510MB |
| upstream | All | getDefinition after edit | True | 50 | 1.96 | 0.00 | 3.69 | 26.74 | 2.74 | 0.01 | 30.44 | 19 | 16 | 1375 | 5918 | 31281 | 51 | 287MB | 132671MB |
| HEAD | All | getDefinition after edit | True | 50 | 1.97 | 0.00 | 3.47 | 20.49 | 2.96 | 0.01 | 23.97 | 19 | 16 | 1375 | 5918 | 31281 | 51 | 252MB | 112141MB |
| upstream | All | completions | True | 50 | 1.93 | 0.00 | 4.29 | 0.01 | 3.07 | 0.01 | 4.30 | 5125 | 5124 | 5125 | 5914 | 31265 | 2 | 226MB | 19450MB |
| HEAD | All | completions | True | 50 | 1.93 | 0.00 | 4.19 | 0.01 | 2.94 | 0.01 | 4.20 | 5127 | 5126 | 5127 | 5914 | 31265 | 2 | 223MB | 19448MB |
| upstream | All | completions after edit | True | 50 | 2.04 | 0.00 | 11.26 | 21.49 | 3.03 | 0.08 | 32.75 | 23 | 20 | 1373 | 5914 | 31265 | 51 | 327MB | 144324MB |
| HEAD | All | completions after edit | True | 50 | 2.07 | 0.00 | 11.54 | 14.58 | 3.18 | 0.09 | 26.13 | 23 | 20 | 1373 | 5914 | 31265 | 51 | 271MB | 125718MB |
| upstream | All | code actions | True | 50 | 2.04 | 0.00 | 4.46 | 0.02 | 2.84 | 0.02 | 4.49 | 5178 | 5177 | 5178 | 5965 | 31638 | 2 | 211MB | 23204MB |
| HEAD | All | code actions | True | 50 | 1.93 | 0.00 | 4.45 | 0.03 | 2.85 | 0.02 | 4.48 | 5127 | 5126 | 5127 | 5965 | 31638 | 2 | 211MB | 23083MB |
| upstream | All | code actions after edit | True | 50 | 2.01 | 0.00 | 34.50 | 0.03 | 2.92 | 0.32 | 34.53 | 19 | 17 | 1370 | 5965 | 31638 | 51 | 300MB | 145813MB |
| HEAD | All | code actions after edit | True | 50 | 2.05 | 0.00 | 27.99 | 0.02 | 3.02 | 0.25 | 28.01 | 21 | 18 | 1373 | 5965 | 31638 | 51 | 236MB | 130768MB |
| upstream | All | code actions after cradle edit | True | 50 | 2.15 | 0.00 | 164.78 | 0.02 | 4.99 | 1.63 | 164.80 | 1299 | 803 | 2538 | 5965 | 31393 | 100 | 511MB | 311292MB |
| HEAD | All | code actions after cradle edit | True | 50 | 2.02 | 0.00 | 164.01 | 0.03 | 4.76 | 1.63 | 164.04 | 1299 | 803 | 2538 | 5965 | 31393 | 100 | 440MB | 304506MB |
| upstream | All | documentSymbols after edit | True | 50 | 2.12 | 0.00 | 6.15 | 3.40 | 3.18 | 0.06 | 9.55 | 14 | 11 | 1362 | 5905 | 31249 | 3 | 239MB | 44436MB |
| HEAD | All | documentSymbols after edit | True | 50 | 1.95 | 0.00 | 8.19 | 3.32 | 2.93 | 0.08 | 11.51 | 17 | 14 | 1357 | 5905 | 31249 | 3 | 391MB | 55890MB |
| upstream | All | hole fit suggestions | True | 50 | 2.26 | 0.00 | 39.09 | 0.03 | 2.95 | 0.37 | 39.12 | 19 | 16 | 1369 | 5910 | 31261 | 51 | 316MB | 161675MB |
| HEAD | All | hole fit suggestions | True | 50 | 2.10 | 0.00 | 32.13 | 0.29 | 3.09 | 0.30 | 32.43 | 19 | 16 | 1369 | 5910 | 31261 | 51 | 259MB | 146484MB |
Could you also change the following in typecheckRule to throw an error?
when (foi == NotFOI) $
logWith recorder Logger.Warning $ LogTypecheckedFOI file
Earlier it was very easy to trigger this warning/error, you just opened a file in vscode, waited for it to typecheck and then closed it.
If this patch avoids that we can have more confidence that it is working as intended.
If this patch avoids that we can have more confidence that it is working as intended.
ok, let's see if we trigger error here
Earlier it was very easy to trigger this warning/error, you just opened a file in vscode, waited for it to typecheck and then closed it.
🎆 Not to my eyes now in the editor.
seems we are seeing some failure in ghcide, 5/6 out of 367 tests failed (495.99s)
Imported symbol: FAIL (0.83s)
test/exe/TestUtils.hs:293:
Expecting exactly one definition, got 0
Use -p '/ghcide.get.definition.Imported/&&$0=="ghcide.get.definition.Imported symbol"' to rerun this test only.
Imported symbol (reexported): FAIL (0.79s)
test/exe/TestUtils.hs:293:
Expecting exactly one definition, got 0
Use -p '/ghcide.get.definition.Imported/&&/Imported symbol (reexported)/' to rerun this test only.
It won't fail before we change to throw error on non-FOI typecheck.
It seems our test did not take care of it. can pass if ignoring it in the test.
Just ShakeExtras{ideTesting=IdeTesting b} <- getShakeExtra
when (foi == NotFOI && not b) $
it is orthogonal to this pr, we should probably do a fix on the test and add the error throw behaviour in the following up seperate pr.
Also, I have added a test case to prevent phantom dependencies from occuring again in hls-graph.
These eval plugin tests https://github.com/haskell/haskell-language-server/actions/runs/8018144432/job/21903462404?pr=4087 usually fail due to the GetLinkable rule failure, so not sure how much this improves the issue. I'll go over this PR tomorrow and try to see how much it improves the failure rate:
In the end I got the (improved) error after 180 iterations of running that test. But it seems to occur less frequently now. "message": "called GetLinkable for a file without a linkable: NormalizedFilePath "/tmp/extra-dir-10246150986263/TI_Info.hs"\nCallStack (from HasCallStack):\n error, called at src/Development/IDE/Core/Rules.hs:1120:18 in ghcide-2.6.0.0-inplace:Development.IDE.Core.Rules"
I guess we can also try to see if this error still happened ? @jhrcek
I would like to test this out locally before we merge, as there are a lot of tricky properties we need to maintain.
I'll try to do this in the next couple of days,
thanx, take your time.
@wz1000 Is there still some test you want to do locally ? If not, I am considering resolve the conflict and merge this week.