Copilot commented on code in PR #56763:
URL: https://github.com/apache/doris/pull/56763#discussion_r2489738387
##########
be/src/vec/exec/vjdbc_connector.cpp:
##########
@@ -97,80 +103,71 @@ Status JdbcConnector::open(RuntimeState* state, bool read)
{
}
JNIEnv* env = nullptr;
- RETURN_IF_ERROR(JniUtil::GetJNIEnv(&env));
- RETURN_IF_ERROR(JniUtil::get_jni_scanner_class(env,
JDBC_EXECUTOR_FACTORY_CLASS,
- &_executor_factory_clazz));
+ RETURN_IF_ERROR(Jni::Env::Get(&env));
- JNI_CALL_METHOD_CHECK_EXCEPTION(
- , _executor_factory_ctor_id, env,
- GetStaticMethodID(_executor_factory_clazz, "getExecutorClass",
-
"(Lorg/apache/doris/thrift/TOdbcTableType;)Ljava/lang/String;"));
+ // todo xxx
+ RETURN_IF_ERROR(Jni::Util::get_jni_scanner_class(env,
JDBC_EXECUTOR_FACTORY_CLASS,
+
&_executor_factory_clazz));
- jobject jtable_type = nullptr;
- RETURN_IF_ERROR(_get_java_table_type(env, _conn_param.table_type,
&jtable_type));
+ RETURN_IF_ERROR(_executor_factory_clazz.get_static_method(
Review Comment:
Commented-out old code at lines 187-189 and 203-205 should be removed to
improve code readability.
##########
be/test/util/jni_util_test.cpp:
##########
@@ -0,0 +1,682 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you 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.
+
+#include <gtest/gtest.h>
+
+#include <iomanip>
+#include <iostream>
+
+#include "util/jni-util.h"
+
+namespace doris {
+
+struct JavaMemInfo {
+ jlong used;
+ jlong free;
+ jlong total;
+ jlong max;
+};
+
+class JniUtilTest : public testing::Test {
+public:
+ JniUtilTest() {
+ _test = false;
+
+ // can not run as UT now, because ASAN will report memory leak.
+ //
https://blog.gypsyengineer.com/en/security/running-java-with-addresssanitizer.html
+ // <<How to use AddressSanitizer with Java>>
+ // should export ASAN_OPTIONS=detect_leaks=0;
+ if (!_test) {
+ return;
+ }
+
+ std::cout << "init = " << init();
+ }
+ ~JniUtilTest() override = default;
+
+ Status init() {
+ auto st = doris::Jni::Util::Init();
+
+ if (!st.ok()) {
+ exit(1);
+ }
+ JNIEnv* env;
+
+ // jvm mem
+ RETURN_IF_ERROR(Jni::Env::Get(&env));
+ RETURN_IF_ERROR(Jni::Util::find_class(env, "java/lang/Runtime",
&runtime_cls));
+
+ RETURN_IF_ERROR(runtime_cls.get_static_method(env, "getRuntime",
"()Ljava/lang/Runtime;",
+ &get_runtime));
+ RETURN_IF_ERROR(runtime_cls.get_method(env, "totalMemory", "()J",
&total_mem));
+ RETURN_IF_ERROR(runtime_cls.get_method(env, "freeMemory", "()J",
&free_mem));
+ RETURN_IF_ERROR(runtime_cls.get_method(env, "maxMemory", "()J",
&max_mem));
+
+ // gc
+ RETURN_IF_ERROR(Jni::Util::find_class(env, "java/lang/System",
&system_cls));
+ RETURN_IF_ERROR(system_cls.get_static_method(env, "gc", "()V",
&gc_method));
+
+ RETURN_IF_ERROR(trigger_gc(env));
+
+ return Status::OK();
+ }
+
+ Status get_mem_info(JNIEnv* env, JavaMemInfo* info) {
+ Jni::LocalObject runtime_obj;
+ RETURN_IF_ERROR(runtime_cls.call_static_object_method(env,
get_runtime).call(&runtime_obj));
+
+ jlong total, free, max, used;
+ RETURN_IF_ERROR(runtime_obj.call_long_method(env,
total_mem).call(&total));
+ RETURN_IF_ERROR(runtime_obj.call_long_method(env,
free_mem).call(&free));
+ RETURN_IF_ERROR(runtime_obj.call_long_method(env, max_mem).call(&max));
+ used = total - free;
+
+ info->used = used;
+ info->free = free;
+ info->total = total;
+ info->max = max;
+
+ return Status::OK();
+ }
+
+ void print_mem_info(const std::string& title, const JavaMemInfo& info) {
+ std::cout << std::fixed << std::setprecision(2);
+ std::cout << "==== " << title << " ====\n";
+ std::cout << "Used: " << (info.used / 1024.0 / 1024.0) << " MB\n";
+ std::cout << "Free: " << (info.free / 1024.0 / 1024.0) << " MB\n";
+ std::cout << "Total: " << (info.total / 1024.0 / 1024.0) << " MB\n";
+ std::cout << "Max: " << (info.max / 1024.0 / 1024.0) << " MB\n";
+ }
+
+ Status trigger_gc(JNIEnv* env) {
+ RETURN_IF_ERROR(system_cls.call_static_void_method(env,
gc_method).call());
+ return Status::OK();
+ }
+
+ bool check_mem_diff(const JavaMemInfo& before, const JavaMemInfo& after,
+ double allowed_diff_mb) {
+ auto to_mb = [](jlong bytes) { return bytes / 1024.0 / 1024.0; };
+
+ double used_before = to_mb(before.used);
+ double used_after = to_mb(after.used);
+
+ double diff = used_after - used_before;
+
+ std::cout << std::fixed << std::setprecision(2);
+ std::cout << "==== Memory Diff Check ====\n";
+ std::cout << "Used: " << used_before << " -> " << used_after << "
(Δ=" << diff << " MB)\n";
+
+ if (diff > allowed_diff_mb) {
+ std::cout << "Memory change " << diff << " MB exceeds allowed " <<
allowed_diff_mb
+ << " MB\n";
+ return false;
+ }
+
+ std::cout << "Memory change within limit (" << diff << " MB ≤ " <<
allowed_diff_mb
+ << " MB)\n";
+ return true;
+ }
+
+private:
+ Jni::GlobalClass runtime_cls;
+ Jni::GlobalClass system_cls;
+
+ Jni::MethodId get_runtime;
+ Jni::MethodId total_mem;
+ Jni::MethodId free_mem;
+ Jni::MethodId max_mem;
+
+ Jni::MethodId gc_method;
+
+ bool _test;
+};
+
+TEST_F(JniUtilTest, MemoryStableAfterStringAllocations) {
+ if (!_test) {
+ std::cout << "Skip MemoryStableAfterStringAllocations test\n";
+ return;
+ }
+ JNIEnv* env = nullptr;
+ auto st = Jni::Env::Get(&env);
+ EXPECT_TRUE(st.ok()) << st;
+
+ JavaMemInfo info_before;
+ st = get_mem_info(env, &info_before);
+ ASSERT_TRUE(st.ok()) << st;
+
+ print_mem_info("MemoryStableAfterStringAllocations_before", info_before);
+ for (int i = 0; i < 10000; i++) {
+ Jni::LocalClass string_cls;
+ st = Jni::Util::find_class(env, "java/lang/String", &string_cls);
+ ASSERT_TRUE(st.ok()) << st;
+
+ Jni::MethodId length_method;
+ st = string_cls.get_method(env, "length", "()I", &length_method);
+ ASSERT_TRUE(st.ok()) << st;
+
+ Jni::MethodId empty_method;
+ st = string_cls.get_method(env, "isEmpty", "()Z", &empty_method);
+ ASSERT_TRUE(st.ok()) << st;
+
+ Jni::LocalString jstr_obj;
+ st = Jni::LocalString::new_string(env, "hello", &jstr_obj);
+ ASSERT_TRUE(st.ok()) << st;
+
+ Jni::LocalStringBufferGuard str_buf;
+ st = jstr_obj.get_string_chars(env, &str_buf);
+ ASSERT_TRUE(st.ok()) << st;
+
+ ASSERT_EQ(strlen(str_buf.get()), 5);
+
+ ASSERT_EQ(strcmp(str_buf.get(), "hello"), 0);
+
+ jint len;
+ st = jstr_obj.call_int_method(env, length_method).call(&len);
+ ASSERT_TRUE(st.ok()) << st;
+ ASSERT_EQ(len, 5);
+
+ st = jstr_obj.call_int_method(env, length_method).call();
+ ASSERT_TRUE(st.ok()) << st;
+
+ jboolean empty;
+ st = jstr_obj.call_boolean_method(env, empty_method).call(&empty);
+ ASSERT_TRUE(st.ok()) << st;
+ ASSERT_EQ(empty, false);
+ }
+
+ JavaMemInfo info_after;
+ st = get_mem_info(env, &info_after);
+ ASSERT_TRUE(st.ok()) << st;
+ print_mem_info("MemoryStableAfterStringAllocations_after", info_after);
+
+ st = trigger_gc(env);
+ ASSERT_TRUE(st.ok()) << st;
+
+ JavaMemInfo info_after_gc;
+ st = get_mem_info(env, &info_after_gc);
+ ASSERT_TRUE(st.ok()) << st;
+ print_mem_info("MemoryStableAfterStringAllocations_after_gc",
info_after_gc);
+
+ ASSERT_TRUE(check_mem_diff(info_before, info_after_gc, 5.0));
+}
+
+TEST_F(JniUtilTest, ByteBuffer) {
+ if (!_test) {
+ std::cout << "Skip ByteBuffer test\n";
+ return;
+ }
+ JNIEnv* env = nullptr;
+ auto st = Jni::Env::Get(&env);
+ EXPECT_TRUE(st.ok()) << st;
+
+ JavaMemInfo info_before;
+ st = get_mem_info(env, &info_before);
+ ASSERT_TRUE(st.ok()) << st;
+ print_mem_info("ByteBuffer_before", info_before);
+
+ for (int i = 0; i < 300; i++) { // allocate 300M
+ Jni::LocalClass local_class;
+ st = Jni::Util::find_class(env, "java/nio/ByteBuffer", &local_class);
+ ASSERT_TRUE(st.ok()) << st;
+
+ Jni::MethodId allocate_method;
+ st = local_class.get_static_method(env, "allocate",
"(I)Ljava/nio/ByteBuffer;",
+ &allocate_method);
+ ASSERT_TRUE(st.ok()) << st;
+
+ Jni::LocalObject bytebuffer_obj;
+ st = local_class.call_static_object_method(env, allocate_method)
+ .with_arg(1024 * 1024)
+ .call(&bytebuffer_obj); // 1M
+
+ ASSERT_TRUE(st.ok()) << st;
+ }
+
+ JavaMemInfo info_after;
+ st = get_mem_info(env, &info_after);
+ ASSERT_TRUE(st.ok()) << st;
+ print_mem_info("ByteBuffer_after", info_after);
+
+ st = trigger_gc(env);
+ ASSERT_TRUE(st.ok()) << st;
+
+ JavaMemInfo info_after_gc;
+ st = get_mem_info(env, &info_after_gc);
+ ASSERT_TRUE(st.ok()) << st;
+ print_mem_info("ByteBuffer_after_gc", info_after_gc);
+
+ ASSERT_TRUE(check_mem_diff(info_before, info_after_gc, 5.0));
+}
+
+TEST_F(JniUtilTest, StringMethodTest) {
+ if (!_test) {
+ std::cout << "Skip StringMethodTest test\n";
+ return;
+ }
+ JNIEnv* env;
+ Status st = Jni::Env::Get(&env);
+ ASSERT_TRUE(st.ok()) << st;
+
+ JavaMemInfo info_before;
+ st = get_mem_info(env, &info_before);
+ ASSERT_TRUE(st.ok()) << st;
+ print_mem_info("String_before", info_before);
+
+ Jni::LocalClass string_cls;
+ st = Jni::Util::find_class(env, "java/lang/String", &string_cls);
+ ASSERT_TRUE(st.ok()) << st;
+
+ // get methodId
+ Jni::MethodId equals_method;
+ st = string_cls.get_method(env, "equals", "(Ljava/lang/Object;)Z",
&equals_method);
+ ASSERT_TRUE(st.ok()) << st;
+
+ Jni::MethodId equals_ignore_case_method;
+ st = string_cls.get_method(env, "equalsIgnoreCase",
"(Ljava/lang/String;)Z",
+ &equals_ignore_case_method);
+ ASSERT_TRUE(st.ok()) << st;
+
+ Jni::MethodId to_upper_method;
+ st = string_cls.get_method(env, "toUpperCase", "()Ljava/lang/String;",
&to_upper_method);
+ ASSERT_TRUE(st.ok()) << st;
+
+ Jni::MethodId to_lower_method;
+ st = string_cls.get_method(env, "toLowerCase", "()Ljava/lang/String;",
&to_lower_method);
+ ASSERT_TRUE(st.ok()) << st;
+
+ Jni::MethodId substring_method;
+ st = string_cls.get_method(env, "substring", "(II)Ljava/lang/String;",
&substring_method);
+ ASSERT_TRUE(st.ok()) << st;
+
+ Jni::MethodId contains_method;
+ st = string_cls.get_method(env, "contains", "(Ljava/lang/CharSequence;)Z",
&contains_method);
+ ASSERT_TRUE(st.ok()) << st;
+
+ Jni::MethodId starts_with_method;
+ st = string_cls.get_method(env, "startsWith", "(Ljava/lang/String;)Z",
&starts_with_method);
+ ASSERT_TRUE(st.ok()) << st;
+
+ Jni::MethodId ends_with_method;
+ st = string_cls.get_method(env, "endsWith", "(Ljava/lang/String;)Z",
&ends_with_method);
+ ASSERT_TRUE(st.ok()) << st;
+
+ // 3. create some local string object.
+ Jni::LocalString str_hello, str_hello2, str_hello_upper, str_world;
+ st = Jni::LocalString::new_string(env, "hello", &str_hello);
+ ASSERT_TRUE(st.ok()) << st;
+
+ st = Jni::LocalString::new_string(env, "HELLO", &str_hello_upper);
+ ASSERT_TRUE(st.ok()) << st;
+
+ st = Jni::LocalString::new_string(env, "hello", &str_hello2);
+ ASSERT_TRUE(st.ok()) << st;
+
+ st = Jni::LocalString::new_string(env, "world", &str_world);
+ ASSERT_TRUE(st.ok()) << st;
+
+ for (int i = 0; i < 10000; i++) {
+ // 4. equals() and equalsIgnoreCase()
+ {
+ jboolean eq;
+ st = str_hello.call_boolean_method(env,
equals_method).with_arg(str_hello2).call(&eq);
+ ASSERT_TRUE(st.ok()) << st;
+ ASSERT_EQ(eq, JNI_TRUE); // "hello".equals("hello")
+
+ jboolean eq_ic;
+ st = str_hello.call_boolean_method(env, equals_ignore_case_method)
+ .with_arg(str_hello_upper)
+ .call(&eq_ic);
+ ASSERT_TRUE(st.ok()) << st;
+ ASSERT_EQ(eq_ic, JNI_TRUE); // "hello".equalsIgnoreCase("HELLO")
+
+ jboolean neq;
+ st = str_hello.call_boolean_method(env,
equals_method).with_arg(str_world).call(&neq);
+ ASSERT_TRUE(st.ok()) << st;
+ ASSERT_EQ(neq, JNI_FALSE);
+ }
+
+ // 5. toUpperCase() / toLowerCase()
+ {
+ Jni::LocalString upper_obj;
+ st = str_hello.call_object_method(env,
to_upper_method).call(&upper_obj);
+ ASSERT_TRUE(st.ok()) << st;
+
+ Jni::LocalStringBufferGuard buf;
+ st = upper_obj.get_string_chars(env, &buf);
+ ASSERT_TRUE(st.ok()) << st;
+ ASSERT_STREQ(buf.get(), "HELLO");
+
+ Jni::LocalString lower_obj;
+ st = str_hello_upper.call_object_method(env,
to_lower_method).call(&lower_obj);
+ ASSERT_TRUE(st.ok()) << st;
+
+ Jni::LocalStringBufferGuard buf2;
+ st = lower_obj.get_string_chars(env, &buf2);
+ ASSERT_TRUE(st.ok()) << st;
+ ASSERT_STREQ(buf2.get(), "hello");
+ }
+
+ // 6. substring() 测试
Review Comment:
Chinese comment in English codebase. Should be: '// 6. substring() test'
```suggestion
// 6. substring() test
```
##########
be/src/vec/exprs/table_function/udf_table_function.cpp:
##########
@@ -117,8 +105,9 @@ Status UDFTableFunction::process_init(Block* block,
RuntimeState* state) {
{"required_fields", input_table_schema.first},
{"columns_types", input_table_schema.second}};
- jobject input_map = nullptr;
- RETURN_IF_ERROR(JniUtil::convert_to_java_map(env, input_params,
&input_map));
+ // jobject input_map = nullptr;
Review Comment:
Commented-out old code should be removed for cleaner codebase.
```suggestion
```
##########
be/src/util/jvm_metrics.cpp:
##########
@@ -520,73 +461,47 @@ Status JvmStats::refresh(JvmMetrics* jvm_metrics) const {
jvm_metrics->jvm_thread_terminated_count->set_value(threadsTerminated < 0
? 0
: threadsTerminated);
- JNI_CALL_METHOD_CHECK_EXCEPTION_DELETE_REF(
- jobject, gcMXBeansList, env,
- CallStaticObjectMethod(_managementFactoryClass,
_getGarbageCollectorMXBeansMethod));
-
- JNI_CALL_METHOD_CHECK_EXCEPTION(jint, numCollectors, env,
- CallIntMethod(gcMXBeansList,
_getListSizeMethod));
+ Jni::LocalObject gcMXBeansList;
+ RETURN_IF_ERROR(_managementFactoryClass
+ .call_static_object_method(env,
_getGarbageCollectorMXBeansMethod)
+ .call(&gcMXBeansList));
+ jint numCollectors = 0;
+ RETURN_IF_ERROR(gcMXBeansList.call_int_method(env,
_getListSizeMethod).call(&numCollectors));
for (int i = 0; i < numCollectors; i++) {
- JNI_CALL_METHOD_CHECK_EXCEPTION_DELETE_REF(
- jobject, gcMXBean, env, CallObjectMethod(gcMXBeansList,
_getListUseIndexMethod, i));
+ Jni::LocalObject gcMXBean;
+ RETURN_IF_ERROR(gcMXBeansList.call_object_method(env,
_getListUseIndexMethod)
+ .with_arg(i)
+ .call(&gcMXBean));
+
+ Jni::LocalString gcName;
+ RETURN_IF_ERROR(gcMXBeansList.call_object_method(env,
_getGCNameMethod).call(&gcName));
- JNI_CALL_METHOD_CHECK_EXCEPTION_DELETE_REF(jobject, gcName, env,
- CallObjectMethod(gcMXBean,
_getGCNameMethod));
+ jlong gcCollectionCount = 0;
+ RETURN_IF_ERROR(gcMXBeansList.call_long_method(env,
_getGCCollectionCountMethod)
+ .call(&gcCollectionCount));
- JNI_CALL_METHOD_CHECK_EXCEPTION(jlong, gcCollectionCount, env,
- CallLongMethod(gcMXBean,
_getGCCollectionCountMethod));
+ jlong gcCollectionTime = 0;
+ RETURN_IF_ERROR(gcMXBeansList.call_long_method(env,
_getGCCollectionTimeMethod)
+ .call(&gcCollectionTime));
- JNI_CALL_METHOD_CHECK_EXCEPTION(jlong, gcCollectionTime, env,
- CallLongMethod(gcMXBean,
_getGCCollectionTimeMethod));
+ Jni::LocalStringBufferGuard gcNameStr;
+ RETURN_IF_ERROR(gcName.get_string_chars(env, &gcNameStr));
- const char* gcNameStr = env->GetStringUTFChars((jstring)gcName, NULL);
- if (gcNameStr != nullptr) {
- if (strcmp(gcNameStr, "G1 Young Generation") == 0) {
+ if (gcNameStr.get() != nullptr) { // todo nulllptr in get_string_chars
??
Review Comment:
Typo in comment: 'nulllptr' should be 'nullptr'. Also, clarify the TODO
about when `get_string_chars` might return nullptr.
```suggestion
if (gcNameStr.get() != nullptr) { // TODO: get_string_chars may
return nullptr if the Java string is null or if a JNI error occurs.
```
##########
be/src/util/jvm_metrics.cpp:
##########
@@ -455,58 +388,66 @@ Status JvmStats::refresh(JvmMetrics* jvm_metrics) const {
jvm_metrics->jvm_old_size_bytes_peak_used->set_value(peakUsed
< 0 ? 0 : peakUsed);
jvm_metrics->jvm_old_size_bytes_max->set_value(max < 0 ? 0 :
max);
}
-
- env->ReleaseStringUTFChars((jstring)name,
- nameStr); // ReleaseStringUTFChars not
throw exception
}
}
- JNI_CALL_METHOD_CHECK_EXCEPTION_DELETE_REF(
- jobject, threadMXBean, env,
- CallStaticObjectMethod(_managementFactoryClass,
_getThreadMXBeanMethod));
- JNI_CALL_METHOD_CHECK_EXCEPTION_DELETE_REF(
- jobject, threadIdsObject, env, CallObjectMethod(threadMXBean,
_getAllThreadIdsMethod));
+ Jni::LocalObject threadMXBean;
+ RETURN_IF_ERROR(_managementFactoryClass.call_static_object_method(env,
_getThreadMXBeanMethod)
+ .call(&threadMXBean));
+
+ Jni::LocalArray threadIds;
+ RETURN_IF_ERROR(threadMXBean.call_object_method(env,
_getAllThreadIdsMethod).call(&threadIds));
- auto threadIds = (jlongArray)threadIdsObject;
+ // auto = (jlongArray)threadIdsObject;
+ // Jni::LocalArray threadIds;
Review Comment:
Commented-out old code at lines 401-402, 414-416 should be removed.
##########
be/src/vec/exec/vjdbc_connector.cpp:
##########
@@ -97,80 +103,71 @@ Status JdbcConnector::open(RuntimeState* state, bool read)
{
}
JNIEnv* env = nullptr;
- RETURN_IF_ERROR(JniUtil::GetJNIEnv(&env));
- RETURN_IF_ERROR(JniUtil::get_jni_scanner_class(env,
JDBC_EXECUTOR_FACTORY_CLASS,
- &_executor_factory_clazz));
+ RETURN_IF_ERROR(Jni::Env::Get(&env));
- JNI_CALL_METHOD_CHECK_EXCEPTION(
- , _executor_factory_ctor_id, env,
- GetStaticMethodID(_executor_factory_clazz, "getExecutorClass",
-
"(Lorg/apache/doris/thrift/TOdbcTableType;)Ljava/lang/String;"));
+ // todo xxx
Review Comment:
Incomplete TODO comment. Either complete the TODO or remove it if not needed.
```suggestion
```
##########
be/src/vec/exec/vjdbc_connector.cpp:
##########
@@ -632,17 +653,41 @@ Status JdbcConnector::_cast_string_to_json(const
SlotDescriptor* slot_desc, Bloc
return Status::OK();
}
+//Status JdbcConnector::_get_java_table_type(JNIEnv* env, TOdbcTableType::type
table_type,
+// jobject* java_enum_obj) {
+//
+// Jni::LocalClass enum_class;
+// RETURN_IF_ERROR(Jni::find_class(env,
"org/apache/doris/thrift/TOdbcTableType", &enum_class));
+//
+// Jni::MethodId find_by_value_method;
+// RETURN_IF_ERROR(enum_class.get_static_method(env, "findByValue",
"(I)Lorg/apache/doris/thrift/TOdbcTableType;", &find_by_value_method));
+////// jclass enum_class = env->FindClass();
+//// jmethodID find_by_value_method = env->GetStaticMethodID(
+//// enum_class,
+//
+// Jni::LocalObject java_enum_local_obj;
+// RETURN_IF_ERROR(enum_class.call_static_method(env,
find_by_value_method,&java_enum_local_obj,static_cast<jint>(table_type)));
+//
+// RETURN_ERROR_IF_EXC(env);
+// RETURN_IF_ERROR(JniUtil::LocalToGlobalRef(env, java_enum_local_obj,
java_enum_obj));
+// env->DeleteLocalRef(java_enum_local_obj);
+// return Status::OK();
+//}
+
Status JdbcConnector::_get_java_table_type(JNIEnv* env, TOdbcTableType::type
table_type,
- jobject* java_enum_obj) {
- jclass enum_class =
env->FindClass("org/apache/doris/thrift/TOdbcTableType");
- jmethodID find_by_value_method = env->GetStaticMethodID(
- enum_class, "findByValue",
"(I)Lorg/apache/doris/thrift/TOdbcTableType;");
- jobject java_enum_local_obj = env->CallStaticObjectMethod(enum_class,
find_by_value_method,
-
static_cast<jint>(table_type));
- RETURN_ERROR_IF_EXC(env);
- RETURN_IF_ERROR(JniUtil::LocalToGlobalRef(env, java_enum_local_obj,
java_enum_obj));
- env->DeleteLocalRef(java_enum_local_obj);
- return Status::OK();
+ Jni::LocalObject* java_enum_obj) {
+ Jni::LocalClass enum_class; // todo global
Review Comment:
Incomplete TODO comment. If `enum_class` should be Global instead of Local,
either fix it or provide more context about why it remains Local.
```suggestion
Jni::LocalClass enum_class; // Local reference is sufficient as
enum_class is only used within this method.
```
##########
be/src/vec/exec/vjdbc_connector.cpp:
##########
@@ -78,15 +78,21 @@ Status JdbcConnector::close(Status /*unused*/) {
RETURN_IF_ERROR(abort_trans());
}
JNIEnv* env = nullptr;
- RETURN_IF_ERROR(JniUtil::GetJNIEnv(&env));
- env->CallNonvirtualVoidMethod(_executor_obj, _executor_clazz,
_executor_close_id);
- RETURN_ERROR_IF_EXC(env);
- env->DeleteGlobalRef(_executor_factory_clazz);
- RETURN_ERROR_IF_EXC(env);
- env->DeleteGlobalRef(_executor_clazz);
- RETURN_IF_ERROR(JniUtil::GetJniExceptionMsg(env));
- env->DeleteGlobalRef(_executor_obj);
- RETURN_ERROR_IF_EXC(env);
+ RETURN_IF_ERROR(Jni::Env::Get(&env));
+
+ // RETURN_IF_ERROR
+ RETURN_IF_ERROR(
+ _executor_obj.call_nonvirtual_void_method(env, _executor_clazz,
_executor_close_id)
+ .call());
+
+ // env->CallNonvirtualVoidMethod(_executor_obj, _executor_clazz,
_executor_close_id); todo
+ // RETURN_ERROR_IF_EXC(env);
+ // env->DeleteGlobalRef(_executor_factory_clazz);
+ // RETURN_ERROR_IF_EXC(env);
+ // env->DeleteGlobalRef(_executor_clazz);
+ // RETURN_IF_ERROR(JniUtil::GetJniExceptionMsg(env));
+ // env->DeleteGlobalRef(_executor_obj);
+ // RETURN_ERROR_IF_EXC(env);
Review Comment:
Commented-out old code should be removed. These lines are unnecessary and
reduce code clarity.
```suggestion
RETURN_IF_ERROR(
_executor_obj.call_nonvirtual_void_method(env, _executor_clazz,
_executor_close_id)
.call());
```
##########
be/src/util/jvm_metrics.cpp:
##########
@@ -455,58 +388,66 @@ Status JvmStats::refresh(JvmMetrics* jvm_metrics) const {
jvm_metrics->jvm_old_size_bytes_peak_used->set_value(peakUsed
< 0 ? 0 : peakUsed);
jvm_metrics->jvm_old_size_bytes_max->set_value(max < 0 ? 0 :
max);
}
-
- env->ReleaseStringUTFChars((jstring)name,
- nameStr); // ReleaseStringUTFChars not
throw exception
}
}
- JNI_CALL_METHOD_CHECK_EXCEPTION_DELETE_REF(
- jobject, threadMXBean, env,
- CallStaticObjectMethod(_managementFactoryClass,
_getThreadMXBeanMethod));
- JNI_CALL_METHOD_CHECK_EXCEPTION_DELETE_REF(
- jobject, threadIdsObject, env, CallObjectMethod(threadMXBean,
_getAllThreadIdsMethod));
+ Jni::LocalObject threadMXBean;
+ RETURN_IF_ERROR(_managementFactoryClass.call_static_object_method(env,
_getThreadMXBeanMethod)
+ .call(&threadMXBean));
+
+ Jni::LocalArray threadIds;
+ RETURN_IF_ERROR(threadMXBean.call_object_method(env,
_getAllThreadIdsMethod).call(&threadIds));
- auto threadIds = (jlongArray)threadIdsObject;
+ // auto = (jlongArray)threadIdsObject;
+ // Jni::LocalArray threadIds;
+ jsize threadCount = 0;
+ RETURN_IF_ERROR(threadIds.get_length(env, &threadCount));
- JNI_CALL_METHOD_CHECK_EXCEPTION(jint, threadCount, env,
GetArrayLength(threadIds));
+ // JNI_CALL_METHOD_CHECK_EXCEPTION(jint, threadCount, env,
GetArrayLength(threadIds));
- JNI_CALL_METHOD_CHECK_EXCEPTION_DELETE_REF(
- jobject, threadInfos, env,
- CallObjectMethod(threadMXBean, _getThreadInfoMethod,
(jlongArray)threadIds, 0));
+ Jni::LocalArray threadInfos;
+ RETURN_IF_ERROR(threadMXBean.call_object_method(env, _getThreadInfoMethod)
+ .with_arg(threadIds)
+ .with_arg(0)
+ .call(&threadInfos));
+
+ // JNI_CALL_METHOD_CHECK_EXCEPTION_DELETE_REF(
+ // jobject, threadInfos, env,
+ // CallObjectMethod(threadMXBean, _getThreadInfoMethod,
(jlongArray)threadIds, 0));
int threadsNew = 0, threadsRunnable = 0, threadsBlocked = 0,
threadsWaiting = 0,
threadsTimedWaiting = 0, threadsTerminated = 0;
- JNI_CALL_METHOD_CHECK_EXCEPTION(jint, peakThreadCount, env,
- CallIntMethod(threadMXBean,
_getPeakThreadCountMethod));
+ jint peakThreadCount = 0;
+ RETURN_IF_ERROR(
+ threadMXBean.call_int_method(env,
_getPeakThreadCountMethod).call(&peakThreadCount));
jvm_metrics->jvm_thread_peak_count->set_value(peakThreadCount < 0 ? 0 :
peakThreadCount);
jvm_metrics->jvm_thread_count->set_value(threadCount < 0 ? 0 :
threadCount);
for (int i = 0; i < threadCount; i++) {
- JNI_CALL_METHOD_CHECK_EXCEPTION_DELETE_REF(
- jobject, threadInfo, env,
GetObjectArrayElement((jobjectArray)threadInfos, i));
-
- if (threadInfo == nullptr) {
+ Jni::LocalObject threadInfo;
+ RETURN_IF_ERROR(threadInfos.get_object_array_element(env, i,
&threadInfo));
+ if (threadInfo.uninitialized()) {
continue;
}
+ Jni::LocalObject threadState;
+ RETURN_IF_ERROR(
+ threadInfo.call_object_method(env,
_getThreadStateMethod).call(&threadState));
- JNI_CALL_METHOD_CHECK_EXCEPTION_DELETE_REF(
- jobject, threadState, env, CallObjectMethod(threadInfo,
_getThreadStateMethod));
-
- //IsSameObject not throw exception
- if (env->IsSameObject(threadState, _newThreadStateObj)) {
+ //IsSameObject not throw exception todo ??
Review Comment:
Unclear TODO comment. Clarify what needs to be verified about `IsSameObject`
exception behavior or remove if confirmed.
```suggestion
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
