adonis0147 commented on code in PR #18204:
URL: https://github.com/apache/doris/pull/18204#discussion_r1152672062


##########
be/src/util/jni-util.cpp:
##########
@@ -66,40 +66,88 @@ const std::string GetDorisJNIClasspath() {
     }
 }
 
-void FindOrCreateJavaVM() {
+// Only used on non-x86 platform
+[[maybe_unused]] void FindOrCreateJavaVM() {
     int num_vms;
-    int rv = LibJVMLoader::JNI_GetCreatedJavaVMs(&g_vm, 1, &num_vms);
+    int rv = JNI_GetCreatedJavaVMs(&g_vm, 1, &num_vms);
     if (rv == 0) {
+        JavaVMOption* options;
         auto classpath = GetDorisJNIClasspath();
-        std::string heap_size = fmt::format("-Xmx{}", 
config::jvm_max_heap_size);
-        std::string log_path = fmt::format("-DlogPath={}/log/udf-jdbc.log", 
getenv("DORIS_HOME"));
+        // The following 5 opts are default opts,
+        // they can be override by JAVA_OPTS env var.
+        std::string heap_size = fmt::format("-Xmx{}", "1g");
+        std::string log_path = fmt::format("-DlogPath={}/log/jni.log", 
getenv("DORIS_HOME"));
         std::string jvm_name = fmt::format("-Dsun.java.command={}", "DorisBE");
+        std::string critical_jni = "-XX:-CriticalJNINatives";
+        std::string max_fd_limit = "-XX:-MaxFDLimit";
 
-        JavaVMOption options[] = {
-                {const_cast<char*>(classpath.c_str()), nullptr},
-                {const_cast<char*>(heap_size.c_str()), nullptr},
-                {const_cast<char*>(log_path.c_str()), nullptr},
-                {const_cast<char*>(jvm_name.c_str()), nullptr},
+        char* java_opts = getenv("JAVA_OPTS");
+
+        int no_args;
+        if (java_opts == nullptr) {
+            no_args = 5; // classpath, heapsize, log path, jvm_name, critical
+#ifdef __APPLE__
+            no_args++; // -XX:-MaxFDLimit
+#endif
+            options = (JavaVMOption*)calloc(no_args, sizeof(JavaVMOption));
+            options[0].optionString = const_cast<char*>(classpath.c_str());
+            options[1].optionString = const_cast<char*>(heap_size.c_str());
+            options[2].optionString = const_cast<char*>(log_path.c_str());
+            options[3].optionString = const_cast<char*>(jvm_name.c_str());
+            options[4].optionString = const_cast<char*>(critical_jni.c_str());
 #ifdef __APPLE__
-                // On macOS, we should disable MaxFDLimit, otherwise the 
RLIMIT_NOFILE
-                // will be assigned the minimum of OPEN_MAX (10240) and 
rlim_cur (See src/hotspot/os/bsd/os_bsd.cpp)
-                // and it can not pass the check performed by storage engine.
-                // The newer JDK has fixed this issue.
-                {const_cast<char*>("-XX:-MaxFDLimit"), nullptr},
+            // On macOS, we should disable MaxFDLimit, otherwise the 
RLIMIT_NOFILE
+            // will be assigned the minimum of OPEN_MAX (10240) and rlim_cur 
(See src/hotspot/os/bsd/os_bsd.cpp)
+            // and it can not pass the check performed by storage engine.
+            // The newer JDK has fixed this issue.
+            options[5].optionString = const_cast<char*>(max_fd_limit.c_str());
 #endif
-        };
+        } else {
+            // user specified opts
+            // 1. find the number of args
+            java_opts = strdup(java_opts);
+            char *str, *token, *save_ptr;
+            char jvm_arg_delims[] = " ";
+            for (no_args = 1, str = java_opts;; no_args++, str = nullptr) {
+                token = strtok_r(str, jvm_arg_delims, &save_ptr);
+                if (token == nullptr) {
+                    break;
+                }
+            }
+            free(java_opts);
+            // 2. set args
+            options = (JavaVMOption*)calloc(no_args, sizeof(JavaVMOption));
+            options[0].optionString = const_cast<char*>(classpath.c_str());
+            java_opts = getenv("JAVA_OPTS");
+            if (java_opts != NULL) {
+                java_opts = strdup(java_opts);
+                for (no_args = 1, str = java_opts;; no_args++, str = nullptr) {
+                    token = strtok_r(str, jvm_arg_delims, &save_ptr);
+                    if (token == nullptr) {
+                        break;
+                    }
+                    options[no_args].optionString = token;
+                }
+            }
+        }
+
         JNIEnv* env;
         JavaVMInitArgs vm_args;
         vm_args.version = JNI_VERSION_1_8;
         vm_args.options = options;
-        vm_args.nOptions = sizeof(options) / sizeof(JavaVMOption);
+        vm_args.nOptions = no_args;
         // Set it to JNI_FALSE because JNI_TRUE will let JVM ignore the max 
size config.
         vm_args.ignoreUnrecognized = JNI_FALSE;
 
-        jint res = LibJVMLoader::JNI_CreateJavaVM(&g_vm, (void**)&env, 
&vm_args);
+        jint res = JNI_CreateJavaVM(&g_vm, (void**)&env, &vm_args);
         if (JNI_OK != res) {
             DCHECK(false) << "Failed to create JVM, code= " << res;
         }
+
+        if (java_opts != nullptr) {
+            free(java_opts);
+        }
+        free(options);

Review Comment:
   It's better to use RAII.



-- 
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: commits-unsubscr...@doris.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscr...@doris.apache.org
For additional commands, e-mail: commits-h...@doris.apache.org

Reply via email to