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

Local state is not safe #6

Open
noughtmare opened this issue May 5, 2021 · 3 comments
Open

Local state is not safe #6

noughtmare opened this issue May 5, 2021 · 3 comments

Comments

@noughtmare
Copy link

noughtmare commented May 5, 2021

E.g. the program:

import Control.Ev.Eff

main :: IO ()
main = print $ runEff $ local 0 $ do
  localModify (+ 1)
  localGet

Prints (when compiled with -O):

0

I think that this is due to common sub-expression elimination. First the code above gets turned into something like:

import Control.Ev.Eff

main :: IO ()
main =
  let !r = unsafePerformIO (newIORef 0)
      !x1 = unsafePerformIO (readIORef r)
      !() = unsafePerformIO (writeIORef r (x1 + 1)
      !x2 = unsafePerformIO (readIORef r)
  in pure x2

And common sub-expression elimination transforms that to:

import Control.Ev.Eff

main :: IO ()
main =
  let !r = unsafePerformIO (newIORef 0)
      !x1andx2 = unsafePerformIO (readIORef r)
      !() = unsafePerformIO (writeIORef r (x1andx2 + 1)
  in pure x1andx2

A simple solution seems to be to mark perform NOINLINE, but that probably affects performance.

@noughtmare
Copy link
Author

noughtmare commented May 5, 2021

I have a better solution, we can rewrite Ctl into a monad transformer CtlM which can run IO actions properly. Then you only need one unsafePerformIO at the top level in runCtl.

I have been able to make it work, but I have to clean up my code a bit. Expect a pull request soon.

Update: the change significantly affects some benchmarks, so I am investigating further. But maybe the microbenchmarks are also not representative of real-world scenarios w.r.t. inlining and specialization as Alexis King's talk "Effects for Less" showed.

@mmhat
Copy link

mmhat commented Jul 1, 2021

@noughtmare I tried to reproduce this with the current HEAD and it works as intended. Can your confirm that?

@noughtmare
Copy link
Author

If I compile with optimizations then I can still reproduce it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants