Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Weird behavior when chaining tasks #257

Open
Tylerwbrown opened this issue Feb 6, 2023 · 4 comments
Open

Weird behavior when chaining tasks #257

Tylerwbrown opened this issue Feb 6, 2023 · 4 comments
Labels
bug Something isn't working correctly

Comments

@Tylerwbrown
Copy link

Tylerwbrown commented Feb 6, 2023

I have a more complex app, but I tried to write the simplest app to reproduce the issue:

{-# LANGUAGE OverloadedStrings #-}
{-# LANGUAGE LambdaCase #-}

import Monomer
import Data.Functor (($>))
import System.Sleep (sleep)

data Ev = Inc | Dec | Exec deriving Eq

build _ count = vstack [ hstack [ labelS count, spacer, button "Increment" Inc ] ]

handle _ _ count = \case
  Inc  -> [Model $ count + 1, Task $ sleep 1 $> Exec]
  Exec -> [Task $ sleep 1 $> Dec]
  Dec  -> [Model $ count - 1]
  
main = startApp (0 :: Int) handle build [ appTheme darkTheme, appFontDef "Regular" "./Roboto-Regular.ttf" ]

this is on monomer 1.5.0.0 and GHC 9.2.5.

If you spam the increment button, the GUI will get choppy after the sleeps finish up, and count will eventually be negative, going further down depending on how much you spammed it, when really it should never go below 0.

Some workarounds:

  • Use Control.Concurrent (threadDelay) instead of System.Sleep (sleep) (this sleep seems to block threads which is probably why the choppiness occurs, as that doesn't occur with threadDelay).
  • Don't have an intermediate event Exec, just sleep for 2 seconds and immediately call Dec in the first task instead of Exec.

I'd be totally happy just using threadDelay, I'm just worried if I'm doing some other IO operation that blocks in a similar way to System.Sleep (sleep) that this issue will arise again.

@Deltaspace0
Copy link
Contributor

Deltaspace0 commented Feb 6, 2023

I think the problem with sleep is not that it blocks the thread, but that it is implemented by spinning (busy-waiting) - it calls itself recursively and checks the time on each call, i.e. just wastes the CPU. It is very inefficient and I believe that threadDelay should be always preferred. I don't think there is some other IO operation that is so wasteful.

@fjvallarino
Copy link
Owner

Hi @Tylerwbrown!

What @Deltaspace0 explains sounds reasonable. I think the sleep function somehow affects the multi-threaded runtime, and using threadDelay sounds like the most reasonable option.

I'm not sure in which package System.Sleep (sleep) can be found. I tried in base and extra, but it either doesn't exist in the package or is not in the System.Sleep module. If you could point me to the package you're getting that function from, I can give it a try and see if something can be improved on the library's side.

Thanks!

@fjvallarino fjvallarino added the question Further information is requested label Feb 7, 2023
@Tylerwbrown
Copy link
Author

@fjvallarino hey, thanks for taking a look, the packag I'm using is this one https://hackage.haskell.org/package/sleep-0.1.0.1/docs/System-Sleep.html

@fjvallarino
Copy link
Owner

@Tylerwbrown it seems I will need to revamp how the Task/Producer lifecycle is managed. The way the status of a given task or producer is checked is a bit wasteful, and I think the use of poll may be related to the issue you found.

Currently, when a Task or Producer is created, an async instance is launched. The option I will test in the next couple of weeks (I'm a bit tight on time right now) is, instead of using poll, to have the user action wrapped in library code that takes care of marking the task/producer as done as soon as it returns (most likely using a channel to be consumed in the WidgetTask module).

Funny enough, if instead of a Task you use a Producer, the issue of the negative number does not seem to happen (although the UI gets quite laggy once you spam-click the button). This is why I suspect about how I use poll: the return value of a Task is retrieved with it, but in the case of a Producer a channel is used.

{-# LANGUAGE OverloadedStrings #-}
{-# LANGUAGE LambdaCase #-}
{-# LANGUAGE BangPatterns #-}

import Monomer
import Data.Functor (($>))
import System.Sleep (sleep)

data Ev = Inc | Dec | Exec deriving Eq

build _ count = vstack [ hstack [ labelS count, spacer, button "Increment" Inc ] ]

handle _ _ !count = \case
  Inc  -> [Model $ count + 1, Producer $ \sendMsg -> do
    sleep 1
    sendMsg Exec]
  Exec -> [Producer $ \sendMsg -> do
    sleep 1
    sendMsg Dec]
  Dec  -> [Model $ count - 1]

main = startApp (0 :: Int) handle build [ appTheme darkTheme, appFontDef "Regular" "./Roboto-Regular.ttf" ]

@fjvallarino fjvallarino added bug Something isn't working correctly and removed question Further information is requested labels Feb 11, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working correctly
Projects
None yet
Development

No branches or pull requests

3 participants