PLT-380 Replace TChan with bounded queue

[andreabedini]

Someone made me notice that I was using an unbounded tchan where I could get away with a mvar. Basically I want to have the two threads (the one running chain-sync and the stream consumer) in lock-step.

In the current version, blocks are pushed in the channel at full speed independently from the speed at which they are consumed, this can cause memory issues if the consumer is slower than the network (which is likely).

[sjoerdvisscher]

I'm completely inexperienced when it comes to concurrency programming (in Haskell or any other language), but this all sounds very reasonable. And it is simpler, and simple is good 👍

[ghost]

You may use Control.Concurrent.STM.TBQueue to accumulate the items with a limited size queue to control the memory usage. That might have a bigger throughput than using a single MVar.

UPD. For blocks with transactions we have Control.Concurrent.STM.TBMQueue which allows to use a measure function to count the number of transactions in blocks instead of the number blocks. The example of usage might be found in chain index.

[koslambrou]

I second @ak3n. The TBMQueue will solve the memory issues.

[andreabedini]

You may use Control.Concurrent.STM.TBQueue to accumulate the items with a limited size queue to control the memory usage. That might have a bigger throughput than using a single MVar.

You are right that throughput is important (which reminds me I should implement pipelining as well).

UPD. For blocks with transactions we have Control.Concurrent.STM.TBMQueue which allows to use a measure function to count the number of transactions in blocks instead of the number blocks. The example of usage might be found in chain index.

Control.Concurrent.STM.TBMQueue looks nice and the ability of bounding the channel size in terms of transactions rather than blocks is interesting but it's in plutus-chain-index and plutus-streaming cannot depend on that. Maybe worth extracting it out at some point?

I'll change this PR to use Control.Concurrent.STM.TBQueue

[ghost]

Control.Concurrent.STM.TBQueue is unreliable in terms of memory usage in this case because the number of transactions in the blocks could take much more memory with a fixed number of blocks queue. We experienced that with chain index.

Maybe worth extracting it out at some point?

I have extracted it to a separate personal repo https://git.sr.ht/~ak3n/tbmqueue some time ago where I wanted to play with it and maybe tune but had no time and haven't uploaded to hackage (also it seems I messed up with the LICENSE file).

We can start using it which will make me a maintainer and maybe I will extend it. Or we can extract the module to a new package within the plutus-apps repo. Or just copy the module until the dependencies picture will be clearer.

[andreabedini]

Control.Concurrent.STM.TBQueue is unreliable in terms of memory usage in this case because the number of transactions in the blocks could take much more memory with a fixed number of blocks queue. We experienced that with chain index.

Do you happen to remember how big was the variation? I am a bit conflicted. I don't even have evicence of lower throughput with an MVar. I will run some tests tomorrow to decide what to do.

[andreabedini]

@ak3n @koslambrou I appreciate your feedback but I have decided to stick with a dead simple MVar. I added a comment explaining the thread off. I am concerned about spending time optimising for the wrong thing. We can revise this later if it becomes a problem.

[koslambrou]

@ak3n @koslambrou I appreciate your feedback but I have decided to stick with a dead simple MVar. I added a comment explaining the thread off. I am concerned about spending time optimising for the wrong thing. We can revise this later if it becomes a problem.

No problem, we can add a story for this. I may be wrong, but feels like using something like TBMQueue will indeed improve performance (it did improve the performance of plutus-chain-index, and the current use case is very similar. I might be wrong.

[andreabedini]

I may be wrong, but feels like using something like TBMQueue will indeed improve performance (it did improve the performance of plutus-chain-index, and the current use case is very similar. I might be wrong.

I don't mean to say you are wrong! I saw the issue on JIRA 👍