IntersectMBO / IntersectMBO/ouroboros-consensus

Strange space leak in PBFT.ChainState

Open
#741 1 comment 0 reactions 0 assignees View on GitHub
:wastebasket: :question: possibly stale 🏎️ performance technical debt
Dominant language
Haskell
Stars
67
Forks
43
Avg merge
5d 13h
Merged PRs (30d)
43

Description

It has been reported tthat https://github.com/input-output-hk/ouroboros-network/commit/f2a050ba9ada3bf3ee2421f5e610947619d28337 introduced a space leak. If we remove the following line, the space leak is gone: https://github.com/input-output-hk/ouroboros-network/blob/f2a050ba9ada3bf3ee2421f5e610947619d28337/ouroboros-consensus/src/Ouroboros/Consensus/Protocol/PBFT/ChainState.hs#L356
Note that the space leak has _nothing_ to do with EBBs themselves! Even with an empty `ebbs`, we still have the space leak.

Looking at the implementation of `pruneEBBsLT`: https://github.com/input-output-hk/ouroboros-network/blob/f2a050ba9ada3bf3ee2421f5e610947619d28337/ouroboros-consensus/src/Ouroboros/Consensus/Protocol/PBFT/ChainState.hs#L624
No matter how I try to force `anchorSlot cs` (bangs, `seq`, pass it in as an argument, ...), it causes a thunk in the `StrictSeq` fields. Even if I pattern match on it, ignore it (!) and do `Map.map id` instead of filtering. Strangely, `cs { ebbs = ebbs }` doesn't have the leak.

If we add `{-# OPTIONS_GHC -O0 #-}` to the module, the leak goes away. The leak is there with `-O1` and `-O2` (`-fno-full-laziness` doesn't help). This makes me think the leak is caused by a bug in GHC :scream:.

You can use the following snippet to quickly detect the leak:
```haskell
main :: IO ()
main = do
varCS <- Strict.newTVarWithInvariantM unsafeNoThunks CS.empty
let go slot@(SlotNo s) = do
when (s `mod` 1000 == 0) $ print s
atomically $ modifyTVar varCS $
CS.append securityParam windowSize (signerForSlot slot)
go (succ slot)
go 0
where
securityParam = SecurityParam 2
windowSize = CS.WindowSize 2
signerForSlot :: SlotNo -> CS.PBftSigner PBftMockCrypto
signerForSlot slot@(SlotNo s) = CS.PBftSigner
{ pbftSignerSlotNo = slot
, pbftSignerGenesisKey = Crypto.VerKeyMockDSIGN (fromIntegral s `mod` 7)
}
```
The thunk detection (`unsafeNoThunks`) will catch the thunk. Profiling the heap confirms that thunk detection is correct.

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.