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 FnvHashMap again #2211

Merged
merged 9 commits into from
Sep 1, 2017
Merged

*: use FnvHashMap again #2211

merged 9 commits into from
Sep 1, 2017

Conversation

overvenus
Copy link
Member

Benchmark at https://github.com/overvenus/benches-rust

Base on the benchmark, fnv hash map outperforms ordermap and fnv ordermap.

test collections::bench_fnv_map_little                 ... bench:         165 ns/iter (+/- 7)
test collections::bench_ordermap_little                ... bench:         387 ns/iter (+/- 9)
test collections::bench_fnv_ordermap_little            ... bench:         337 ns/iter (+/- 11)

test collections::bench_fnv_map_large                  ... bench:      20,211 ns/iter (+/- 495)
test collections::bench_ordermap_large                 ... bench:      42,226 ns/iter (+/- 589)
test collections::bench_fnv_ordermap_large             ... bench:      37,202 ns/iter (+/- 860)

test collections::bench_fnv_map_query_16               ... bench:          15 ns/iter (+/- 0)
test collections::bench_fnv_map_query_32               ... bench:          17 ns/iter (+/- 0)
test collections::bench_fnv_map_query_64               ... bench:          15 ns/iter (+/- 0)
test collections::bench_fnv_map_query_128              ... bench:          15 ns/iter (+/- 0)
test collections::bench_fnv_map_query_512              ... bench:          16 ns/iter (+/- 0)

test collections::bench_ordermap_query_16              ... bench:          23 ns/iter (+/- 0)
test collections::bench_ordermap_query_32              ... bench:          23 ns/iter (+/- 0)
test collections::bench_ordermap_query_64              ... bench:          24 ns/iter (+/- 0)
test collections::bench_ordermap_query_128             ... bench:          24 ns/iter (+/- 0)
test collections::bench_ordermap_query_512             ... bench:          25 ns/iter (+/- 0)

test collections::bench_fnv_ordermap_query_16          ... bench:          18 ns/iter (+/- 0)
test collections::bench_fnv_ordermap_query_32          ... bench:          18 ns/iter (+/- 0)
test collections::bench_fnv_ordermap_query_64          ... bench:          18 ns/iter (+/- 0)
test collections::bench_fnv_ordermap_query_128         ... bench:          19 ns/iter (+/- 0)
test collections::bench_fnv_ordermap_query_512         ... bench:          20 ns/iter (+/- 0)

test collections::bench_fnv_map_delete_insert_16       ... bench:          42 ns/iter (+/- 1)
test collections::bench_fnv_map_delete_insert_32       ... bench:          34 ns/iter (+/- 0)
test collections::bench_fnv_map_delete_insert_64       ... bench:          34 ns/iter (+/- 0)
test collections::bench_fnv_map_delete_insert_128      ... bench:          44 ns/iter (+/- 1)
test collections::bench_fnv_map_delete_insert_512      ... bench:          38 ns/iter (+/- 1)

test collections::bench_ordermap_delete_insert_16      ... bench:          61 ns/iter (+/- 0)
test collections::bench_ordermap_delete_insert_32      ... bench:          64 ns/iter (+/- 2)
test collections::bench_ordermap_delete_insert_64      ... bench:          64 ns/iter (+/- 1)
test collections::bench_ordermap_delete_insert_128     ... bench:          63 ns/iter (+/- 0)
test collections::bench_ordermap_delete_insert_512     ... bench:          67 ns/iter (+/- 1)

test collections::bench_fnv_ordermap_delete_insert_16  ... bench:          55 ns/iter (+/- 2)
test collections::bench_fnv_ordermap_delete_insert_32  ... bench:          54 ns/iter (+/- 0)
test collections::bench_fnv_ordermap_delete_insert_64  ... bench:          56 ns/iter (+/- 0)
test collections::bench_fnv_ordermap_delete_insert_128 ... bench:          56 ns/iter (+/- 1)
test collections::bench_fnv_ordermap_delete_insert_512 ... bench:          61 ns/iter (+/- 2)

@BusyJay
Copy link
Member

BusyJay commented Aug 24, 2017

Why OrderMap is prefered now? The result seems conflict with the previous benchmark, so which is correct?

@overvenus
Copy link
Member Author

Actually, fnv hashmap is prefered now :-P
It's up to compilation strategy. The previous benchmark compiled with CARGO_INCREMENTAL=1 that makes ordermap faster than fnv hashmap. Since TiKV release builds without incremental, this PR is correct!

@overvenus
Copy link
Member Author

FYI, rust has an issue (rust-lang/rust#39773) about CARGO_INCREMENTAL=1 performance.

@siddontang
Copy link
Contributor

any performance improved for TiKV?

@overvenus
Copy link
Member Author

FnvHashMap slightly improves performance.

RawKV puts on 1 TiKV.

  OrderMap FnvHashMap
TPS (per sec. avg(2min)) 65180.04 65326.89
raftstore CPU ~86.7% ~85.3%
apply worker CPU ~97.8% ~97.9%

@arthurprs
Copy link

Probably influenced by this PR as well rust-lang/rust#40561

@overvenus
Copy link
Member Author

Thanks for pointing out! @arthurprs

Your implementation has been adopted since nightly-2017-04-07. I did benchmarks for both nightly-2017-04-07 and nightly-2017-04-06 (the old). 04-07 is faster!

Left: 04-06
Right: 04-07

test collections::bench_fnv_map_delete_insert_128            39 ns/iter (+/- 0)          36 ns/iter (+/- 0)
test collections::bench_fnv_map_delete_insert_16             37 ns/iter (+/- 1)          40 ns/iter (+/- 0)
test collections::bench_fnv_map_delete_insert_32             35 ns/iter (+/- 0)          30 ns/iter (+/- 0)
test collections::bench_fnv_map_delete_insert_512            41 ns/iter (+/- 0)          36 ns/iter (+/- 0)
test collections::bench_fnv_map_delete_insert_64             44 ns/iter (+/- 0)          35 ns/iter (+/- 0)
test collections::bench_fnv_map_large                    19,216 ns/iter (+/- 179)    17,992 ns/iter (+/- 164)
test collections::bench_fnv_map_little                      144 ns/iter (+/- 2)         149 ns/iter (+/- 3)
test collections::bench_fnv_map_query_128                    18 ns/iter (+/- 0)          17 ns/iter (+/- 0)
test collections::bench_fnv_map_query_16                     17 ns/iter (+/- 0)          15 ns/iter (+/- 0)
test collections::bench_fnv_map_query_32                     17 ns/iter (+/- 0)          15 ns/iter (+/- 0)
test collections::bench_fnv_map_query_512                    22 ns/iter (+/- 0)          19 ns/iter (+/- 0)
test collections::bench_fnv_map_query_64                     17 ns/iter (+/- 0)          15 ns/iter (+/- 0)
test collections::bench_fnv_ordermap_delete_insert_128       48 ns/iter (+/- 0)          45 ns/iter (+/- 0)
test collections::bench_fnv_ordermap_delete_insert_16        45 ns/iter (+/- 0)          54 ns/iter (+/- 0)
test collections::bench_fnv_ordermap_delete_insert_32        46 ns/iter (+/- 0)          43 ns/iter (+/- 0)
test collections::bench_fnv_ordermap_delete_insert_512       47 ns/iter (+/- 1)          49 ns/iter (+/- 0)
test collections::bench_fnv_ordermap_delete_insert_64        43 ns/iter (+/- 0)          44 ns/iter (+/- 0)
test collections::bench_fnv_ordermap_large               26,292 ns/iter (+/- 265)    24,857 ns/iter (+/- 334)
test collections::bench_fnv_ordermap_little                 215 ns/iter (+/- 4)         215 ns/iter (+/- 6)
test collections::bench_fnv_ordermap_query_128               19 ns/iter (+/- 0)          20 ns/iter (+/- 0)
test collections::bench_fnv_ordermap_query_16                18 ns/iter (+/- 0)          18 ns/iter (+/- 0)
test collections::bench_fnv_ordermap_query_32                20 ns/iter (+/- 0)          19 ns/iter (+/- 0)
test collections::bench_fnv_ordermap_query_512               20 ns/iter (+/- 0)          20 ns/iter (+/- 0)
test collections::bench_fnv_ordermap_query_64                20 ns/iter (+/- 0)          19 ns/iter (+/- 0)
test collections::bench_ordermap_delete_insert_128           48 ns/iter (+/- 0)          48 ns/iter (+/- 0)
test collections::bench_ordermap_delete_insert_16            53 ns/iter (+/- 0)          53 ns/iter (+/- 1)
test collections::bench_ordermap_delete_insert_32            48 ns/iter (+/- 1)          49 ns/iter (+/- 0)
test collections::bench_ordermap_delete_insert_512           52 ns/iter (+/- 0)          52 ns/iter (+/- 0)
test collections::bench_ordermap_delete_insert_64            47 ns/iter (+/- 0)          48 ns/iter (+/- 0)
test collections::bench_ordermap_large                   30,014 ns/iter (+/- 336)    29,758 ns/iter (+/- 314)
test collections::bench_ordermap_little                     242 ns/iter (+/- 6)         240 ns/iter (+/- 2)
test collections::bench_ordermap_query_128                   24 ns/iter (+/- 0)          24 ns/iter (+/- 0)
test collections::bench_ordermap_query_16                    24 ns/iter (+/- 0)          24 ns/iter (+/- 0)
test collections::bench_ordermap_query_32                    24 ns/iter (+/- 0)          24 ns/iter (+/- 0)
test collections::bench_ordermap_query_512                   26 ns/iter (+/- 0)          25 ns/iter (+/- 0)
test collections::bench_ordermap_query_64                    25 ns/iter (+/- 0)          25 ns/iter (+/- 0)

@siddontang
Copy link
Contributor

I think we can reuse FNV again

What is your opinion, @BusyJay?

@ngaut
Copy link
Member

ngaut commented Aug 31, 2017

LGTM

1 similar comment
@BusyJay
Copy link
Member

BusyJay commented Sep 1, 2017

LGTM

@overvenus overvenus merged commit 1a873b0 into master Sep 1, 2017
@zhangjinpeng87 zhangjinpeng87 deleted the ov/back-to-fnv-map branch September 1, 2017 09:13
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.

7 participants