Skip to content

Modernize bitv and implement a bit vector set #5005

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

Closed
wants to merge 6 commits into from

Conversation

alexcrichton
Copy link
Member

These commits take the old bitv implementation and modernize it with an explicit self, some minor touchups, and using what I think is some more recent patterns (like ::new instead of Type()).

Additionally, this adds an implementation of container::Set on top of a bit vector to have as a set of uints. I initially tried to parameterize the type for the set to be T: NumCast but I was hitting build problems in stage0 which I think means that it's not in a snapshot yet, so it's just hardcoded as a set of uints now. In the future perhaps it could be parameterized. I'm not sure if it would really add anything, though, so maybe it's nicer to be hardcoded anyway.

I also added some extra methods to do normal bit vector operations on the set in-place, but these aren't a part of the Set trait right now. I haven't benchmarked any of these operations just yet, but I imagine that there's quite a lot of room for optimization here and there.

@catamorphism
Copy link
Contributor

Nice work! Performance benchmarks for the Set implementation would be nice, but that can happen in the future.

@alexcrichton
Copy link
Member Author

Using this benchmark, I get these results:

$ ./src/test/bench/core-set 1000000                          
core::hashmap::LinearSet
               sequential_ints 0.474796 s
                   random_ints 0.235602 s
                   delete_ints 0.208556 s
            sequential_strings 2.108495 s
                random_strings 1.340088 s
                delete_strings 1.05437 s
std::treemap::TreeSet
               sequential_ints 0.739227 s
                   random_ints 0.747073 s
                   delete_ints 0.438264 s
            sequential_strings 3.898206 s
                random_strings 3.312389 s
                delete_strings 1.680569 s
std::bitv::BitvSet
               sequential_ints 0.111517 s
                   random_ints 0.112675 s
                   delete_ints 0.08035 s
            sequential_strings 0 s
                random_strings 0 s
                delete_strings 0 s

I'm not convinced that it's all that great of a benchmark, but at least it's performing admirably for inputs with small numbers. If the cap for the random int benchmark is removed, it's far slower than TreeSet and LinearSet.

I'll also see if I can benchmark the move from a global function to a closure to make sure that it didn't hit perf at all (as per the comment on 393a4b4)

@catamorphism
Copy link
Contributor

Great! Good benchmarks are hard to do, but it's nice to have a starting point (and one that shows good preliminary results).

bors added a commit that referenced this pull request Feb 19, 2013
These commits take the old bitv implementation and modernize it with an explicit self, some minor touchups, and using what I think is some more recent patterns (like `::new` instead of `Type()`).

Additionally, this adds an implementation of `container::Set` on top of a bit vector to have as a set of `uint`s. I initially tried to parameterize the type for the set to be `T: NumCast` but I was hitting build problems in stage0 which I think means that it's not in a snapshot yet, so it's just hardcoded as a set of `uint`s now. In the future perhaps it could be parameterized. I'm not sure if it would really add anything, though, so maybe it's nicer to be hardcoded anyway.

I also added some extra methods to do normal bit vector operations on the set in-place, but these aren't a part of the `Set` trait right now. I haven't benchmarked any of these operations just yet, but I imagine that there's quite a lot of room for optimization here and there.
@bors bors closed this Feb 19, 2013
bors added a commit to rust-lang-ci/rust that referenced this pull request May 2, 2020
Travis: Use windows-msvc target for Windows build

changelog: none
closes rust-lang#5005
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.

3 participants