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

Use faster hash map implementation #1

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

andrewhop
Copy link
Contributor

Issue #, if available:
N/A

Description of changes:
While reading this blog post about how hash maps and Google's SwissTable work I was curious to see it's impact to this code. Hashbrown is a rust implementation of SwissTable. There is a discussion about using this in the standard rust hash map in rust-lang/rust#55514 but until that get migrated this saves us ~3 seconds. It only affects the final sieve:
Standard:

[bloom_t1] Completed 4294967296 in 13811ms
[t2_map] Completed 4294967296 in 6261ms
[final_sieve] Completed in 4272ms
[total]: Completed in 24375ms

[bloom_t1] Completed 4294967296 in 14673ms
[t2_map] Completed 4294967296 in 6162ms
[final_sieve] Completed in 4349ms
[total]: Completed in 25223ms

hashbrown:

[bloom_t1] Completed 4294967296 in 13715ms
[t2_map] Completed 4294967296 in 6182ms
[final_sieve] Completed in 1898ms
[total]: Completed in 21821ms

[bloom_t1] Completed 4294967296 in 13854ms
[t2_map] Completed 4294967296 in 6156ms
[final_sieve] Completed in 1882ms
[total]: Completed in 21920ms

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

src/main.rs Outdated
@@ -21,6 +22,7 @@ fn main() {

// fp p<=0.001, 32GiB, k=2
let filter = bloom_t1(&T1_INVERSE);
println!("[timing]: bloom_t1 {} milliseconds", total.elapsed().as_millis());
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This will break entirely if we don't have the unstable feature enabled.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Updated it to use seconds/milliseconds correctly:

cargo +nightly run --features numa --release | grep Completed
[bloom_t1] Completed 4294967296 in 120 sec
[t2_map] Completed 4294967296 in 12 sec
[final_sieve] Completed in 1 sec
Completed in: 134 sec, primes found: 55

cargo +nightly run --features numa,unstable --release | grep Completed
[bloom_t1] Completed 4294967296 in 13813 ms
[t2_map] Completed 4294967296 in 6179 ms
[final_sieve] Completed in 1923 ms
Completed in: 21945 ms, primes found: 55

@@ -49,7 +49,7 @@ pub use self::numa::*;
mod numa {
extern crate nix;

use libc::{c_char, c_uint, c_int, c_ulong};
use numa_threadpool::numa::nix::libc::{c_char, c_uint, c_int, c_ulong};
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't quite understand why removing the unstable flag caused the build to fail for missing imports, the compile suggested this and it fixed it.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's because lib.rs has a #[cfg(feature="unstable")] mark on the libc extern crate (and I think cargo conditional-depends on it as well)

Copy link

@bdonlan bdonlan Dec 10, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To fix this:

  1. Remove optional=true from the libc clause in Cargo.toml
  2. Remove "libc" from the feature definitions in Cargo.toml
  3. Add an extern crate libc; to src/lib.rs - or update to Rust 2018 :)

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.

None yet

3 participants