Skip to content

Commit

Permalink
Reduce redundant code in CUDF JNI (#10019)
Browse files Browse the repository at this point in the history
This commit cleans up some of the redundancy in CUDF JNI native code, and modifies some of the `unique_ptr` handling:

### Improvements
1. Added utility function to convert from `native_jpointerArray<column_view>` to `vector<column_view>`. Removed redundant code in `stringConcat` functions.
2. Added utility to convert from `unique_ptr<column>` to `jlong`. This eliminates variables from nearly all the JNI functions.
3. Added utility functions for extracting `std::vector` from `native_jArray` classes. This reduces raw loops in `TableJni.cpp`.
4. Eliminated unnecessary use of `unique_ptr` in locations where they are immediately `release()`d. (E.g. `ColumnVectorJni::getNativeColumnView()`.)
5. Removed unnecessary `std::move()` calls.
6. Removed unnecessary `reinterpret_cast` (E.g. in `copyColumnViewToCV()`.)
7. `typedef` -> `using`.

Authors:
  - MithunR (https://github.com/mythrocks)

Approvers:
  - Robert (Bobby) Evans (https://github.com/revans2)
  - Jason Lowe (https://github.com/jlowe)

URL: #10019
  • Loading branch information
mythrocks authored Jan 25, 2022
1 parent baff5cf commit 22daaea
Show file tree
Hide file tree
Showing 4 changed files with 463 additions and 664 deletions.
93 changes: 87 additions & 6 deletions java/src/main/native/include/jni_utils.hpp
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright (c) 2019-2021, NVIDIA CORPORATION.
* Copyright (c) 2019-2022, NVIDIA CORPORATION.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
Expand All @@ -15,6 +15,7 @@
*/
#pragma once

#include <algorithm>
#include <memory>
#include <vector>

Expand Down Expand Up @@ -68,6 +69,32 @@ inline void check_java_exception(JNIEnv *const env) {
}
}

/**
* @brief Helper to convert a pointer to a jlong.
*
* This is useful when, for instance, converting a cudf::column pointer
* to a jlong, for use in JNI.
*/
template <typename T> jlong ptr_as_jlong(T *ptr) {
return reinterpret_cast<jlong>(ptr);
}

/**
* @brief Helper to release the data held by a unique_ptr, and return
* the pointer as a jlong.
*/
template <typename T> jlong release_as_jlong(std::unique_ptr<T> &&ptr) {
return ptr_as_jlong(ptr.release());
}

/**
* @brief Helper to release the data held by a unique_ptr, and return
* the pointer as a jlong.
*/
template <typename T> jlong release_as_jlong(std::unique_ptr<T> &ptr) {
return release_as_jlong(std::move(ptr));
}

class native_jdoubleArray_accessor {
public:
jdouble *getArrayElements(JNIEnv *const env, jdoubleArray arr) const {
Expand Down Expand Up @@ -256,6 +283,19 @@ template <typename N_TYPE, typename J_ARRAY_TYPE, typename ACCESSOR> class nativ

J_ARRAY_TYPE get_jArray() { return orig; }

/**
* @brief Conversion to std::vector
*
* @tparam target_t Target data type
* @return std::vector<target_t> Vector with the copied contents
*/
template <typename target_t = N_TYPE> std::vector<target_t> to_vector() const {
std::vector<target_t> ret;
ret.reserve(size());
std::copy(begin(), end(), std::back_inserter(ret));
return ret;
}

/**
* @brief if data has been written back into this array, don't commit
* it.
Expand All @@ -277,11 +317,34 @@ template <typename N_TYPE, typename J_ARRAY_TYPE, typename ACCESSOR> class nativ
~native_jArray() { commit(); }
};

typedef native_jArray<jdouble, jdoubleArray, native_jdoubleArray_accessor> native_jdoubleArray;
typedef native_jArray<jlong, jlongArray, native_jlongArray_accessor> native_jlongArray;
typedef native_jArray<jint, jintArray, native_jintArray_accessor> native_jintArray;
typedef native_jArray<jbyte, jbyteArray, native_jbyteArray_accessor> native_jbyteArray;
typedef native_jArray<jboolean, jbooleanArray, native_jbooleanArray_accessor> native_jbooleanArray;
using native_jdoubleArray = native_jArray<jdouble, jdoubleArray, native_jdoubleArray_accessor>;
using native_jlongArray = native_jArray<jlong, jlongArray, native_jlongArray_accessor>;
using native_jintArray = native_jArray<jint, jintArray, native_jintArray_accessor>;
using native_jbyteArray = native_jArray<jbyte, jbyteArray, native_jbyteArray_accessor>;

/**
* @brief Specialization of native_jArray for jboolean
*
* This class adds special support for conversion to std::vector<X>, where the element
* value is chosen depending on the jboolean value.
*/
struct native_jbooleanArray
: native_jArray<jboolean, jbooleanArray, native_jbooleanArray_accessor> {
native_jbooleanArray(JNIEnv *const env, jbooleanArray orig)
: native_jArray<jboolean, jbooleanArray, native_jbooleanArray_accessor>(env, orig) {}

native_jbooleanArray(native_jbooleanArray const &) = delete;
native_jbooleanArray &operator=(native_jbooleanArray const &) = delete;

template <typename target_t>
std::vector<target_t> transform_if_else(target_t const &if_true, target_t const &if_false) const {
std::vector<target_t> ret;
ret.reserve(size());
std::transform(begin(), end(), std::back_inserter(ret),
[&](jboolean const &b) { return b ? if_true : if_false; });
return ret;
}
};

/**
* @brief wrapper around native_jlongArray to make it take pointers instead.
Expand Down Expand Up @@ -336,6 +399,24 @@ template <typename T> class native_jpointerArray {

jlongArray get_jArray() { return wrapped.get_jArray(); }

void assert_no_nulls() const {
if (std::any_of(data(), data() + size(), [](T *const ptr) { return ptr == nullptr; })) {
throw_java_exception(env, NPE_CLASS, "pointer is NULL");
}
}

/**
* @brief Convert from `T*[]` to `vector<T>`.
*/
std::vector<T> get_dereferenced() const {
assert_no_nulls();
auto ret = std::vector<T>{};
ret.reserve(size());
std::transform(data(), data() + size(), std::back_inserter(ret),
[](T *const &p) { return *p; });
return ret;
}

/**
* @brief if data has been written back into this array, don't commit
* it.
Expand Down
Loading

0 comments on commit 22daaea

Please sign in to comment.