-
Notifications
You must be signed in to change notification settings - Fork 6.4k
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
ARM64 commits to 5.18.3 to create 5.18.4 #6250
Conversation
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
Summary: Verified with an Ampere Computing eMAG aarch64 system. Pull Request resolved: facebook#5258 Differential Revision: D15807309 Pulled By: maysamyabandeh fbshipit-source-id: ab85d2fd3fe40e6094430ab0eba557b1e979510d
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: facebook#5565 Differential Revision: D16233654 fbshipit-source-id: c32a9dda7465dbf65f9ccafef159124db92cdffd
Summary: Pull Request resolved: facebook#5674 Differential Revision: D16870338 fbshipit-source-id: c8dac644b1479fa734b491f3a8d50151772290f7
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: facebook#4277 Pull Request resolved: facebook#5836 Differential Revision: D17532470 fbshipit-source-id: ca98078ecbc6a9416c69de3bd6ffcfa33a0f0185
Summary: **NOTE**: This also needs to be back-ported to be 6.4.6 Fix a regression introduced in f2bf0b2 by facebook#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: facebook#6080 Differential Revision: D18710351 fbshipit-source-id: d4db87ef378263b57de7f9edce1b7d15644cf9de
@adamretter |
@jiameixe I think there is an issue still on Arm that I made @siying aware of. I was waiting to hear from him on that. After that, once FB tag the release I can get it up onto Maven Central |
@adamretter I can confirm that it is a bug somewhere, but not related to ARM. It's just somehow got triggered because the ARM environment. But it's bug is in core RocksDB, which we need to spend time to figure out. But the release should be good to go. |
@siying okie dokie. Can you create the v5.18.4 tag please? I don't have permission :-( |
@adamretter @siying Have v5.18.4 been released? I couldn't find it. |
Hi @siying, could you please help to create the v5.18.4 tag? Thank you very much! |
One thing I am worried is that if I tagged it, will it become "Latest Release" in github? |
Hi @siying, could you kindly try to do :
|
OK. Now I tagged it. |
@siying I am not sure what happened but the tag looks wrong :-/ If you compare:
You will see that the branch has the correct ARM64 commits at its head, but the tag does not. Instead the tag has an additional commit which hasn't come from the branch Also the file @siying could you take a look please? |
@siying could you take a look please? |
@adamretter Should be fixed now. https://github.com/facebook/rocksdb/releases/tag/v5.18.4 Read the note about updated tags. |
@pdillinger @adamretter @siying Has it been published to maven central? I can't find this in https://mvnrepository.com/artifact/org.rocksdb/rocksdbjni. |
@jiameixie I am waiting on this one at the moment - #6497 |
@jiameixie I just released 5.18.4 to Maven Central (it might take up to 2 hours to appear) |
@adamretter That's great! Thanks for your effort. |
Cool, greate works, and I will also bring this rocksdbjni version to Apache Storm community (Bump the rocksdb from 5.18.3 to 5.18.4) [1]. : ) And looks like the v5.18.4 is still on the road, it doesn't appear in [2] yet. [1] https://issues.apache.org/jira/browse/STORM-3599 |
@Yikun mvnrepository is only an aggregator. Take a look in Maven Central which is cannonical for RocksJava |
Current version of leveldbjni dependency cannot support ARM64 platform, this change bump the version to 5.18.4 to support ARM64 platform. A special rocksdbjni version for aarch64 has been released by rocksdb community. [1][2] [1] https://github.com/facebook/rocksdb/releases/tag/v5.18.4 [2] facebook/rocksdb#6250 Fix #12818 pr-link: #12819 change-id: cid-a6934d0fcfa4e439a57b2129c942a07e45492c6b
In response to #6188
As requested by @siying