-
Notifications
You must be signed in to change notification settings - Fork 141
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
Hand SAT optimization for Data.ByteString.unfoldrN #356
Conversation
Cool! Could you please add a benchmark to And if you throw |
@SkamDart feel free to ping me if you need any guidance with regards to adding a benchmark. |
@Bodigrim - Thanks for reaching out. Will address your comments this weekend and reach out to you if I am having any troubles! |
With Optimizations:
Without Optimizations
|
bench/BenchAll.hs
Outdated
@@ -416,6 +416,8 @@ main = do | |||
nf (S.foldl' (\acc x -> acc + fromIntegral x) (0 :: Int)) s) foldInputs | |||
, bgroup "foldr'" $ map (\s -> bench (show $ S.length s) $ | |||
nf (S.foldr' (\x acc -> fromIntegral x + acc) (0 :: Int)) s) foldInputs | |||
, bgroup "unfoldrN" $ map (\s -> bench (show $ S.length s) $ | |||
nf (S.unfoldrN (S.length s) (\_ -> Nothing) ) s) foldInputs |
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 is a strange choice of benchmark: you are essentially building an empty string and do not invoke the inner loop of unfoldrN
at all. Please put something like
nf (S.unfoldrN (S.length s) (\a -> (a, a + 1)) 0)
Please rebase your branch atop of master. Then rerun benchmarks as follows
|
@SkamDart could you please rebase your branch? |
33dbbd5
to
9f73055
Compare
Using my suggested benchmark fix:
So there's hardly an effect, but it doesn't seem to hurt either. |
@SkamDart are you still interested to carry on with this PR? |
9f73055
to
3c7f9cc
Compare
@Bodigrim appreciate your patience through the radio silence on my end.
|
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.
Thanks! :)
* appropriate unfoldrN benchmark * Hand SAT optimization for Data.ByteString.unfoldrN * inline createAndTrim'
* appropriate unfoldrN benchmark * Hand SAT optimization for Data.ByteString.unfoldrN * inline createAndTrim'
This follows the approach described in #350 to optimize
unfoldrN
to float outp
and usepokeByteOff