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

Conversation

Projects
None yet
7 participants
@overvenus
Member

overvenus commented Aug 24, 2017

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)

@overvenus overvenus requested review from siddontang and BusyJay Aug 24, 2017

@BusyJay

This comment has been minimized.

Show comment
Hide comment
@BusyJay

BusyJay Aug 24, 2017

Contributor

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

Contributor

BusyJay commented Aug 24, 2017

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

@overvenus

This comment has been minimized.

Show comment
Hide comment
@overvenus

overvenus Aug 24, 2017

Member

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!

Member

overvenus commented Aug 24, 2017

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

This comment has been minimized.

Show comment
Hide comment
@overvenus

overvenus Aug 24, 2017

Member

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

Member

overvenus commented Aug 24, 2017

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

@siddontang

This comment has been minimized.

Show comment
Hide comment
@siddontang

siddontang Aug 27, 2017

Contributor

any performance improved for TiKV?

Contributor

siddontang commented Aug 27, 2017

any performance improved for TiKV?

@overvenus

This comment has been minimized.

Show comment
Hide comment
@overvenus

overvenus Aug 30, 2017

Member

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%
Member

overvenus commented Aug 30, 2017

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

This comment has been minimized.

Show comment
Hide comment
@arthurprs

arthurprs Aug 30, 2017

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

arthurprs commented Aug 30, 2017

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

@overvenus

This comment has been minimized.

Show comment
Hide comment
@overvenus

overvenus Aug 31, 2017

Member

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)
Member

overvenus commented Aug 31, 2017

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

This comment has been minimized.

Show comment
Hide comment
@siddontang

siddontang Aug 31, 2017

Contributor

I think we can reuse FNV again

What is your opinion, @BusyJay?

Contributor

siddontang commented Aug 31, 2017

I think we can reuse FNV again

What is your opinion, @BusyJay?

@ngaut

This comment has been minimized.

Show comment
Hide comment
@ngaut

ngaut Aug 31, 2017

Member

LGTM

Member

ngaut commented Aug 31, 2017

LGTM

@BusyJay

This comment has been minimized.

Show comment
Hide comment
@BusyJay

BusyJay Sep 1, 2017

Contributor

LGTM

Contributor

BusyJay commented Sep 1, 2017

LGTM

@overvenus overvenus merged commit 1a873b0 into master Sep 1, 2017

2 checks passed

ci/circleci Your tests passed on CircleCI!
Details
license/cla Contributor License Agreement is signed.
Details

@zhangjinpeng1987 zhangjinpeng1987 deleted the ov/back-to-fnv-map branch Sep 1, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment