Gabriella439 / pipes-concurrency

Concurrency for the pipes ecosystem
BSD 3-Clause "New" or "Revised" License
43 stars 12 forks source link

feature: added a new buffer type 'LatestOnce' #19

Closed urv closed 10 years ago

urv commented 10 years ago

LatestOnce: Like Latest but the 'Latest' message can be read out of the mailbox just once. Any other read attempt will block until new messages arrive - writers should not block.

Maybe a stupid buffer type but most probably a stupid name for the buffer.

Gabriella439 commented 10 years ago

Sorry for the delay. I didn't have internet the last few days.

It will help if you give me some idea for what use case you had in mind for this buffer. If it's a useful case that can't be solved with other buffer types then I will probably add it (and perhaps change the name like you suggested).

urv commented 10 years ago

Hi Gabriel, no problem and thanks a lot for your help! We have N readers that should be notified whenever new updates are available and otherwise wait&sleep but they should react to the latest updates as soon as possible. In our case the updates are just changes of the top of the order book of a bitcoin exchange. We also have M writers that get updates (ANY change of the order book) from an external source, the bitcoin exchange, and notify the readers whenever the top of the order book changes. The writers shouldn't sleep but just put the update in the mailbox and continue their busy work. Currently N == 1 and M == 2. Cheers Arvin

Gabriella439 commented 10 years ago

Okay, so my understanding of the code is that this buffer basically enforces that there is always at least one write between every two reads. The motivation behind this is so that reads can sleep waiting for new changes while allowing values to be discarded if writes outpace reads. I can definitely see how this is useful so I will accept this feature, but I would like to ask if you could make some small modifications.

First, this could be implemented without providing an initial value. In fact, it could probably be backed by a single TMVar that is initially empty, and then define the read/write actions as follows:

case buffer of
    ...
    New -> do
        m <- S.newEmptyTMVarIO
        return (S.takeTMVar m, S.tryTakeTMVar m *> S.putTMVar m x)

In other words, it's basically identical to the Single buffer, except that you prefix putTMVar with a tryTakeTMVar to empty the TMVar if it is full.

As the above code suggests, I also recommend changing the name of the buffer to New to be more suggestive of its purpose.

urv commented 10 years ago

Nice one! We were also thinking that providing an initial value is somehow fishy. It makes our code even cleaner und simpler now. Thanks a lot! I will apply all of your suggestions and submit another pull request. What about the documentation and test cases? Should those also be adapted?

Gabriella439 commented 10 years ago

Yeah, adapt the test cases if you have time. I can take care of documenting it and mentioning it in the tutorial.

urv commented 10 years ago

Here we go. It looks definitely better now! I implemented all of your suggestions. Cheers and thanks!

Gabriella439 commented 10 years ago

This looks great! Can you either merge or rebase against master so that I can merge this in?

urv commented 10 years ago

Yes of course. I just rebased it on top of your master branch.

Gabriella439 commented 10 years ago

Beautiful! Thanks a lot for contributing this feature! :)

Gabriella439 commented 10 years ago

So I just wanted to mention that I also added a Newest mailbox in commit 3eeb535dd55616f879c557f19bed811aef5465e0. This generalizes the New mailbox in the same way that Bounded generalizes Single. I'm still keeping New as the optimized case for one message.

urv commented 10 years ago

This is a great idea and just beautiful! :) I am impressed over and over again how expressive Haskell modules can be just with the right abstraction! Thanks a lot for your efforts!

Gabriella439 commented 10 years ago

You're welcome! :)