All jni_get_helpers MultiGetKeys methods static

So we have 2 “classes” which are nearer to namespaces for static helper methods: MultiGetKeys, and MultiGetValues
facebook · Sep 26, 2023 · 1f49666 · 1f49666
1 parent ab49157
commit 1f49666
Show file tree

Hide file tree

Showing 4 changed files with 24 additions and 69 deletions.
diff --git a/java/rocksjni/jni_get_helpers.cc b/java/rocksjni/jni_get_helpers.cc
@@ -107,27 +107,29 @@ jint rocksjni_get_helper(JNIEnv* env, const ROCKSDB_NAMESPACE::FnGet& fn_get,
   return pinnable_value_len;
 }
 
-bool MultiGetKeys::fromByteArrays(JNIEnv* env, jobjectArray jkeys,
-                                  jintArray jkey_offs, jintArray jkey_lens) {
+std::unique_ptr<std::vector<ROCKSDB_NAMESPACE::Slice>> MultiGetKeys::fromByteArrays(JNIEnv* env, jobjectArray jkeys, jintArray jkey_offs,
+                      jintArray jkey_lens) {
+
   const jsize num_keys = env->GetArrayLength(jkeys);
 
-  jint key_offs[num_keys];
-  env->GetIntArrayRegion(jkey_offs, 0, num_keys, key_offs);
+  std::unique_ptr<jint[]> key_offs = std::make_unique<jint[]>(num_keys);
+  env->GetIntArrayRegion(jkey_offs, 0, num_keys, key_offs.get());
   if (env->ExceptionCheck()) {
-    return false;  // exception thrown: ArrayIndexOutOfBoundsException
+    return nullptr;  // exception thrown: ArrayIndexOutOfBoundsException
   }
 
-  jint key_lens[num_keys];
-  env->GetIntArrayRegion(jkey_lens, 0, num_keys, key_lens);
+  std::unique_ptr<jint[]> key_lens = std::make_unique<jint[]>(num_keys);
+  env->GetIntArrayRegion(jkey_lens, 0, num_keys, key_lens.get());
   if (env->ExceptionCheck()) {
-    return false;  // exception thrown: ArrayIndexOutOfBoundsException
+    return nullptr;  // exception thrown: ArrayIndexOutOfBoundsException
   }
 
+  auto slices = std::make_unique<std::vector<ROCKSDB_NAMESPACE::Slice>>(0);
   for (jsize i = 0; i < num_keys; i++) {
     jobject jkey = env->GetObjectArrayElement(jkeys, i);
     if (env->ExceptionCheck()) {
       // exception thrown: ArrayIndexOutOfBoundsException
-      return false;
+      return nullptr;
     }
 
     jbyteArray jkey_ba = reinterpret_cast<jbyteArray>(jkey);
@@ -138,22 +140,16 @@ bool MultiGetKeys::fromByteArrays(JNIEnv* env, jobjectArray jkeys,
       // exception thrown: ArrayIndexOutOfBoundsException
       delete[] key;
       env->DeleteLocalRef(jkey);
-      return false;
+      return nullptr;
     }
 
-    keys.push_back(
+    slices->push_back(
         ROCKSDB_NAMESPACE::Slice(reinterpret_cast<char*>(key), len_key));
     env->DeleteLocalRef(jkey);
   }
-  return true;
+  return slices;
 }
 
-std::vector<ROCKSDB_NAMESPACE::Slice>& MultiGetKeys::vector() { return keys; }
-
-ROCKSDB_NAMESPACE::Slice* MultiGetKeys::array() { return keys.data(); }
-
-size_t MultiGetKeys::size() { return keys.size(); }
-
 template <class TValue>
 jobjectArray MultiGetValues::byteArrays(
     JNIEnv* env, std::vector<TValue>& values,

diff --git a/java/rocksjni/jni_get_helpers.h b/java/rocksjni/jni_get_helpers.h
@@ -36,18 +36,11 @@ jint rocksjni_get_helper(JNIEnv* env, const ROCKSDB_NAMESPACE::FnGet& fn_get,
  *
  */
 class MultiGetKeys {
- private:
-  std::vector<ROCKSDB_NAMESPACE::Slice> keys;
 
  public:
-  bool fromByteArrays(JNIEnv* env, jobjectArray jkeys, jintArray jkey_offs,
-                      jintArray jkey_lens);
+  static std::unique_ptr<std::vector<ROCKSDB_NAMESPACE::Slice>> fromByteArrays(JNIEnv* env, jobjectArray jkeys, jintArray jkey_offs,
+                      jintArray jkey_lens);                  
 
-  std::vector<ROCKSDB_NAMESPACE::Slice>& vector();
-
-  ROCKSDB_NAMESPACE::Slice* array();
-
-  size_t size();
 };
 
 /**

diff --git a/java/rocksjni/rocksjni.cc b/java/rocksjni/rocksjni.cc
@@ -1981,42 +1981,6 @@ void multi_get_helper_direct(JNIEnv* env, jobject, ROCKSDB_NAMESPACE::DB* db,
   env->SetIntArrayRegion(jvalue_sizes, 0, num_keys, value_size.data());
 }
 
-/*
- * Class:     org_rocksdb_RocksDB
- * Method:    multiGet
- * Signature: (J[[B[I[I)[[B
- */
-JNIEXPORT jobjectArray JNICALL Java_org_rocksdb_RocksDB_multiGetOld(
-    JNIEnv* env, jobject jdb, jlong jdb_handle, jobjectArray jkeys,
-    jintArray jkey_offs, jintArray jkey_lens) {
-  return multi_get_helper(
-      env, jdb, reinterpret_cast<ROCKSDB_NAMESPACE::DB*>(jdb_handle),
-      ROCKSDB_NAMESPACE::ReadOptions(), jkeys, jkey_offs, jkey_lens, nullptr);
-}
-
-/*
- * TODO (AP) - intermediate version
- *
- * Class:     org_rocksdb_RocksDB
- * Method:    multiGet
- * Signature: (J[[B[I[I)[[B
- */
-jobjectArray Java_org_rocksdb_RocksDB_multiGetIntermediate(
-    JNIEnv* env, jobject, jlong jdb_handle, jobjectArray jkeys,
-    jintArray jkey_offs, jintArray jkey_lens) {
-  ROCKSDB_NAMESPACE::MultiGetKeys keys;
-  if (!keys.fromByteArrays(env, jkeys, jkey_offs, jkey_lens)) {
-    return nullptr;
-  }
-  // TODO (AP) - use this in the efficient version -
-  // std::vector<ROCKSDB_NAMESPACE::PinnableSlice> values(keys.size());
-  std::vector<std::string> values;
-  auto* db = reinterpret_cast<ROCKSDB_NAMESPACE::DB*>(jdb_handle);
-  auto statuses =
-      db->MultiGet(ROCKSDB_NAMESPACE::ReadOptions(), keys.vector(), &values);
-  return ROCKSDB_NAMESPACE::MultiGetValues::byteArrays(env, values, statuses);
-}
-
 /*
  * Use the efficient/optimized variant of MultiGet()
  *
@@ -2027,15 +1991,16 @@ jobjectArray Java_org_rocksdb_RocksDB_multiGetIntermediate(
 jobjectArray Java_org_rocksdb_RocksDB_multiGet__J_3_3B_3I_3I(
     JNIEnv* env, jobject, jlong jdb_handle, jobjectArray jkeys,
     jintArray jkey_offs, jintArray jkey_lens) {
-  ROCKSDB_NAMESPACE::MultiGetKeys keys;
-  if (!keys.fromByteArrays(env, jkeys, jkey_offs, jkey_lens)) {
+
+  auto keys = ROCKSDB_NAMESPACE::MultiGetKeys::fromByteArrays(env, jkeys, jkey_offs, jkey_lens);
+  if (!keys) {
     return nullptr;
   }
-  std::vector<ROCKSDB_NAMESPACE::PinnableSlice> values(keys.size());
-  std::vector<ROCKSDB_NAMESPACE::Status> statuses(keys.size());
+  std::vector<ROCKSDB_NAMESPACE::PinnableSlice> values(keys->size());
+  std::vector<ROCKSDB_NAMESPACE::Status> statuses(keys->size());
   auto* db = reinterpret_cast<ROCKSDB_NAMESPACE::DB*>(jdb_handle);
   db->MultiGet(ROCKSDB_NAMESPACE::ReadOptions(), db->DefaultColumnFamily(),
-               keys.size(), keys.array(), values.data(), statuses.data(),
+               keys->size(), keys->data(), values.data(), statuses.data(),
                false /* sorted_input*/);
   return ROCKSDB_NAMESPACE::MultiGetValues::byteArrays(env, values, statuses);
 }

diff --git a/java/src/main/java/org/rocksdb/RocksDB.java b/java/src/main/java/org/rocksdb/RocksDB.java
@@ -2359,7 +2359,8 @@ public List<ByteBufferGetStatus> multiGetByteBuffers(final ReadOptions readOptio
     // Check if key size equals cfList size. If not a exception must be
     // thrown. If not a Segmentation fault happens.
     if (values.size() != keys.size()) {
-      throw new IllegalArgumentException("Mismatch " + keys.size() + " keys, but not the same as " + values.size() + " values.");
+      throw new IllegalArgumentException("For each key there must be a corresponding value. "
+          + keys.size() + " keys were supplied, but " + values.size() + " values were supplied.");
     }
 
     // TODO (AP) support indirect buffers