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

Benchmark unfoldrN and inline createAndTrim' #128

Closed
piyush-kurur opened this issue May 29, 2017 · 6 comments · Fixed by #356
Closed

Benchmark unfoldrN and inline createAndTrim' #128

piyush-kurur opened this issue May 29, 2017 · 6 comments · Fixed by #356

Comments

@piyush-kurur
Copy link

piyush-kurur commented May 29, 2017

In many cases one wants to unfoldr exactly so many bytes (e.g base16/base64 encoding). In such cases
a version of unfoldrN of the following type is quite useful (and probably faster)

unfoldrN' :: Int -> (a -> (Word8, a)) -> (ByteString, a)

Any reason why this is not yet present.

@Bodigrim
Copy link
Contributor

Does unfoldrN' provide measurable performance benefits?

@piyush-kurur
Copy link
Author

@Bodigrim not sure but likely as you avoid testing for both length as well as whether the function returns a Just/Nothing. But even if not relevant for speed, it would be good just for completeness.

@Bodigrim
Copy link
Contributor

Bodigrim commented Sep 27, 2020

It seems there is a low demand for this function, and it can be easily defined on the user side:

unfoldrN' :: Int -> (a -> (Word8, a)) -> a -> (ByteString, a)
unfoldrN' n f s = fromJust <$> unfoldrN n (Just . f) s

Thanks to {-# INLINE unfoldrN #-} there will be no performance penalty for wrapping/unwrapping Just.

I'm quite reluctant to add new functions without a good justification, because they need to be implemented at least four times (lazy/strict ByteString, Word8/Char interfaces), plus potentially ShortByteString, plus ideally syncronized with Text.


That said, looking at Core program for unfoldrN I noticed that it could be better if we inline createAndTrim', similar to the existing pragma for createAndTrim.

@sjakobi
Copy link
Member

sjakobi commented Oct 1, 2020

I'm quite reluctant to add new functions without a good justification

👍

That said, looking at Core program for unfoldrN I noticed that it could be better if we inline createAndTrim', similar to the existing pragma for createAndTrim.

Sounds like a good idea to me.

@Bodigrim
Copy link
Contributor

Bodigrim commented Oct 1, 2020

A PR adding {-# INLINE createAndTrim' #-} and benchmarks for unfoldrN would be welcome.

@Bodigrim Bodigrim changed the title A stricter version unfoldrN Benchmark unfoldrN and inline createAndTrim' Oct 1, 2020
@piyush-kurur
Copy link
Author

@Bodigrim The 4 time implementation is no fun. I agree with you that this may not be very useful.

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

Successfully merging a pull request may close this issue.

3 participants