Restore rparWith r0 documented behaviour

[Borgvall]

In addition I have added a test case, to prevent this regression in the future.

[treeowl]

This is exactly the same without optimization, but broken with optimization.

[Borgvall]

No it's not the order of the output of the test program changes, depending on the implementation of rparWith. See the following bash output on my Fedora 27 workstation with GHC 8.0.2.

[johannes@fedorawork parallel]$ cp test\rparWith_r0.hs .
[johannes@fedorawork parallel]$ ghc -O0 -threaded -fforce-recomp rparWith_r0.hs  
[1 of 4] Compiling Control.Parallel ( Control/Parallel.hs, Control/Parallel.o )
[2 of 4] Compiling Control.Seq      ( Control/Seq.hs, Control/Seq.o )
[3 of 4] Compiling Control.Parallel.Strategies ( Control/Parallel/Strategies.hs, Control/Parallel/Strategies.o )
[4 of 4] Compiling Main             ( rparWith_r0.hs, rparWith_r0.o )
Linking rparWith_r0 ...
[johannes@fedorawork parallel]$ ./rparWith_r0 
Must be printed first.
Must be printed at the end.
[johannes@fedorawork parallel]$ git revert fdce43b84
[restore-rparWith-r0 763661f] Revert "Restore the documented behaviour of `rparWith r0`"
 1 file changed, 1 insertion(+), 8 deletions(-)
[johannes@fedorawork parallel]$ ghc -O0 -threaded -fforce-recomp rparWith_r0.hs 
[1 of 4] Compiling Control.Parallel ( Control/Parallel.hs, Control/Parallel.o )
[2 of 4] Compiling Control.Seq      ( Control/Seq.hs, Control/Seq.o )
[3 of 4] Compiling Control.Parallel.Strategies ( Control/Parallel/Strategies.hs, Control/Parallel/Strategies.o )
[4 of 4] Compiling Main             ( rparWith_r0.hs, rparWith_r0.o )
Linking rparWith_r0 ...
[johannes@fedorawork parallel]$ ./rparWith_r0 
Must be printed at the end.
Must be printed first.
[johannes@fedorawork parallel]$ ghc -O2 -threaded -fforce-recomp rparWith_r0.hs  
[1 of 4] Compiling Control.Parallel ( Control/Parallel.hs, Control/Parallel.o )
[2 of 4] Compiling Control.Seq      ( Control/Seq.hs, Control/Seq.o )
[3 of 4] Compiling Control.Parallel.Strategies ( Control/Parallel/Strategies.hs, Control/Parallel/Strategies.o )
[4 of 4] Compiling Main             ( rparWith_r0.hs, rparWith_r0.o )
Linking rparWith_r0 ...
[johannes@fedorawork parallel]$ ./rparWith_r0 
Must be printed at the end.
Must be printed first.
[johannes@fedorawork parallel]$ git checkout v3.2.1.0
Hinweis: Checke 'v3.2.1.0' aus.

Sie befinden sich im Zustand eines 'lösgelösten HEAD'. Sie können sich
umschauen, experimentelle Änderungen vornehmen und diese committen, und
Sie können alle möglichen Commits, die Sie in diesem Zustand machen,
ohne Auswirkungen auf irgendeinen Branch verwerfen, indem Sie einen
weiteren Checkout durchführen.

Wenn Sie einen neuen Branch erstellen möchten, um Ihre erstellten Commits
zu behalten, können Sie das (jetzt oder später) durch einen weiteren Checkout
mit der Option -b tun. Beispiel:

  git checkout -b <neuer-Branchname>

HEAD ist jetzt bei dbb8d9e... Fix minor typo in .cabal file
[johannes@fedorawork parallel]$ ghc -O2 -threaded -fforce-recomp rparWith_r0.hs 
[1 of 4] Compiling Control.Parallel ( Control/Parallel.hs, Control/Parallel.o )
[2 of 4] Compiling Control.Seq      ( Control/Seq.hs, Control/Seq.o )
[3 of 4] Compiling Control.Parallel.Strategies ( Control/Parallel/Strategies.hs, Control/Parallel/Strategies.o )
[4 of 4] Compiling Main             ( rparWith_r0.hs, rparWith_r0.o )
Linking rparWith_r0 ...
[johannes@fedorawork parallel]$ ./rparWith_r0 
Must be printed first.
Must be printed at the end.
[johannes@fedorawork parallel]$ ghc -O0 -threaded -fforce-recomp rparWith_r0.hs  
[1 of 4] Compiling Control.Parallel ( Control/Parallel.hs, Control/Parallel.o )
[2 of 4] Compiling Control.Seq      ( Control/Seq.hs, Control/Seq.o )
[3 of 4] Compiling Control.Parallel.Strategies ( Control/Parallel/Strategies.hs, Control/Parallel/Strategies.o )
[4 of 4] Compiling Main             ( rparWith_r0.hs, rparWith_r0.o )
Linking rparWith_r0 ...
[johannes@fedorawork parallel]$ ./rparWith_r0 
Must be printed first.
Must be printed at the end.

[treeowl]

Instead of reverting 9ea4c07, define rparWith like this:

rparWith s a = do
  l <- rpar r
  return (case l of Lift x -> x)

  where
    r = runEval (Lift <$> s a)

data Lift a = Lift a

That's actually the same as before, but doesn't have the realWorld# invocation that led to #17.

[treeowl]

The documentation for rparWith also really needs to be clarified to explain how rparWith s is different from rpar `dot` s. The business about exiting the Eval monad is also fairly bogus and should probably be removed.

[simonmar]

@Borgvall thanks for spotting the problem and for the test case!

Yes we also need to fix the documentation. Personally I'd also like to have a better understanding of where the original implementation went wrong - @treeowl is there a succinct way to describe the problem that we could put in a comment?

It would be nice if parBuffer worked the same way as rparWith, it seems strange to me that parBuffer r0 should be the same as parBuffer rseq.

[treeowl]

If we adopt my proposed evalBuffer = buffering, then parBuffer n s = evalBuffer n (rparWith s) should do the trick.

[simonmar]

We still need to get this fixed. @Borgvall would you be able to update the PR with @treeowl's suggestion to use runEval?

@treeowl is there any way to add a test so that the bad behaviour with realWorld# doesn't reappear?

[treeowl]

@simonmar, I opened #36 to replace @Borgvall's second commit with one that uses runEval and fixes the documentation.

[Borgvall]

I tried to reproduce the issue #17 with this branch, using the example code of the report, where I replaced parList with parTraversable. With GHC (8.0.2), threaded runtime and "-O2" it created 1000 sparks as I expect. Probably #17 was not fixed, by changing rparWith? Or does it only affects newer GHCs?

module Main where

import Control.Parallel.Strategies

main :: IO ()
main = do let xs = take 1000 $ product . take 1000 . repeat <$> [1..]
              x  = product (xs `using` parTraversable rseq)
          putStrLn (show x)

[treeowl]

#17 was fixed by using unsafeDupablePerformIO (i.e., runRW#) instead of manually passing realWorld#.

[treeowl]

I think this PR can be closed. It's subsumed by #36.

[Borgvall]

Since #3 has been accepted, one can simplify this mess after merge to master:

printWhenEvaluated <- unsafePerformIO (putStrnLn "Must be printed at the end.") `usingIO` rparWith r0