-
Notifications
You must be signed in to change notification settings - Fork 6.7k
JNI Reader for Table Iterator #12511
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
base: main
Are you sure you want to change the base?
Changes from all commits
b547bb4
dcd5135
8c4ca27
f1be8b5
9700892
209f8fb
0bfa474
5fe932b
5370842
e0511d6
e567393
0569b4f
cb81a60
eade39d
5b5a25f
b192395
a0b6213
4a15702
e30392b
80dfbd6
130581f
fa151f1
9ea4270
5c743bc
973362e
0633d15
8a82a9e
45a22d3
daa0a89
26f9a50
21eca90
ccc6b59
48c829c
b754f11
8695829
f175bc4
d7d48d8
1587b6c
5c043bc
3a7bd0a
3157e52
7c0575c
2b426f2
52d6095
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,199 @@ | ||
| // Copyright (c) 2011-present, Facebook, Inc. All rights reserved. | ||
| // This source code is licensed under both the GPLv2 (found in the | ||
| // COPYING file in the root directory) and Apache 2.0 License | ||
| // (found in the LICENSE.Apache file in the root directory). | ||
| // | ||
| // This file implements the "bridge" between Java and C++ and enables | ||
| // calling c++ ROCKSDB_NAMESPACE::Iterator methods from Java side. | ||
|
|
||
| #include <jni.h> | ||
| #include <stdlib.h> | ||
|
|
||
| #include "include/org_rocksdb_ParsedEntryInfo.h" | ||
| #include "rocksdb/options.h" | ||
| #include "rocksdb/types.h" | ||
| #include "rocksdb/utilities/types_util.h" | ||
| #include "rocksjni/cplusplus_to_java_convert.h" | ||
| #include "rocksjni/portal.h" | ||
|
|
||
| /* | ||
| * Class: org_rocksdb_ParsedEntryInfo | ||
| * Method: newParseEntryInstance | ||
| * Signature: ()J | ||
| */ | ||
| jlong JNICALL Java_org_rocksdb_ParsedEntryInfo_newParseEntryInstance( | ||
| JNIEnv * /*env*/, jclass /*cls*/) { | ||
| ROCKSDB_NAMESPACE::ParsedEntryInfo *parsed_entry_info = | ||
| new ROCKSDB_NAMESPACE::ParsedEntryInfo(); | ||
| return GET_CPLUSPLUS_POINTER(parsed_entry_info); | ||
| } | ||
|
|
||
| /* | ||
| * Class: org_rocksdb_ParsedEntryInfo | ||
| * Method: parseEntry | ||
| * Signature: (JJ[BI)V | ||
| */ | ||
| void JNICALL Java_org_rocksdb_ParsedEntryInfo_parseEntry( | ||
| JNIEnv *env, jclass /*cls*/, jlong handle, jlong options_handle, | ||
| jbyteArray jtarget, jint len) { | ||
| auto *options = | ||
| reinterpret_cast<const ROCKSDB_NAMESPACE::Options *>(options_handle); | ||
| auto *parsed_entry_info = | ||
| reinterpret_cast<ROCKSDB_NAMESPACE::ParsedEntryInfo *>(handle); | ||
| jbyte* target = new jbyte[len]; | ||
| env->GetByteArrayRegion(jtarget, 0, len, target); | ||
| if (env->ExceptionCheck()) { | ||
| return; | ||
| } | ||
| ROCKSDB_NAMESPACE::Slice target_slice(reinterpret_cast<char *>(target), len); | ||
| ROCKSDB_NAMESPACE::Status s = ROCKSDB_NAMESPACE::ParseEntry(target_slice, options->comparator, | ||
| parsed_entry_info, true); | ||
| if (!s.ok()) { | ||
| ROCKSDB_NAMESPACE::RocksDBExceptionJni::ThrowNew(env, s); | ||
| } | ||
| } | ||
|
|
||
| /* | ||
| * Class: org_rocksdb_ParsedEntryInfo | ||
| * Method: parseEntryDirect | ||
| * Signature: (JJLjava/nio/ByteBuffer;II)V | ||
| */ | ||
| void JNICALL Java_org_rocksdb_ParsedEntryInfo_parseEntryDirect( | ||
| JNIEnv *env, jclass /*clz*/, jlong handle, jlong options_handle, | ||
| jobject jbuffer, jint jbuffer_off, jint jbuffer_len) { | ||
| auto *options = | ||
| reinterpret_cast<const ROCKSDB_NAMESPACE::Options *>(options_handle); | ||
| auto *parsed_entry_info = | ||
| reinterpret_cast<ROCKSDB_NAMESPACE::ParsedEntryInfo *>(handle); | ||
| auto parse = [&env, &parsed_entry_info, | ||
| &options](ROCKSDB_NAMESPACE::Slice &target_slice) { | ||
| ROCKSDB_NAMESPACE::Status s = ROCKSDB_NAMESPACE::ParseEntry(target_slice, options->comparator, | ||
| parsed_entry_info); | ||
| if (!s.ok()) { | ||
| ROCKSDB_NAMESPACE::RocksDBExceptionJni::ThrowNew(env, s); | ||
| return; | ||
| } | ||
| }; | ||
| ROCKSDB_NAMESPACE::JniUtil::k_op_direct(parse, env, jbuffer, jbuffer_off, | ||
| jbuffer_len); | ||
| } | ||
|
|
||
| /* | ||
| * Class: org_rocksdb_ParsedEntryInfo | ||
| * Method: parseEntryByteArray | ||
| * Signature: (JJ[BII)V | ||
| */ | ||
| void JNICALL Java_org_rocksdb_ParsedEntryInfo_parseEntryByteArray( | ||
| JNIEnv *env, jclass /*clz*/, jlong handle, jlong options_handle, | ||
| jbyteArray jtarget, jint joff, jint jlen) { | ||
| auto *options = | ||
| reinterpret_cast<const ROCKSDB_NAMESPACE::Options *>(options_handle); | ||
| auto *parsed_entry_info = | ||
| reinterpret_cast<ROCKSDB_NAMESPACE::ParsedEntryInfo *>(handle); | ||
| auto parse = [&env, &parsed_entry_info, | ||
| &options](ROCKSDB_NAMESPACE::Slice &target_slice) { | ||
| ROCKSDB_NAMESPACE::Status s = ROCKSDB_NAMESPACE::ParseEntry(target_slice, options->comparator, | ||
| parsed_entry_info); | ||
| if (!s.ok()) { | ||
| ROCKSDB_NAMESPACE::RocksDBExceptionJni::ThrowNew(env, s); | ||
| return true; | ||
| } | ||
| return false; | ||
| }; | ||
| ROCKSDB_NAMESPACE::JniUtil::k_op_indirect(parse, env, jtarget, joff, jlen); | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The slice data constructed by I also don't like the
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It would be another GC overhead on the jvm for each and every key we might end up creating an object which might be unoptimal if the number of SstFiles are going to be too many. With the byte buffer api we would have all the entries passed as it is. One possible way would be to have different byteBuffers being passed for different fields but this would mean we won't be able to reuse the SstFileReaderIterator object similar to the c++ implementation.
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I changed the k_op_indirect callback function to return a boolean value to indicate if the target array can be released. This way we can avoid unnecessary copies. |
||
| } | ||
|
|
||
| /* | ||
| * Class: org_rocksdb_ParsedEntryInfo | ||
| * Method: userKeyDirect | ||
| * Signature: (JLjava/nio/ByteBuffer;II)I | ||
| */ | ||
| jint JNICALL Java_org_rocksdb_ParsedEntryInfo_userKeyDirect( | ||
| JNIEnv *env, jclass /*clz*/, jlong handle, jobject jtarget, jint joffset, | ||
| jint jlen) { | ||
| auto *parsed_entry_info = | ||
| reinterpret_cast<ROCKSDB_NAMESPACE::ParsedEntryInfo *>(handle); | ||
| ROCKSDB_NAMESPACE::Slice key_slice = parsed_entry_info->user_key; | ||
| return ROCKSDB_NAMESPACE::JniUtil::copyToDirect(env, key_slice, jtarget, | ||
| joffset, jlen); | ||
| } | ||
|
|
||
| /* | ||
| * Class: org_rocksdb_ParsedEntryInfo | ||
| * Method: userKeyByteArray | ||
| * Signature: (J[BII)I | ||
| */ | ||
| jint JNICALL Java_org_rocksdb_ParsedEntryInfo_userKeyByteArray( | ||
| JNIEnv *env, jclass /*clz*/, jlong handle, jbyteArray jtarget, jint joffset, | ||
| jint jlen) { | ||
| auto *parsed_entry_info = | ||
| reinterpret_cast<ROCKSDB_NAMESPACE::ParsedEntryInfo *>(handle); | ||
| ROCKSDB_NAMESPACE::Slice key_slice = parsed_entry_info->user_key; | ||
| return ROCKSDB_NAMESPACE::JniUtil::copyBytes(env, key_slice, jtarget, joffset, | ||
| jlen); | ||
| } | ||
|
|
||
| /* | ||
| * Class: org_rocksdb_ParsedEntryInfo | ||
| * Method: userKeyJni | ||
| * Signature: (J)[B | ||
| */ | ||
| jbyteArray JNICALL Java_org_rocksdb_ParsedEntryInfo_userKeyJni(JNIEnv *env, | ||
| jclass /*clz*/, | ||
| jlong handle) { | ||
| auto *parsed_entry_info = | ||
| reinterpret_cast<ROCKSDB_NAMESPACE::ParsedEntryInfo *>(handle); | ||
| ROCKSDB_NAMESPACE::Slice key_slice = parsed_entry_info->user_key; | ||
| jbyteArray jkey = env->NewByteArray(static_cast<jsize>(key_slice.size())); | ||
| if (jkey == nullptr) { | ||
| // exception thrown: OutOfMemoryError | ||
| ROCKSDB_NAMESPACE::OutOfMemoryErrorJni::ThrowNew(env, "Memory allocation failed in RocksDB JNI function"); | ||
| return nullptr; | ||
| } | ||
| ROCKSDB_NAMESPACE::JniUtil::copyBytes(env, key_slice, jkey, 0, | ||
| static_cast<jint>(key_slice.size())); | ||
| return jkey; | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Missing an error check to see if
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. added a check in copyBytes function |
||
| } | ||
|
|
||
| /* | ||
| * Class: org_rocksdb_ParsedEntryInfo | ||
| * Method: getSequenceNumberJni | ||
| * Signature: (J)J | ||
| */ | ||
| jlong JNICALL Java_org_rocksdb_ParsedEntryInfo_getSequenceNumberJni( | ||
| JNIEnv * /*env*/, jclass /*clz*/, jlong handle) { | ||
| auto *parsed_entry_info = | ||
| reinterpret_cast<ROCKSDB_NAMESPACE::ParsedEntryInfo *>(handle); | ||
| uint64_t sequence_number = parsed_entry_info->sequence; | ||
| return static_cast<jlong>(sequence_number); | ||
| } | ||
|
|
||
| /* | ||
| * Class: org_rocksdb_ParsedEntryInfo | ||
| * Method: getValueTypeJni | ||
| * Signature: (J)B | ||
| */ | ||
| jbyte JNICALL Java_org_rocksdb_ParsedEntryInfo_getEntryTypeJni(JNIEnv * /*env*/, | ||
| jclass /*clz*/, | ||
| jlong handle) { | ||
| auto *parsed_entry_info = | ||
| reinterpret_cast<ROCKSDB_NAMESPACE::ParsedEntryInfo *>(handle); | ||
| ROCKSDB_NAMESPACE::EntryType type = parsed_entry_info->type; | ||
| return ROCKSDB_NAMESPACE::EntryTypeJni::toJavaEntryType(type); | ||
| } | ||
|
|
||
| /* | ||
| * Class: org_rocksdb_ParsedEntryInfo | ||
| * Method: disposeInternalJni | ||
| * Signature: (J)V | ||
| */ | ||
| void JNICALL Java_org_rocksdb_ParsedEntryInfo_disposeInternalJni( | ||
| JNIEnv * /*env*/, jclass /*clz*/, jlong handle) { | ||
| auto *parsed_entry_info = | ||
| reinterpret_cast<ROCKSDB_NAMESPACE::ParsedEntryInfo *>(handle); | ||
| assert(parsed_entry_info != nullptr); | ||
| if (parsed_entry_info->copied_user_key) { | ||
| delete parsed_entry_info->user_key.data(); | ||
| } | ||
| delete parsed_entry_info; | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -24,6 +24,7 @@ | |
| #include <type_traits> | ||
| #include <vector> | ||
|
|
||
| #include "db/dbformat.h" | ||
| #include "rocksdb/convenience.h" | ||
| #include "rocksdb/db.h" | ||
| #include "rocksdb/filter_policy.h" | ||
|
|
@@ -2477,6 +2478,42 @@ class JniUtil { | |
| return op(key_slice); | ||
| } | ||
|
|
||
| /* | ||
| * Helper for operations on a key on java byte array | ||
| * Copies values from jbyte array to slice and performs op on the slice. | ||
| * for example WriteBatch->Delete | ||
| * | ||
| * from `op` and used for RocksDB->Delete etc. | ||
| */ | ||
| static void k_op_indirect(std::function<bool(ROCKSDB_NAMESPACE::Slice&)> op, | ||
| JNIEnv* env, jbyteArray jkey, jint jkey_off, | ||
| jint jkey_len) { | ||
| if (jkey == nullptr || env->GetArrayLength(jkey) < (jkey_off + jkey_len)) { | ||
| ROCKSDB_NAMESPACE::RocksDBExceptionJni::ThrowNew(env, | ||
| "Invalid key argument"); | ||
| return; | ||
| } | ||
| char* target = new char[jkey_len]; | ||
| if (target == nullptr) { | ||
| ROCKSDB_NAMESPACE::OutOfMemoryErrorJni::ThrowNew(env, | ||
| "Memory allocation failed in RocksDB JNI function"); | ||
| return; | ||
| } | ||
| env->GetByteArrayRegion(jkey, jkey_off, jkey_len, | ||
| reinterpret_cast<jbyte*>(target)); | ||
| if (env->ExceptionCheck()) { | ||
| // exception thrown: ArrayIndexOutOfBoundsException | ||
| ROCKSDB_NAMESPACE::IllegalArgumentExceptionJni::ThrowNew(env, | ||
| "Failed to get byte array region: Out of bounds or invalid array."); | ||
| return; | ||
| } | ||
| ROCKSDB_NAMESPACE::Slice target_slice(target, jkey_len); | ||
| bool release_copy = op(target_slice); | ||
| if (release_copy) { | ||
| delete[] target; | ||
| } | ||
| } | ||
|
|
||
| template <class T> | ||
| static jint copyToDirect(JNIEnv* env, T& source, jobject jtarget, | ||
| jint jtarget_off, jint jtarget_len) { | ||
|
|
@@ -2498,6 +2535,31 @@ class JniUtil { | |
|
|
||
| return cvalue_len; | ||
| } | ||
|
|
||
| /* Helper for copying value in source into a byte array. | ||
| */ | ||
| template <class T> | ||
swamirishi marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| static jint copyBytes(JNIEnv* env, T& source, jbyteArray jtarget, | ||
| jint jtarget_off, jint jtarget_len) { | ||
| if (jtarget == nullptr || | ||
| env->GetArrayLength(jtarget) < (jtarget_off + jtarget_len)) { | ||
| ROCKSDB_NAMESPACE::RocksDBExceptionJni::ThrowNew( | ||
| env, "Invalid target argument"); | ||
| return 0; | ||
| } | ||
|
|
||
| const jint cvalue_len = static_cast<jint>(source.size()); | ||
| const jint length = std::min(jtarget_len, cvalue_len); | ||
| env->SetByteArrayRegion( | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Missing error check - if
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. done |
||
| jtarget, jtarget_off, length, | ||
| const_cast<jbyte*>(reinterpret_cast<const jbyte*>(source.data()))); | ||
| if (env->ExceptionCheck()) { | ||
| ROCKSDB_NAMESPACE::RocksDBExceptionJni::ThrowNew( | ||
| env, "Failed to copy data to byte array."); | ||
| return 0; | ||
| } | ||
| return cvalue_len; | ||
| } | ||
| }; | ||
|
|
||
| class MapJni : public JavaClass { | ||
|
|
@@ -6444,6 +6506,35 @@ class TxnDBWritePolicyJni { | |
| } | ||
| }; | ||
|
|
||
| // The portal class for org.rocksdb.EntryType | ||
| class EntryTypeJni { | ||
| public: | ||
| static jbyte toJavaEntryType(const ROCKSDB_NAMESPACE::EntryType& entry_type) { | ||
| switch (entry_type) { | ||
| case ROCKSDB_NAMESPACE::EntryType::kEntryPut: | ||
| return 0x0; | ||
| case ROCKSDB_NAMESPACE::EntryType::kEntryDelete: | ||
| return 0x1; | ||
| case ROCKSDB_NAMESPACE::EntryType::kEntrySingleDelete: | ||
| return 0x2; | ||
| case ROCKSDB_NAMESPACE::EntryType::kEntryMerge: | ||
| return 0x3; | ||
| case ROCKSDB_NAMESPACE::EntryType::kEntryRangeDeletion: | ||
| return 0x4; | ||
| case ROCKSDB_NAMESPACE::EntryType::kEntryBlobIndex: | ||
| return 0x5; | ||
| case ROCKSDB_NAMESPACE::EntryType::kEntryDeleteWithTimestamp: | ||
| return 0x6; | ||
| case ROCKSDB_NAMESPACE::EntryType::kEntryWideColumnEntity: | ||
| return 0x7; | ||
| case ROCKSDB_NAMESPACE::EntryType::kEntryOther: | ||
| return 0x8; | ||
| default: | ||
| return 0x9; | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There does not appear to be a
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. yeah
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I feel uncomfortable with this approach that raises an exception for an invalid enum value. Is there a better approach perhaps?
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Added an invalid enum on java side to handle this default value. |
||
| } | ||
| } | ||
| }; | ||
|
|
||
| // The portal class for org.rocksdb.TransactionDB.KeyLockInfo | ||
| class KeyLockInfoJni : public JavaClass { | ||
| public: | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should be
charCopyand nottargetintarget_slice(...I think.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Changed the implementation to copy directly to target by using GetByteArrayRegion to avoid unnecessary copy from GetByteArray and release since we are going to be copy anyhow at the end.