From 772f739b4201ecbe5788e782b3062c52d2fed67b Mon Sep 17 00:00:00 2001 From: praveenbingo Date: Wed, 25 Jul 2018 09:08:32 +0530 Subject: [PATCH] GDV-93:[Java][C++]Fixed reference initializations. (#73) Class references are local by default and eligible for GC. We would need to convert it to global reference on library load for it to be safely used for the program lifetime. --- src/gandiva/src/cpp/src/jni/config_builder.cc | 18 +--- src/gandiva/src/cpp/src/jni/env_helper.h | 25 ++++++ src/gandiva/src/cpp/src/jni/native_builder.cc | 87 ++++++++++++------- 3 files changed, 85 insertions(+), 45 deletions(-) create mode 100644 src/gandiva/src/cpp/src/jni/env_helper.h diff --git a/src/gandiva/src/cpp/src/jni/config_builder.cc b/src/gandiva/src/cpp/src/jni/config_builder.cc index ead0a1a9d6067..c7fb5b5b0d3fc 100644 --- a/src/gandiva/src/cpp/src/jni/config_builder.cc +++ b/src/gandiva/src/cpp/src/jni/config_builder.cc @@ -15,15 +15,13 @@ #include "gandiva/configuration.h" #include "jni/config_holder.h" +#include "jni/env_helper.h" #include "jni/org_apache_arrow_gandiva_evaluator_ConfigurationBuilder.h" using gandiva::ConfigHolder; using gandiva::Configuration; using gandiva::ConfigurationBuilder; -jclass configuration_builder_class_; -jmethodID byte_code_accessor_method_id_; - /* * Class: org_apache_arrow_gandiva_evaluator_ConfigBuilder * Method: buildConfigInstance @@ -32,26 +30,16 @@ jmethodID byte_code_accessor_method_id_; JNIEXPORT jlong JNICALL Java_org_apache_arrow_gandiva_evaluator_ConfigurationBuilder_buildConfigInstance( JNIEnv *env, jobject configuration) { - if (configuration_builder_class_ == nullptr) { - const char class_name[] = "org/apache/arrow/gandiva/evaluator/ConfigurationBuilder"; - configuration_builder_class_ = env->FindClass(class_name); - } - - if (byte_code_accessor_method_id_ == nullptr) { - const char method_name[] = "getByteCodeFilePath"; - const char return_type[] = "()Ljava/lang/String;"; - byte_code_accessor_method_id_ = - env->GetMethodID(configuration_builder_class_, method_name, return_type); - } - jstring byte_code_file_path = (jstring)env->CallObjectMethod(configuration, byte_code_accessor_method_id_, 0); ConfigurationBuilder configuration_builder; if (byte_code_file_path != nullptr) { const char *byte_code_file_path_cpp = env->GetStringUTFChars(byte_code_file_path, 0); configuration_builder.set_byte_code_file_path(byte_code_file_path_cpp); + env->ReleaseStringUTFChars(byte_code_file_path, byte_code_file_path_cpp); } std::shared_ptr config = configuration_builder.build(); + env->DeleteLocalRef(byte_code_file_path); return ConfigHolder::MapInsert(config); } diff --git a/src/gandiva/src/cpp/src/jni/env_helper.h b/src/gandiva/src/cpp/src/jni/env_helper.h new file mode 100644 index 0000000000000..ba3093f2b2910 --- /dev/null +++ b/src/gandiva/src/cpp/src/jni/env_helper.h @@ -0,0 +1,25 @@ +// Copyright (C) 2017-2018 Dremio Corporation +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. +#ifndef ENV_HELPER_H +#define ENV_HELPER_H + +#include + +// class references +extern jclass configuration_builder_class_; + +// method references +extern jmethodID byte_code_accessor_method_id_; + +#endif // ENV_HELPER_H diff --git a/src/gandiva/src/cpp/src/jni/native_builder.cc b/src/gandiva/src/cpp/src/jni/native_builder.cc index 2449508cd63c7..686c61a5c01f1 100644 --- a/src/gandiva/src/cpp/src/jni/native_builder.cc +++ b/src/gandiva/src/cpp/src/jni/native_builder.cc @@ -29,6 +29,7 @@ #include "gandiva/configuration.h" #include "gandiva/tree_expr_builder.h" #include "jni/config_holder.h" +#include "jni/env_helper.h" #include "jni/module_holder.h" #include "jni/org_apache_arrow_gandiva_evaluator_NativeBuilder.h" @@ -61,8 +62,46 @@ std::mutex g_mtx_; // atomic counter for projector module ids jlong projector_module_id_(INIT_MODULE_ID); -// exception class -static jclass gandiva_exception_ = nullptr; +static jint JNI_VERSION = JNI_VERSION_1_6; + +// extern refs - initialized for other modules. +jclass configuration_builder_class_; +jmethodID byte_code_accessor_method_id_; + +// refs for self. +static jclass gandiva_exception_; + +jint JNI_OnLoad(JavaVM *vm, void *reserved) { + JNIEnv *env; + if (vm->GetEnv(reinterpret_cast(&env), JNI_VERSION) != JNI_OK) { + return JNI_ERR; + } + jclass local_configuration_builder_class_ = + env->FindClass("org/apache/arrow/gandiva/evaluator/ConfigurationBuilder"); + configuration_builder_class_ = + (jclass)env->NewGlobalRef(local_configuration_builder_class_); + env->DeleteLocalRef(local_configuration_builder_class_); + + jclass localExceptionClass = + env->FindClass("org/apache/arrow/gandiva/exceptions/GandivaException"); + gandiva_exception_ = (jclass)env->NewGlobalRef(localExceptionClass); + env->DeleteLocalRef(localExceptionClass); + + const char method_name[] = "getByteCodeFilePath"; + const char return_type[] = "()Ljava/lang/String;"; + byte_code_accessor_method_id_ = + env->GetMethodID(configuration_builder_class_, method_name, return_type); + env->ExceptionDescribe(); + + return JNI_VERSION; +} + +void JNI_OnUnload(JavaVM *vm, void *reserved) { + JNIEnv *env; + vm->GetEnv(reinterpret_cast(&env), JNI_VERSION); + env->DeleteGlobalRef(configuration_builder_class_); + env->DeleteGlobalRef(gandiva_exception_); +} jlong MapInsert(std::shared_ptr holder) { g_mtx_.lock(); @@ -406,21 +445,6 @@ bool ParseProtobuf(uint8_t *buf, int bufLen, google::protobuf::Message *msg) { return msg->ParseFromCodedStream(&cis); } -void ThrowException(JNIEnv *env, const std::string msg) { - if (gandiva_exception_ == nullptr) { - std::string className = "org/apache/arrow/gandiva/exceptions/GandivaException"; - gandiva_exception_ = env->FindClass(className.c_str()); - } - - if (gandiva_exception_ == nullptr) { - // Cannot find GandivaException class - // Cannot throw exception - return; - } - - env->ThrowNew(gandiva_exception_, msg.c_str()); -} - void releaseInput(jbyteArray schema_arr, jbyte *schema_bytes, jbyteArray exprs_arr, jbyte *exprs_bytes, JNIEnv *env) { env->ReleaseByteArrayElements(schema_arr, schema_bytes, JNI_ABORT); @@ -449,28 +473,30 @@ Java_org_apache_arrow_gandiva_evaluator_NativeBuilder_buildNativeCode( gandiva::Status status; std::shared_ptr config = ConfigHolder::MapLookup(configuration_id); + std::stringstream ss; if (config == nullptr) { - std::cerr << "configuration is mandatory."; + ss << "configuration is mandatory."; releaseInput(schema_arr, schema_bytes, exprs_arr, exprs_bytes, env); + goto err_out; } if (!ParseProtobuf(reinterpret_cast(schema_bytes), schema_len, &schema)) { - std::cerr << "Unable to parse schema protobuf\n"; + ss << "Unable to parse schema protobuf\n"; releaseInput(schema_arr, schema_bytes, exprs_arr, exprs_bytes, env); goto err_out; } if (!ParseProtobuf(reinterpret_cast(exprs_bytes), exprs_len, &exprs)) { releaseInput(schema_arr, schema_bytes, exprs_arr, exprs_bytes, env); - std::cerr << "Unable to parse expressions protobuf\n"; + ss << "Unable to parse expressions protobuf\n"; goto err_out; } // convert types::Schema to arrow::Schema schema_ptr = ProtoTypeToSchema(schema); if (schema_ptr == nullptr) { - std::cerr << "Unable to construct arrow schema object from schema protobuf\n"; + ss << "Unable to construct arrow schema object from schema protobuf\n"; releaseInput(schema_arr, schema_bytes, exprs_arr, exprs_bytes, env); goto err_out; } @@ -480,7 +506,7 @@ Java_org_apache_arrow_gandiva_evaluator_NativeBuilder_buildNativeCode( ExpressionPtr root = ProtoTypeToExpression(exprs.exprs(i)); if (root == nullptr) { - std::cerr << "Unable to construct expression object from expression protobuf\n"; + ss << "Unable to construct expression object from expression protobuf\n"; releaseInput(schema_arr, schema_bytes, exprs_arr, exprs_bytes, env); goto err_out; } @@ -493,7 +519,7 @@ Java_org_apache_arrow_gandiva_evaluator_NativeBuilder_buildNativeCode( status = Projector::Make(schema_ptr, expr_vector, nullptr, config, &projector); if (!status.ok()) { - std::cerr << "Failed to make LLVM module due to " << status.message() << "\n"; + ss << "Failed to make LLVM module due to " << status.message() << "\n"; releaseInput(schema_arr, schema_bytes, exprs_arr, exprs_bytes, env); goto err_out; } @@ -506,7 +532,7 @@ Java_org_apache_arrow_gandiva_evaluator_NativeBuilder_buildNativeCode( return module_id; err_out: - ThrowException(env, "Unable to create native module for the given expressions"); + env->ThrowNew(gandiva_exception_, ss.str().c_str()); return module_id; } @@ -528,20 +554,20 @@ JNIEXPORT void JNICALL Java_org_apache_arrow_gandiva_evaluator_NativeBuilder_eva gandiva::Status status; std::shared_ptr holder = MapLookup(module_id); if (holder == nullptr) { - std::cerr << "Unknown module id\n"; - ThrowException(env, "Unknown module id"); + env->ThrowNew(gandiva_exception_, "Unknown module id\n"); return; } int in_bufs_len = env->GetArrayLength(buf_addrs); if (in_bufs_len != env->GetArrayLength(buf_sizes)) { - ThrowException(env, "mismatch in arraylen of buf_addrs and buf_sizes"); + env->ThrowNew(gandiva_exception_, "mismatch in arraylen of buf_addrs and buf_sizes"); return; } int out_bufs_len = env->GetArrayLength(out_buf_addrs); if (out_bufs_len != env->GetArrayLength(out_buf_sizes)) { - ThrowException(env, "mismatch in arraylen of out_buf_addrs and out_buf_sizes"); + env->ThrowNew(gandiva_exception_, + "mismatch in arraylen of out_buf_addrs and out_buf_sizes"); return; } @@ -630,8 +656,9 @@ JNIEXPORT void JNICALL Java_org_apache_arrow_gandiva_evaluator_NativeBuilder_eva env->ReleaseLongArrayElements(out_buf_sizes, out_sizes, JNI_ABORT); if (!status.ok()) { - std::cerr << "Evaluate returned " << status.message() << "\n"; - ThrowException(env, status.message()); + std::stringstream ss; + ss << "Evaluate returned " << status.message() << "\n"; + env->ThrowNew(gandiva_exception_, status.message().c_str()); return; } }