Skip to content

Conversation

@treeowl
Copy link
Contributor

@treeowl treeowl commented Jul 18, 2019

foldrBits was previously defined just like foldr'Bits, and
foldlBits was defined just like foldl'Bits. That is, each of them
built up an accumulator, lazily, across a whole leaf before producing
anything. Swap that around so the folds can process values promptly.

Fixes #666

treeowl added 2 commits July 17, 2019 20:17
`foldrBits` was previously defined just like `foldr'Bits`, and
`foldlBits` was defined just like `foldl'Bits`. That is, each of them
built up an accumulator, lazily, across a whole leaf before producing
anything. Swap that around so the folds can process values promptly.

Fixes haskell#666
@treeowl
Copy link
Contributor Author

treeowl commented Jul 18, 2019

I've done a bit of benchmarking. My results aren't very stable, but it seems, generally, that foldr is pretty similar (perhaps because of the cost of reversing the bits) while foldl is improved. Perhaps someone better at benchmarking and/or micro-optimization should take a look at it.

@treeowl
Copy link
Contributor Author

treeowl commented Jul 18, 2019

Er.... no, that's not it. The reversing happens with foldl.... I have no explanation.

@treeowl
Copy link
Contributor Author

treeowl commented Jul 22, 2019

@oisdk, this looks like the sort of thing you might know how to think about and test properly. Any ideas?

@meooow25
Copy link
Contributor

@treeowl do you want to continue with this PR? The changes to foldrBits and foldlBits seem ok to me.
Once rebased we could add some benchmarks similar to what we recently added for Data.Tree's foldr and measure if there's an improvement.

@treeowl
Copy link
Contributor Author

treeowl commented Jul 29, 2023

I'm currently recovering from COVID, so can't really do much this week.

@meooow25
Copy link
Contributor

Hope you get well soon! I didn't mean to imply any sort of urgency, this can surely wait.

@treeowl
Copy link
Contributor Author

treeowl commented Jul 29, 2023

I feel bad for making you wait so long on multiple things.

@meooow25
Copy link
Contributor

I have had some free time, don't worry about it 😅

@meooow25
Copy link
Contributor

meooow25 commented Jan 5, 2025

Fixed by #1079.

@meooow25 meooow25 closed this Jan 5, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

IntSet foldl and foldr smell funny

3 participants