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

Unusual definition of foldrBits and foldlBits #966

Closed
meooow25 opened this issue Jul 20, 2023 · 3 comments
Closed

Unusual definition of foldrBits and foldlBits #966

meooow25 opened this issue Jul 20, 2023 · 3 comments

Comments

@meooow25
Copy link
Contributor

foldrBits is defined in Data.IntSet.Internal as

foldrBits prefix f z bitmap = go (revNat bitmap) z
where go 0 acc = acc
go bm acc = go (bm `xor` bitmask) ((f $! (prefix+(WORD_SIZE_IN_BITS-1)-bi)) acc)
where !bitmask = lowestBitMask bm
!bi = indexOfTheOnlyBit bitmask

This is unusual because go traverses the full word, building up thunks, before returning a result.
I would expect a foldr to be defined as

go 0 = z
go bm = f x (go bm')
  where x = ...
        bm' = ...

I don't think this has an effect on correctness, but it might have an effect on performance.

The implementation was introduced in e076b33, which says

Foldr implementations bit-reverse the word and then iterate from the LSB
to MSB using accumulator. That is faster then either not using
accumulator or iterating from MSB to LSB.

Which seems a bit odd and worth rechecking in my opinion. Thoughts?

@treeowl
Copy link
Contributor

treeowl commented Jul 20, 2023

Yes, I completely agree.

@meooow25
Copy link
Contributor Author

Great, happy to take a look at it after I'm done with some of the other PRs here. Don't want to take on too much at once.

@meooow25
Copy link
Contributor Author

I just found #666 which describes this very issue.

@meooow25 meooow25 closed this as not planned Won't fix, can't repro, duplicate, stale Jul 29, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants