Use stm-stats to reduce contention in hls-graph

[pepeiborra]

This PR is another extract from #2357. It adds a new flag stm-stats to hls-graph that enables collection of statistics on STM transactions, based on the stm-stats Hackage package. Then the STM stats are used to minimise the contention of atomically blocks in hls-graph.

STM stats are not yet reported in the benchmark suite, so we need to run the
benchmarks manually in verbose mode to observe them (and build hls-graph with the
stm-stats Cabal flag):

cabal build exe:ghcide ghcide-bench && cabal exec cabal run ghcide-bench -- -- -s "edit" --samples 10 --no-clean --example-module Distribution/Simple.hs --example-module Distribution/Types/Module.hs  -v

The before results show lots of contention in builder and esp. updateReverseDeps.
incDatabase should not have any retries, could be a bug.

Before

Cabal example

STM transaction statistics (2021-11-30 16:51:48.260905 UTC):
Transaction               Commits    Retries      Ratio
_anonymous_                     5          0       0.00
builder                     80886       6000       0.07
compute                     19175        141       0.01
incDatabase                    27        100       3.70
updateReverseDeps            3827        765       0.20

Sigma example

STM transaction statistics (2021-11-30 19:45:30.904126927 UTC):
Transaction               Commits    Retries      Ratio
_anonymous_                     1          0       0.00
builder                    606254      22569       0.04
compute                    324708      10428       0.03
incDatabase                    15          0       0.00
updateReverseDeps          259755     489285       1.88

After

Cabal example

STM transaction statistics (2021-11-30 20:21:57.968789 UTC):
Transaction               Commits    Retries      Ratio
_anonymous_                 48318          2       0.00
builder                   1108126       1276       0.00
compute                     22423        144       0.01
updateReverseDeps           65225        377       0.01

Sigma example

STM transaction statistics (2021-11-30 19:57:27.979412261 UTC):
Transaction               Commits    Retries      Ratio
_anonymous_                   294          0       0.00
builder                    861431       2914       0.00
compute                    324708       1573       0.00
updateReverseDeps          858100      17816       0.02

[jneira]

great work, number looks really good, what would be the gain in time?

[michaelpj]

I guess you learned something that led to you making the changes that improved contention. Could you write some notes in the code about it? Also helps future people not to accidentally pessimise it again...

[pepeiborra]

great work, number looks really good, what would be the gain in time?

Benchmark results are available in the master PR (#2357). I didn't collect results for the individual changes.

[pepeiborra]

I guess you learned something that led to you making the changes that improved contention. Could you write some notes in the code about it? Also helps future people not to accidentally pessimise it again...

What I learnt: which transactions needed optimising.

The actual fix was pretty trivial (7f7e470), just breaking down big transactions in smaller ones, generally by moving atomically inside the loop.

To prevent accidental pessimisation we probably need to extend the benchmark suite to collect the STM stats and flag any regressions.

[michaelpj]

I'm guessing you thought about this but just checking: I'm assuming that in the places where you broke up transactions we don't rely on all that happening in a single transaction, i.e. we won't end up in a weird state if only some of them go through?

[pepeiborra]

I'm guessing you thought about this but just checking: I'm assuming that in the places where you broke up transactions we don't rely on all that happening in a single transaction, i.e. we won't end up in a weird state if only some of them go through?

That's the bit that requires domain knowledge. There's two interesting transactions:

1. Updating reverse deps of a build target: there is no need to update them all atomically so we can break the transaction.
2. Enqueueing the direct deps of a build target: same here, there is no need to enqueue them all atomically.