-
Notifications
You must be signed in to change notification settings - Fork 24
Restore rparWith behavior #36
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
Conversation
Now that I understand runEval (r0 a) = a
runEval (rpar a) = a
runEval (rseq a) = a we need rparWith r0 = r0
rparWith rpar = rpar
rparWith rseq = rpar
rparWith (rparWith strat) = rparWith strat |
Control/Parallel/Strategies.hs
Outdated
-- rparWith strat x | ||
-- @ | ||
-- | ||
-- will spark a thread to perform @strat x@ in parallel. Note |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
let's not use the terminology "thread" here to avoid confusion. How about "will spark @strat x@".
-- WHNF, while @rparWith strat@ need not. | ||
-- | ||
-- > rparWith r0 = r0 | ||
-- > rparWith rpar = rpar |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The LHS creates two sparks, I'm not sure I'd call them equal. It depends what equality you're using, but as I argued elsewhere the only sensible notions of equality here are ones that care about sparks.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
rparWith rpar
is actually a quite weird thing: it creates a spark, who creates a spark, who evaluates to WHNF.
However one can probably replace that law with rparWith rseq = rpar
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How does one replace the other? What we probably actually want is some sort of equivalence relation that equates "essentially equivalent" things. rpar x >>= rseq
is effectively the same as rseq x
, but I don't know how to express that.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@simonmar, my intuition suggests that we're never going to find all the relations (Eval
is a very large structure), but that we should document some of the more useful ones. It's easy to think you're getting more parallelism than you are when composing things. Speaking of relations,
runEval (m >>= rpar) = runEval m
rpar x >>= \x' -> rpar y >>= f = rpar y >>= \y' -> rpar x >>= f
-- when x' is not free in y and y' is not free in x.
I don't think it pays too much to distinguish too much between sparks that
are (almost) guaranteed to fizzle and sparks that don't spark, especially
since I think it reasonable to use rewrite rules to turn the former into
the latter.
|
Control/Parallel/Strategies.hs
Outdated
|
||
data Lift a = Lift a | ||
#else | ||
rparWith s a = do l <- rpar (s a); return (case l of Done x -> x) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
AFAIK, we can also use the GHC >= 7.2 version here, saving some code dupling.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good point.
-- `rseq` in `rparWith`. In particular, we want rparWith r0 = r0, not | ||
-- rparWith r0 = rpar. | ||
rparWith s a = do | ||
l <- rpar r |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This creates (nearly) always a fizzled spark, because l
is immediately pattern matched. A lazy pattern match is needed.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The pattern match is lazy (it's in the argument to return
). Try your test!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Moreover, if you inline everything manually (including the runRW#
) you'll end up with the same thing as the old implementation. We just shouldn't do that because inlining the realWorld#
is like asking the optimizer to completely mangle our intentions.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(FYI, search the bytestring
source code for accursedUnutterablePerformIO
if you want a more detailed explanation.)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry I was wrong with my comment.
It does not pass with the current implementation of `rparWith`.
* Lift the result to avoid always reducing to WHNF. * Rewrite the documentation of `rparWith`. Fixes haskell#35
rparWith
.Fixes #35