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

Provide instances for bitvec #21

Merged
merged 2 commits into from Dec 4, 2019
Merged

Provide instances for bitvec #21

merged 2 commits into from Dec 4, 2019

Conversation

Bodigrim
Copy link
Contributor

Closes #20.

Tests can be found in https://github.com/Bodigrim/hw-rankselect-base/tree/tests, but I do not submit it for merge because of orphan instances.

@Bodigrim
Copy link
Contributor Author

How does it look, @newhoggy?

@newhoggy
Copy link
Member

newhoggy commented Dec 1, 2019

Sorry, I've been pre-occupied with other things. Looking at this now.

@newhoggy
Copy link
Member

newhoggy commented Dec 1, 2019

Do you by any chance know what this comment in the bitvec docs is referring to?

This module exposes an interface with thread-unsafe writes and flips. Consider using Data.Bit.ThreadSafe, which is thread-safe, but slower (up to 20%).

@Bodigrim
Copy link
Contributor Author

Bodigrim commented Dec 1, 2019

Sure, most of the current incarnation of bitvec is written by me.

https://github.com/Bodigrim/bitvec/blob/master/README.md#thread-safety gives a bit more details:

  • Data.Bit is faster, but writes and flips are thread-unsafe. This is because naive updates are not atomic: read the whole word from memory, then modify a bit, then write the whole word back.
  • Data.Bit.ThreadSafe is slower (up to 20%), but writes and flips are thread-safe.

If multiple threads concurrently write neighbouring bits in the same word, they may end up with inconsistent states, effectively losing the work of one of the threads:

thread1: x <- arr[0]
thread2: y <- arr[0]
thread1: x |= 1<<16
thread2: y |= 1<<24
thread1: arr[0] <- x 
thread2: arr[0] <- y

In multithreaded enviroment we must ensure that threads access a mutable vector in turns:

thread1: x <- arr[0]
thread1: x |= 1<<16
thread1: arr[0] <- x 
thread2: y <- arr[0]
thread2: y |= 1<<24
thread2: arr[0] <- y

Data.Bit.ThreadSafe ensures this restriction, using atomic locks. Unfortunately, there is an associated performance cost, non-negligible for some applications. That is why bitvec provides both Data.Bit and Data.Bit.ThreadSafe.

@newhoggy
Copy link
Member

newhoggy commented Dec 1, 2019

Thanks for the explanation.

@newhoggy
Copy link
Member

newhoggy commented Dec 1, 2019

Would you be able to add property tests to verify that the indexing and endian-ness is the same as rank/select for say [Bool] or Data.Vector.Storable.Vector Word64?

@Bodigrim
Copy link
Contributor Author

Bodigrim commented Dec 1, 2019

As mentioned above, I can add the tests (https://github.com/Bodigrim/hw-rankselect-base/commit/b0c6b5c15e0034b4bb9dfa7d83dba921592b7772), but they involve a (rather hastily written) orphan instance BitRead (DVU.Vector Bit.Bit). Is it OK?

Or I can take a long way and add these instances to hw-bits, if you are happy to accept such contributions.

@newhoggy
Copy link
Member

newhoggy commented Dec 2, 2019

Please add the instance to hw-bits. 😁

@Bodigrim
Copy link
Contributor Author

Bodigrim commented Dec 4, 2019

I have added some tests, please re-review.

@newhoggy
Copy link
Member

newhoggy commented Dec 4, 2019

Nice! Thanks!

@newhoggy newhoggy merged commit c3c9cd0 into haskell-works:master Dec 4, 2019
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

Successfully merging this pull request may close these issues.

Instances for Vector Bit
2 participants