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 kXXH3 as default checksum (CPU efficiency) #10778

Closed
wants to merge 3 commits into from

Conversation

pdillinger
Copy link
Contributor

@pdillinger pdillinger commented Oct 5, 2022

Summary: Since this has been supported for about a year, I think it's time to make it the default. This should improve CPU efficiency slightly on most hardware.

A current DB performance comparison using buck+clang build:

TEST_TMPDIR=/dev/shm ./db_bench -checksum_type={1,4} -benchmarks=fillseq[-X1000] -num=3000000 -disable_wal

kXXH3 (+0.2% DB write throughput):
fillseq [AVG 1000 runs] : 822149 (± 1004) ops/sec; 91.0 (± 0.1) MB/sec
kCRC32c:
fillseq [AVG 1000 runs] : 820484 (± 1203) ops/sec; 90.8 (± 0.1) MB/sec

Micro benchmark comparison:

./db_bench --benchmarks=xxh3[-X20],crc32c[-X20]

Machine 1, buck+clang build:
xxh3 [AVG 20 runs] : 3358616 (± 19091) ops/sec; 13119.6 (± 74.6) MB/sec
crc32c [AVG 20 runs] : 2578725 (± 7742) ops/sec; 10073.1 (± 30.2) MB/sec

Machine 2, make+gcc build, DEBUG_LEVEL=0 PORTABLE=0:
xxh3 [AVG 20 runs] : 6182084 (± 137223) ops/sec; 24148.8 (± 536.0) MB/sec
crc32c [AVG 20 runs] : 5032465 (± 42454) ops/sec; 19658.1 (± 165.8) MB/sec

Test Plan: make check, unit tests updated

Summary: Since this has been supported for about a year, I think it's
time to make it the default. This should improve CPU efficiency slightly
on most hardware.

A current performance comparison using buck build:
...

Test Plan: make check, unit tests updated
@facebook-github-bot
Copy link
Contributor

@pdillinger has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

Copy link
Contributor

@ajkr ajkr left a comment

Choose a reason for hiding this comment

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

LGTM

@facebook-github-bot
Copy link
Contributor

@pdillinger has updated the pull request. You must reimport the pull request before landing.

@facebook-github-bot
Copy link
Contributor

@pdillinger has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

romanz added a commit to romanz/rust-rocksdb that referenced this pull request Mar 11, 2023
Following facebook/rocksdb#10778, this change
will allow creating backward-compatible RocksDB tables (that can be read
using old version of RocksDB, without XXH3 support).
romanz added a commit to romanz/rust-rocksdb that referenced this pull request Mar 11, 2023
Following facebook/rocksdb#10778, this change
will allow creating backward-compatible RocksDB tables (that can be read
using older versions of RocksDB, without XXH3 support).
romanz added a commit to romanz/rust-rocksdb that referenced this pull request Apr 14, 2023
Following facebook/rocksdb#10778, this change
will allow creating backward-compatible RocksDB tables (that can be read
using old version of RocksDB, without XXH3 support).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants