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.

Sign up for GitHub

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

Jump to bottom

RocksDB CRC32c optimization with ARMv8 Intrinsic #5221

Closed
wants to merge 1 commit into from
Closed

RocksDB CRC32c optimization with ARMv8 Intrinsic #5221

wants to merge 1 commit into from

Conversation

guyuqi
Copy link
Contributor

@guyuqi guyuqi commented Apr 19, 2019

  1. Add Arm linear crc32c implemtation for RocksDB.
  2. Arm runtime check for crc32

@guyuqi
RocksDB CRC32c optimization with ARMv8 Intrinsic
e9d1066 
Add Arm linear crc32c implemtation for RocksDB

Change-Id: I3a2174812d56a4d8400265bd2e3866bbf843e8a7
Signed-off-by: Yuqi Gu <yuqi.gu@arm.com>
facebook-github-bot
facebook-github-bot reviewed Apr 19, 2019
View reviewed changes
Copy link
Contributor

@facebook-github-bot facebook-github-bot left a comment

Choose a reason for hiding this comment

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

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

@siying
Copy link
Contributor

siying commented Apr 19, 2019

I don't have a platform where I can easily verify the CRC calculated is the same as other platforms. I just to make sure that you've run crc32_test on your platform and seen it passes.

@guyuqi
Copy link
Contributor Author

guyuqi commented Apr 24, 2019

@siying Sorry for late response.
crc32_test result:

builder@vm-rocksdb-yq:~/rocksdb$ arch
aarch64
builder@vm-rocksdb-yq:~/rocksdb$ ./crc32c_test
[==========] Running 4 tests from 1 test case.
[----------] Global test environment set-up.
[----------] 4 tests from CRC
[ RUN      ] CRC.StandardResults
[       OK ] CRC.StandardResults (2 ms)
[ RUN      ] CRC.Values
[       OK ] CRC.Values (0 ms)
[ RUN      ] CRC.Extend
[       OK ] CRC.Extend (0 ms)
[ RUN      ] CRC.Mask
[       OK ] CRC.Mask (0 ms)
[----------] 4 tests from CRC (2 ms total)

[----------] Global test environment tear-down
[==========] 4 tests from 1 test case ran. (2 ms total)
[  PASSED  ] 4 tests.

@facebook-github-bot
Copy link
Contributor

@siying merged this pull request in 03c7ae2.

vagogte pushed a commit to vagogte/rocksdb that referenced this pull request Jun 18, 2019
@guyuqi @facebook-github-bot
RocksDB CRC32c optimization with ARMv8 Intrinsic (facebook#5221)
513a7c4 
Summary:
1. Add Arm linear crc32c implemtation for RocksDB.
2. Arm runtime check for crc32
Pull Request resolved: facebook#5221

Differential Revision: D15013685

Pulled By: siying

fbshipit-source-id: 2c2983743d26656d93f212dc7c1a3cf66a1acf12
@siying siying mentioned this pull request Dec 16, 2019
Closed
adamretter pushed a commit that referenced this pull request Dec 20, 2019
@guyuqi @adamretter
RocksDB CRC32c optimization with ARMv8 Intrinsic (#5221)
a764ed0 
Summary:
1. Add Arm linear crc32c implemtation for RocksDB.
2. Arm runtime check for crc32
Pull Request resolved: #5221

Differential Revision: D15013685

Pulled By: siying

fbshipit-source-id: 2c2983743d26656d93f212dc7c1a3cf66a1acf12
siying pushed a commit that referenced this pull request Jan 6, 2020
@adamretter @guyuqi
@cnqpzhang @Yikun @siying
ARM64 commits to 5.18.3 to create 5.18.4 (#6250)
689c155 
* RocksDB CRC32c optimization with ARMv8 Intrinsic (#5221)

Summary:
1. Add Arm linear crc32c implemtation for RocksDB.
2. Arm runtime check for crc32
Pull Request resolved: #5221

Differential Revision: D15013685

Pulled By: siying

fbshipit-source-id: 2c2983743d26656d93f212dc7c1a3cf66a1acf12

* Support rocksdbjava aarch64 build and test (#5258)

Summary:
Verified with an Ampere Computing eMAG aarch64 system.
Pull Request resolved: #5258

Differential Revision: D15807309

Pulled By: maysamyabandeh

fbshipit-source-id: ab85d2fd3fe40e6094430ab0eba557b1e979510d

* Cleanup the Arm64 CRC32 unused warning (#5565)

Summary:
When 'HAVE_ARM64_CRC' is set, the blew methods:

- bool rocksdb::crc32c::isSSE42()
- bool rocksdb::crc32c::isPCLMULQDQ()

are defined but not used, the unused-function is raised
when do rocksdb build.

This patch try to cleanup these warnings by add ifndef,
if it build under the HAVE_ARM64_CRC, we will not define
`isSSE42` and `isPCLMULQDQ`.
Pull Request resolved: #5565

Differential Revision: D16233654

fbshipit-source-id: c32a9dda7465dbf65f9ccafef159124db92cdffd

* Fixes for building RocksJava releases on arm64v8

Summary: Pull Request resolved: #5674

Differential Revision: D16870338

fbshipit-source-id: c8dac644b1479fa734b491f3a8d50151772290f7

* Remove invalid comparison of va_list and nullptr (#5836)

Summary:
The comparison of va_list and nullptr is always False under any arch, and will raise invalid operands of types error in aarch64 env (`error: invalid operands of types ‘va_list {aka __va_list}’ and ‘std::nullptr_t’ to binary ‘operator!=’`).

This patch removes this invalid assert.

Closes: #4277
Pull Request resolved: #5836

Differential Revision: D17532470

fbshipit-source-id: ca98078ecbc6a9416c69de3bd6ffcfa33a0f0185

* Fix naming of library on PPC64LE (#6080)

Summary:
**NOTE**: This also needs to be back-ported to be 6.4.6

Fix a regression introduced in f2bf0b2 by #5674 whereby the compiled library would get the wrong name on PPC64LE platforms.

On PPC64LE, the regression caused the library to be named `librocksdbjni-linux64.so` instead of `librocksdbjni-linux-ppc64le.so`.

This PR corrects the name back to `librocksdbjni-linux-ppc64le.so` and also corrects the ordering of conditional arguments in the Makefile to match the expected order as defined in the documentation for Make.
Pull Request resolved: #6080

Differential Revision: D18710351

fbshipit-source-id: d4db87ef378263b57de7f9edce1b7d15644cf9de

Co-authored-by: Yuqi <yuqi.gu@arm.com>
Co-authored-by: Patrick Zhang <cnqpzhang@163.com>
Co-authored-by: Yikun Jiang <yikunkero@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@facebook-github-bot facebook-github-bot facebook-github-bot left review comments

Assignees
No one assigned
Labels
CLA Signed Merged
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@guyuqi @siying @facebook-github-bot