Add dh-defaults.vmoptions file (#3057)

Consolidates the recommended VM options into a single place. Allows for these options to be defined in a single place.

Moves dh-compiler-directives.txt and dh-defaults.vmoptions into the etc directory (instead of in lib alongside the jars).

Fixes #3054

Author: devinrsmith

buildSrc/src/main/groovy/io.deephaven.java-toolchain-conventions.gradle

@@ -92,16 +92,16 @@ tasks.withType(JavaCompile).configureEach {
 
 def createCompilerDirectives = tasks.register('createCompilerDirectives') {
   def compilerDirectivesFile = project.layout.buildDirectory.file('dh-compiler-directives.txt')
+  def compilerDirectivesText = new JsonBuilder([{
+    match (['*.*'] as List)
+    // Note: there seems to be a bug where this option doesn't actually get picked up
+    // So using '-XX:DisableIntrinsic=_currentThread' explicitly
+    // DisableIntrinsic('_currentThread')
+  }]).toPrettyString()
+  it.inputs.property('compilerDirectivesText', compilerDirectivesText)
   it.outputs.file(compilerDirectivesFile)
-
   doFirst {
-    def builder = new JsonBuilder([{
-      match (['*.*'] as List)
-      // Note: there seems to be a bug where this option doesn't actually get picked up
-      // So using '-XX:DisableIntrinsic=_currentThread' explicitly
-      // DisableIntrinsic('_currentThread')
-    }])
-    compilerDirectivesFile.get().asFile.text = builder.toPrettyString()
+    compilerDirectivesFile.get().asFile.text = compilerDirectivesText
   }
 }
 
@@ -123,30 +123,62 @@ def devJvmArgs = [
 //  '-XX:+PrintCompilation',          // this optional line shows jit operations as they happen
 ]
 
+// These are supposed to be generally applicable and recommended JVM options, but they aren't hard requirements.
+// Overly specific options do *not* belong here. For example, we should _not_ be setting something like `-Xmx4g` here.
+// If you are tempted to try and put system properties here (`-Dkey=value`), think again; there should be a more
+// appropriate place to set those.
+//
+// From the perspective of our application distribution, these options will be used as defaults for JAVA_OPTS
+// (if the user already has JAVA_OPTS set, these VM options will _not_ apply).
+def defaultJvmOptions = [
+  '-XX:+UseG1GC',
+  '-XX:MaxGCPauseMillis=100',
+  '-XX:+UseStringDeduplication',
+]
+
+def createVmOptions = tasks.register('createVmOptions') {
+  def vmOptionsFile = project.layout.buildDirectory.file('dh-default.vmoptions')
+  def vmOptionsText = defaultJvmOptions.join('\n')
+  it.inputs.property('vmOptionsText', vmOptionsText)
+  it.outputs.file(vmOptionsFile)
+  doFirst {
+    vmOptionsFile.get().asFile.text = vmOptionsText
+  }
+}
+
 tasks.withType(JavaExec).configureEach {
   def compilerDirectivesFile = createCompilerDirectives.get().outputs.files
+  def vmOptionsFile = createVmOptions.get().outputs.files
   inputs.files compilerDirectivesFile
+  inputs.files vmOptionsFile
   javaLauncher.set runtimeLauncher
-  jvmArgs += compilerArgs(compilerDirectivesFile.singleFile.path) + devJvmArgs
+  // Note: we _could_ have the vmOptionsFile constituents directly listed instead of using -XX:VMOptionsFile.
+  // That said, the current approach used here more closely matches how the application start script is defined.
+  jvmArgs += compilerArgs(compilerDirectivesFile.singleFile.path) + ["-XX:VMOptionsFile=${vmOptionsFile.singleFile.path}"] + devJvmArgs
 }
 
 tasks.withType(Test).configureEach {
   def compilerDirectivesFile = createCompilerDirectives.get().outputs.files
+  def vmOptionsFile = createVmOptions.get().outputs.files
   inputs.files compilerDirectivesFile
-
+  inputs.files vmOptionsFile
   javaLauncher.set testRuntimeLauncher
-  jvmArgs += compilerArgs(compilerDirectivesFile.singleFile.path) + devJvmArgs
+  // Note: we _could_ have the vmOptionsFile constituents directly listed instead of using -XX:VMOptionsFile.
+  // That said, the current approach used here more closely matches how the application start script is defined.
+  jvmArgs += compilerArgs(compilerDirectivesFile.singleFile.path) + ["-XX:VMOptionsFile=${vmOptionsFile.singleFile.path}"] + devJvmArgs
 }
 
 tasks.withType(GroovyCompile).configureEach {
   javaLauncher.set groovyCompilerLauncher
 }
 
 plugins.withType(ApplicationPlugin) {
-  applicationDistribution.into('lib') {
+  applicationDistribution.into('etc') {
     from(createCompilerDirectives.get().outputs.files)
+    from(createVmOptions.get().outputs.files)
   }
 }
+
 tasks.withType(CreateStartScripts).configureEach {
   def unixStartScript = resources.text.fromUri(getClass().classLoader.getResource('unixStartScript.txt'))
   inputs.files unixStartScript

buildSrc/src/main/resources/unixStartScript.txt

@@ -47,7 +47,13 @@ APP_BASE_NAME=`basename "\$0"`
 DEFAULT_JVM_OPTS=${defaultJvmOpts}
 
 # Customization for deephaven-core to reference a compiler directives file
-DEFAULT_JVM_OPTS="\${DEFAULT_JVM_OPTS} -XX:+UnlockDiagnosticVMOptions -XX:CompilerDirectivesFile=\"\${APP_HOME}/lib/dh-compiler-directives.txt\""
+# There is no easy way for users to override these options
+DEFAULT_JVM_OPTS="\${DEFAULT_JVM_OPTS} -XX:+UnlockDiagnosticVMOptions -XX:CompilerDirectivesFile=\"\${APP_HOME}/etc/dh-compiler-directives.txt\""
+
+# Customization for deephaven-core to include vm options file by default
+# If users explicitly set JAVA_OPTS, they will be opting out of dh-default.vmoptions
+DH_DEFAULT_VMOPTIONS="-XX:VMOptionsFile=\"\${APP_HOME}/etc/dh-default.vmoptions\""
+JAVA_OPTS="\${JAVA_OPTS:-\${DH_DEFAULT_VMOPTIONS}}"
 
 # Use the maximum available, or set MAX_FD != -1 to use that value.
 MAX_FD="maximum"

docker/server-jetty/src/main/docker/Dockerfile

@@ -32,8 +32,7 @@ HEALTHCHECK --interval=3s --retries=3 --timeout=11s CMD /bin/grpc_health_probe -
 ENV \
     DEEPHAVEN_CACHE_DIR="/cache" \
     DEEPHAVEN_CONFIG_DIR="/opt/deephaven/config" \
-    DEEPHAVEN_DATA_DIR="/data" \
-    JAVA_OPTS="-XX:+UseG1GC -XX:MaxGCPauseMillis=100 -XX:+UseStringDeduplication"
+    DEEPHAVEN_DATA_DIR="/data"
 ENTRYPOINT [ "/opt/deephaven/server/bin/start" ]
 ARG DEEPHAVEN_VERSION
 ARG SERVER

docker/server-slim/src/main/docker/Dockerfile

@@ -32,7 +32,7 @@ ENV \
     DEEPHAVEN_CACHE_DIR="/cache" \
     DEEPHAVEN_CONFIG_DIR="/opt/deephaven/config" \
     DEEPHAVEN_DATA_DIR="/data" \
-    JAVA_OPTS="-XX:+UseG1GC -XX:MaxGCPauseMillis=100 -XX:+UseStringDeduplication -Ddeephaven.console.type=groovy"
+    START_OPTS="-Ddeephaven.console.type=groovy"
 ENTRYPOINT [ "/opt/deephaven/server/bin/start" ]
 
 ARG DEEPHAVEN_VERSION

docker/server/src/main/docker/Dockerfile

@@ -32,8 +32,7 @@ HEALTHCHECK --interval=3s --retries=3 --timeout=11s CMD /bin/grpc_health_probe -
 ENV \
     DEEPHAVEN_CACHE_DIR="/cache" \
     DEEPHAVEN_CONFIG_DIR="/opt/deephaven/config" \
-    DEEPHAVEN_DATA_DIR="/data" \
-    JAVA_OPTS="-XX:+UseG1GC -XX:MaxGCPauseMillis=100 -XX:+UseStringDeduplication"
+    DEEPHAVEN_DATA_DIR="/data"
 ENTRYPOINT [ "/opt/deephaven/server/bin/start" ]
 ARG DEEPHAVEN_VERSION
 ARG SERVER

py/embedded-server/deephaven_server/start_jvm.py

@@ -17,6 +17,9 @@ def _jars_path():
 def _compiler_directives():
     return _jars_path() / 'dh-compiler-directives.txt'
 
+def _default_vmoptions():
+    return _jars_path() / 'dh-default.vmoptions'
+
 def _jars():
     return _jars_path().glob('*.jar')
 
@@ -42,6 +45,7 @@ def _jars():
     f"-XX:CompilerDirectivesFile={_compiler_directives()}",
     # (deephaven-core#2500): Remove DisableIntrinsic for currentThread
     '-XX:DisableIntrinsic=_currentThread',
+    f"-XX:VMOptionsFile={_default_vmoptions()}",
 ]
 
 # Provide a util func to start the JVM, will use its own defaults if none are offered

py/embedded-server/java-runtime/build.gradle

@@ -33,6 +33,7 @@ def serverClasspath = tasks.register('serverClasspath', Sync) {
     from configurations.runtimeClasspath
     from jar
     from tasks.named('createCompilerDirectives')
+    from tasks.named('createVmOptions')
     into layout.buildDirectory.dir('classpath')
 }
 

server/jetty-app/build.gradle

@@ -76,10 +76,6 @@ if (hasProperty('quiet')) {
 tasks.withType(JavaExec).configureEach {
     // This appends to the existing jvm args, so that java-open-nio still takes effect
     jvmArgs extraJvmArgs
-
-    jvmArgs '-XX:+UseG1GC',
-            '-XX:MaxGCPauseMillis=100',
-            '-XX:+UseStringDeduplication'
 }
 
 tasks.withType(CreateStartScripts).configureEach {

server/netty-app/build.gradle

@@ -76,12 +76,6 @@ if (hasProperty('quiet')) {
 tasks.withType(JavaExec).configureEach {
     // This appends to the existing jvm args, so that java-open-nio still takes effect
     jvmArgs extraJvmArgs
-
-    jvmArgs '-XX:+UseG1GC',
-            '-XX:MaxGCPauseMillis=100',
-            '-XX:+UseStringDeduplication',
-            // For development in this repository via Gradle, use the top-level data directory
-            "-Dstorage.path=${rootDir}/data"
 }
 
 tasks.withType(CreateStartScripts).configureEach {