Update fxhash Hashes to hashbrown Hashes #160
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Hoverbear
reviewed
Dec 31, 2018
There was a problem hiding this comment.
Hi! Thanks so much for this super cool PR! Yes, we've been tracking the hashbrown changes. :)
I want to do a bit of benchmarking before we merge, but otherwise LGTM!
|
Awesome! Thank you :) If I see anything I can help with I will create another PR. 👍 |
Hoverbear
previously approved these changes
Jan 4, 2019
There was a problem hiding this comment.
Changes look good. This is not a clear cut improvement, some less frequently used functions are slower now, but some more frequently used ones are much faster.
Benchmarking summary:
- Initialization is slower (~10%)
Raft::new(),ProgressSet::with_capacity()had minor slowdowns in most cases.
- Inserting learners/voters is generally slowed down (varying between 10% faster and 10% slower, more slower).
- Getting a specific node, getting a set of nodes, or iterating over nodes is notably faster. (up to 35% faster, as low as 3% slower)
@siddontang What do you think? See full report in expandy below:
Full Benchmark Output
hoverbear@Obsidian:/mnt/c/Users/Hoverbear/Git/raft-rs$ cargo bench --bench benches -- --baseline master
Compiling raft v0.4.0 (/mnt/c/Users/Hoverbear/Git/raft-rs)
Finished release [optimized] target(s) in 7.55s
Running target/release/deps/benches-eea92d6733b3563a
Gnuplot not found, disabling plotting
Raft::new (0, 0) time: [660.53 ns 660.79 ns 661.08 ns]
change: [-0.8360% -0.7288% -0.6230%] (p = 0.00 < 0.05)
Change within noise threshold.
Found 7 outliers among 100 measurements (7.00%)
1 (1.00%) high mild
6 (6.00%) high severe
Raft::new (3, 1) time: [1.0857 us 1.0862 us 1.0869 us]
change: [+2.7570% +3.2795% +3.7550%] (p = 0.00 < 0.05)
Performance has regressed.
Found 14 outliers among 100 measurements (14.00%)
6 (6.00%) low mild
4 (4.00%) high mild
4 (4.00%) high severe
Raft::new (5, 2) time: [1.3799 us 1.3804 us 1.3810 us]
change: [+11.426% +11.582% +11.735%] (p = 0.00 < 0.05)
Performance has regressed.
Found 7 outliers among 100 measurements (7.00%)
4 (4.00%) high mild
3 (3.00%) high severe
Raft::new (7, 3) time: [1.5640 us 1.5646 us 1.5653 us]
change: [+8.1325% +9.0040% +9.5447%] (p = 0.00 < 0.05)
Performance has regressed.
Found 13 outliers among 100 measurements (13.00%)
2 (2.00%) low severe
5 (5.00%) high mild
6 (6.00%) high severe
Raft::campaign (3, 1, CampaignPreElection)
time: [1.5533 us 1.5552 us 1.5576 us]
change: [-0.9589% -0.7757% -0.5781%] (p = 0.00 < 0.05)
Change within noise threshold.
Found 11 outliers among 100 measurements (11.00%)
4 (4.00%) high mild
7 (7.00%) high severe
Raft::campaign (3, 1, CampaignElection)
time: [1.6494 us 1.6510 us 1.6532 us]
change: [-0.7879% -0.5272% -0.2864%] (p = 0.00 < 0.05)
Change within noise threshold.
Found 9 outliers among 100 measurements (9.00%)
1 (1.00%) low mild
3 (3.00%) high mild
5 (5.00%) high severe
Raft::campaign (3, 1, CampaignTransfer)
time: [1.7036 us 1.7072 us 1.7115 us]
change: [-0.4484% -0.2045% +0.0411%] (p = 0.12 > 0.05)
No change in performance detected.
Found 15 outliers among 100 measurements (15.00%)
1 (1.00%) high mild
14 (14.00%) high severe
Raft::campaign (5, 2, CampaignPreElection)
time: [2.3414 us 2.3508 us 2.3642 us]
change: [+5.2652% +5.5974% +6.0495%] (p = 0.00 < 0.05)
Performance has regressed.
Found 2 outliers among 100 measurements (2.00%)
1 (1.00%) high mild
1 (1.00%) high severe
Raft::campaign (5, 2, CampaignElection)
time: [2.4227 us 2.4273 us 2.4328 us]
change: [+4.9844% +5.2740% +5.5660%] (p = 0.00 < 0.05)
Performance has regressed.
Found 1 outliers among 100 measurements (1.00%)
1 (1.00%) high mild
Raft::campaign (5, 2, CampaignTransfer)
time: [2.5054 us 2.5098 us 2.5147 us]
change: [+3.5979% +3.8133% +4.0378%] (p = 0.00 < 0.05)
Performance has regressed.
Found 9 outliers among 100 measurements (9.00%)
5 (5.00%) high mild
4 (4.00%) high severe
Raft::campaign (7, 3, CampaignPreElection)
time: [2.9630 us 2.9647 us 2.9671 us]
change: [+3.2680% +3.4532% +3.6287%] (p = 0.00 < 0.05)
Performance has regressed.
Found 6 outliers among 100 measurements (6.00%)
2 (2.00%) high mild
4 (4.00%) high severe
Raft::campaign (7, 3, CampaignElection)
time: [3.0598 us 3.0609 us 3.0621 us]
change: [+2.6993% +2.9991% +3.2534%] (p = 0.00 < 0.05)
Performance has regressed.
Found 11 outliers among 100 measurements (11.00%)
2 (2.00%) high mild
9 (9.00%) high severe
Raft::campaign (7, 3, CampaignTransfer)
time: [3.2211 us 3.2273 us 3.2335 us]
change: [+3.0433% +3.2661% +3.4881%] (p = 0.00 < 0.05)
Performance has regressed.
Found 1 outliers among 100 measurements (1.00%)
1 (1.00%) high mild
RawNode::new time: [864.66 ns 865.46 ns 866.42 ns]
change: [-1.7034% -1.5750% -1.4265%] (p = 0.00 < 0.05)
Performance has improved.
Found 7 outliers among 100 measurements (7.00%)
5 (5.00%) high mild
2 (2.00%) high severe
Progress::default time: [20.373 ns 20.376 ns 20.380 ns]
change: [+2.5634% +2.7126% +2.8170%] (p = 0.00 < 0.05)
Performance has regressed.
Found 10 outliers among 100 measurements (10.00%)
7 (7.00%) high mild
3 (3.00%) high severe
ProgressSet::new time: [14.750 ns 14.753 ns 14.756 ns]
change: [-51.746% -51.716% -51.683%] (p = 0.00 < 0.05)
Performance has improved.
Found 11 outliers among 100 measurements (11.00%)
3 (3.00%) low mild
6 (6.00%) high mild
2 (2.00%) high severe
ProgressSet::with_capacity (0, 0)
time: [38.042 ns 38.046 ns 38.050 ns]
change: [+22.282% +22.354% +22.431%] (p = 0.00 < 0.05)
Performance has regressed.
Found 11 outliers among 100 measurements (11.00%)
7 (7.00%) high mild
4 (4.00%) high severe
ProgressSet::with_capacity (3, 1)
time: [89.843 ns 89.853 ns 89.865 ns]
change: [-6.9527% -6.9002% -6.8488%] (p = 0.00 < 0.05)
Performance has improved.
Found 13 outliers among 100 measurements (13.00%)
6 (6.00%) high mild
7 (7.00%) high severe
ProgressSet::with_capacity (5, 2)
time: [89.853 ns 89.866 ns 89.881 ns]
change: [-7.5653% -7.5094% -7.4420%] (p = 0.00 < 0.05)
Performance has improved.
Found 6 outliers among 100 measurements (6.00%)
3 (3.00%) high mild
3 (3.00%) high severe
ProgressSet::with_capacity (7, 3)
time: [89.046 ns 89.057 ns 89.070 ns]
change: [-8.4126% -8.3034% -8.2178%] (p = 0.00 < 0.05)
Performance has improved.
Found 6 outliers among 100 measurements (6.00%)
1 (1.00%) high mild
5 (5.00%) high severe
ProgressSet::insert_voter (0, 0)
time: [143.45 ns 143.55 ns 143.69 ns]
change: [+4.1462% +4.2315% +4.3178%] (p = 0.00 < 0.05)
Performance has regressed.
Found 18 outliers among 100 measurements (18.00%)
5 (5.00%) high mild
13 (13.00%) high severe
ProgressSet::insert_voter (3, 1)
time: [239.62 ns 239.94 ns 240.31 ns]
change: [-3.0834% -2.9575% -2.8202%] (p = 0.00 < 0.05)
Performance has improved.
Found 17 outliers among 100 measurements (17.00%)
6 (6.00%) high mild
11 (11.00%) high severe
ProgressSet::insert_voter (5, 2)
time: [307.16 ns 307.99 ns 308.99 ns]
change: [+7.3162% +8.3995% +9.3250%] (p = 0.00 < 0.05)
Performance has regressed.
Found 17 outliers among 100 measurements (17.00%)
3 (3.00%) high mild
14 (14.00%) high severe
ProgressSet::insert_voter (7, 3)
time: [349.07 ns 349.12 ns 349.18 ns]
change: [+11.165% +11.891% +12.296%] (p = 0.00 < 0.05)
Performance has regressed.
Found 13 outliers among 100 measurements (13.00%)
9 (9.00%) high mild
4 (4.00%) high severe
ProgressSet::insert_learner (0, 0)
time: [143.39 ns 143.41 ns 143.43 ns]
change: [+3.9997% +4.1978% +4.3356%] (p = 0.00 < 0.05)
Performance has regressed.
Found 6 outliers among 100 measurements (6.00%)
1 (1.00%) high mild
5 (5.00%) high severe
ProgressSet::insert_learner (3, 1)
time: [221.86 ns 222.09 ns 222.41 ns]
change: [-12.765% -11.872% -11.138%] (p = 0.00 < 0.05)
Performance has improved.
Found 14 outliers among 100 measurements (14.00%)
7 (7.00%) high mild
7 (7.00%) high severe
ProgressSet::insert_learner (5, 2)
time: [304.91 ns 305.45 ns 306.13 ns]
change: [+8.3026% +8.9372% +9.4217%] (p = 0.00 < 0.05)
Performance has regressed.
Found 10 outliers among 100 measurements (10.00%)
4 (4.00%) high mild
6 (6.00%) high severe
ProgressSet::insert_learner (7, 3)
time: [309.36 ns 309.63 ns 309.98 ns]
change: [-0.8372% -0.1840% +0.2765%] (p = 0.60 > 0.05)
No change in performance detected.
Found 10 outliers among 100 measurements (10.00%)
1 (1.00%) high mild
9 (9.00%) high severe
ProgressSet::promote (0, 0)
time: [34.717 ns 34.725 ns 34.734 ns]
change: [-1.7343% -1.6115% -1.5203%] (p = 0.00 < 0.05)
Performance has improved.
Found 7 outliers among 100 measurements (7.00%)
2 (2.00%) high mild
5 (5.00%) high severe
ProgressSet::promote (3, 1)
time: [209.32 ns 209.38 ns 209.46 ns]
change: [+1.8823% +2.3610% +2.6973%] (p = 0.00 < 0.05)
Performance has regressed.
Found 13 outliers among 100 measurements (13.00%)
8 (8.00%) high mild
5 (5.00%) high severe
ProgressSet::promote (5, 2)
time: [180.55 ns 180.83 ns 181.23 ns]
change: [-21.019% -20.407% -19.866%] (p = 0.00 < 0.05)
Performance has improved.
Found 14 outliers among 100 measurements (14.00%)
2 (2.00%) high mild
12 (12.00%) high severe
ProgressSet::promote (7, 3)
time: [217.40 ns 217.58 ns 217.86 ns]
change: [-18.084% -16.872% -15.818%] (p = 0.00 < 0.05)
Performance has improved.
Found 14 outliers among 100 measurements (14.00%)
6 (6.00%) high mild
8 (8.00%) high severe
ProgressSet::remove (0, 0)
time: [75.893 ns 75.901 ns 75.910 ns]
change: [-2.5186% -2.4594% -2.4057%] (p = 0.00 < 0.05)
Performance has improved.
Found 12 outliers among 100 measurements (12.00%)
2 (2.00%) low mild
6 (6.00%) high mild
4 (4.00%) high severe
ProgressSet::remove (3, 1)
time: [193.05 ns 193.29 ns 193.58 ns]
change: [-21.677% -21.596% -21.501%] (p = 0.00 < 0.05)
Performance has improved.
Found 12 outliers among 100 measurements (12.00%)
1 (1.00%) high mild
11 (11.00%) high severe
ProgressSet::remove (5, 2)
time: [229.40 ns 229.98 ns 230.74 ns]
change: [-18.526% -18.080% -17.718%] (p = 0.00 < 0.05)
Performance has improved.
Found 13 outliers among 100 measurements (13.00%)
3 (3.00%) high mild
10 (10.00%) high severe
ProgressSet::remove (7, 3)
time: [266.18 ns 266.54 ns 267.19 ns]
change: [-13.762% -13.676% -13.571%] (p = 0.00 < 0.05)
Performance has improved.
Found 10 outliers among 100 measurements (10.00%)
4 (4.00%) high mild
6 (6.00%) high severe
ProgressSet::iter (0, 0)
time: [30.000 ns 30.019 ns 30.046 ns]
change: [-1.8214% -1.4258% -0.7095%] (p = 0.00 < 0.05)
Change within noise threshold.
Found 13 outliers among 100 measurements (13.00%)
2 (2.00%) high mild
11 (11.00%) high severe
ProgressSet::iter (3, 1)
time: [133.82 ns 133.97 ns 134.15 ns]
change: [-34.102% -34.002% -33.915%] (p = 0.00 < 0.05)
Performance has improved.
Found 9 outliers among 100 measurements (9.00%)
2 (2.00%) high mild
7 (7.00%) high severe
ProgressSet::iter (5, 2)
time: [172.47 ns 172.93 ns 173.57 ns]
change: [-31.092% -30.541% -30.087%] (p = 0.00 < 0.05)
Performance has improved.
Found 9 outliers among 100 measurements (9.00%)
3 (3.00%) high mild
6 (6.00%) high severe
ProgressSet::iter (7, 3)
time: [209.91 ns 210.49 ns 211.49 ns]
change: [-24.688% -24.423% -24.178%] (p = 0.00 < 0.05)
Performance has improved.
Found 8 outliers among 100 measurements (8.00%)
2 (2.00%) high mild
6 (6.00%) high severe
ProgressSet::get (0, 0) time: [29.990 ns 29.995 ns 30.002 ns]
change: [+3.7093% +3.7742% +3.8356%] (p = 0.00 < 0.05)
Performance has regressed.
Found 7 outliers among 100 measurements (7.00%)
1 (1.00%) high mild
6 (6.00%) high severe
ProgressSet::get (3, 1) time: [133.45 ns 133.67 ns 134.02 ns]
change: [-30.273% -29.517% -28.896%] (p = 0.00 < 0.05)
Performance has improved.
Found 15 outliers among 100 measurements (15.00%)
1 (1.00%) high mild
14 (14.00%) high severe
ProgressSet::get (5, 2) time: [171.63 ns 172.93 ns 174.39 ns]
change: [-23.691% -22.675% -21.785%] (p = 0.00 < 0.05)
Performance has improved.
Found 19 outliers among 100 measurements (19.00%)
1 (1.00%) high mild
18 (18.00%) high severe
ProgressSet::get (7, 3) time: [207.84 ns 208.07 ns 208.38 ns]
change: [-17.017% -16.736% -16.426%] (p = 0.00 < 0.05)
Performance has improved.
Found 13 outliers among 100 measurements (13.00%)
1 (1.00%) high mild
12 (12.00%) high severe
ProgressSet::nodes (0, 0)
time: [29.995 ns 30.001 ns 30.009 ns]
change: [-1.8226% -1.7582% -1.6787%] (p = 0.00 < 0.05)
Performance has improved.
Found 12 outliers among 100 measurements (12.00%)
3 (3.00%) high mild
9 (9.00%) high severe
ProgressSet::nodes (3, 1)
time: [133.84 ns 133.95 ns 134.10 ns]
change: [-34.382% -34.194% -34.045%] (p = 0.00 < 0.05)
Performance has improved.
Found 11 outliers among 100 measurements (11.00%)
3 (3.00%) high mild
8 (8.00%) high severe
ProgressSet::nodes (5, 2)
time: [171.82 ns 171.84 ns 171.86 ns]
change: [-32.504% -31.803% -31.210%] (p = 0.00 < 0.05)
Performance has improved.
Found 5 outliers among 100 measurements (5.00%)
2 (2.00%) high mild
3 (3.00%) high severe
ProgressSet::nodes (7, 3)
time: [209.91 ns 209.94 ns 209.98 ns]
change: [-24.741% -24.401% -24.121%] (p = 0.00 < 0.05)
Performance has improved.
Found 9 outliers among 100 measurements (9.00%)
2 (2.00%) high mild
7 (7.00%) high severe
|
The most of reduction is for insert, which has a low frequency. I think it is fine, but we should still know why. |
brson
reviewed
Jan 15, 2019
src/progress.rs
Outdated
| use fxhash::{FxBuildHasher, FxHashMap, FxHashSet}; | ||
| use hashbrown::{HashMap, HashSet}; | ||
| type DefaultHashMapBuilder = ::hashbrown::hash_map::DefaultHashBuilder; | ||
| type DefaultHashSetBuilder = ::hashbrown::hash_map::DefaultHashBuilder; |
There was a problem hiding this comment.
I'd prefer not to have type aliases in the import list, to either turn this into a use .. as statement or move it below the imports. I wouldn't hold up the PR on it though since it's already been in review a long time.
|
The code itself looks good. There are enough performance wins here that it seems worth merging. @bdelmas if you are up to it, it would be very nice to investigate the regressions. Maybe there's something half-baked about hashbrown yet. @siddontang do you think we should investigate the regression cases here before landing? |
|
I don't see us not moving to hashbrown since it's already in use in TiKV, the wins here are encouraging, and I'd like not to hold up @bdelmas's PR longer. I think we should go ahead and merge, and if investigating the regressions is important to you @siddontang maybe we can talk offline and I can do it. LGTM. Thanks for your patience @bdelmas ! |
brson
previously approved these changes
Jan 15, 2019
|
Thank you guys! @brson I made the changes you wanted plus since you already use Ya I will try to investigate. If I find anything I will comment here or create an issue/PR. Thanks again! |
brson
approved these changes
Jan 15, 2019
|
Thanks @bdelmas |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Hey!
I was looking for small improvements I could do to improve this project and I was thinking about using the "new" SwissTable hashes from Google.
hashbrownis a crate that implements them and it says it is around 2 times faster thanfxhash.This PR is updating all
HashMapandHashSetoffxhashto use the more performanteHashMapandHashSetfromhashbrown.Even if
hashbrownis a drop-in replacement for the standard hashes, this PR changes only the hashes offxhash, just to make the changes more atomic.Feel free to change this PR if you want!
Thank you guys for this great crate 😃