Passing JPS process JVM memory options to daemon, refactoring options processing

compiler/rmi/kotlinr/src/org/jetbrains/kotlin/rmi/kotlinr/KotlinCompilerClient.kt

@@ -31,6 +31,7 @@ import kotlin.concurrent.read
 import kotlin.concurrent.thread
 import kotlin.concurrent.write
 import kotlin.platform.platformStatic
+import kotlin.reflect.KProperty1
 
 fun Process.isAlive() =
         try {
@@ -79,10 +80,10 @@ public class KotlinCompilerClient {
             // TODO: add some specific environment variables to the cp and may be command line, to allow some specific startup configs
             val args = listOf(javaExecutable.absolutePath,
                               "-cp", compilerId.compilerClasspath.joinToString(File.pathSeparator)) +
-                       daemonLaunchingOptions.jvmParams +
+                       daemonLaunchingOptions.extractors.flatMap { it.extract("-") } +
                        COMPILER_DAEMON_CLASS_FQN +
-                       daemonOptions.asParams +
-                       compilerId.asParams
+                       daemonOptions.extractors.flatMap { it.extract(COMPILE_DAEMON_CMDLINE_OPTIONS_PREFIX) } +
+                       compilerId.extractors.flatMap { it.extract(COMPILE_DAEMON_CMDLINE_OPTIONS_PREFIX) }
             errStream.println("[daemon client] starting the daemon as: " + args.joinToString(" "))
             val processBuilder = ProcessBuilder(args).redirectErrorStream(true)
             // assuming daemon process is deaf and (mostly) silent, so do not handle streams
@@ -141,7 +142,7 @@ public class KotlinCompilerClient {
                     errStream.println("[daemon client] found the suitable daemon")
                     return service
                 }
-                errStream.println("[daemon client] compiler identity don't match: " + compilerId.asParams.joinToString(" "))
+                errStream.println("[daemon client] compiler identity don't match: " + compilerId.extractors.flatMap { it.extract("") }.joinToString(" "))
                 if (!autostart) return null;
                 errStream.println("[daemon client] shutdown the daemon")
                 service.shutdown()
@@ -193,21 +194,11 @@ public class KotlinCompilerClient {
             }
         }
 
-        public fun isDaemonEnabled(): Boolean = System.getProperty(COMPILE_DAEMON_ENABLED_PROPERTY) != null
-
-        public fun configureDaemonLaunchingOptions(opts: DaemonLaunchingOptions) {
-            System.getProperty(COMPILE_DAEMON_JVM_OPTIONS_PROPERTY)?.let {
-                // TODO: find better way to pass and parse jvm options for daemon
-                opts.jvmParams = it.split("##")
-            }
-        }
-
         data class ClientOptions(
                 public var stop: Boolean = false
         ) :CmdlineParams {
-            override val asParams: Iterable<String>
-                get() =
-                    if (stop) listOf("stop") else listOf()
+            override val extractors: List<PropExtractor<*, *, *>>
+                get() = listOf( BoolPropExtractor(this, ::stop))
 
             override val parsers: List<PropParser<*,*,*>>
                 get() = listOf( BoolPropParser(this, ::stop))
@@ -218,7 +209,7 @@ public class KotlinCompilerClient {
             val daemonOptions = DaemonOptions()
             val daemonLaunchingOptions = DaemonLaunchingOptions()
             val clientOptions = ClientOptions()
-            val filteredArgs = args.asIterable().propParseFilter(compilerId, daemonOptions, daemonLaunchingOptions, clientOptions)
+            val filteredArgs = args.asIterable().filterSetProps(compilerId, daemonOptions, daemonLaunchingOptions, clientOptions, prefix = COMPILE_DAEMON_CMDLINE_OPTIONS_PREFIX)
 
             if (!clientOptions.stop) {
                 if (compilerId.compilerClasspath.none()) {

compiler/rmi/rmi-interface/src/org/jetbrains/kotlin/rmi/DaemonParams.kt

@@ -18,6 +18,7 @@ package org.jetbrains.kotlin.rmi
 
 import java.io.File
 import java.io.Serializable
+import java.lang.management.ManagementFactory
 import java.security.DigestInputStream
 import java.security.MessageDigest
 import kotlin.platform.platformStatic
@@ -31,29 +32,78 @@ public val COMPILER_DAEMON_CLASS_FQN: String = "org.jetbrains.kotlin.rmi.service
 public val COMPILE_DAEMON_DEFAULT_PORT: Int = 17031
 public val COMPILE_DAEMON_ENABLED_PROPERTY: String ="kotlin.daemon.enabled"
 public val COMPILE_DAEMON_JVM_OPTIONS_PROPERTY: String ="kotlin.daemon.jvm.options"
+public val COMPILE_DAEMON_OPTIONS_PROPERTY: String ="kotlin.daemon.options"
+public val COMPILE_DAEMON_CMDLINE_OPTIONS_PREFIX: String ="--daemon-"
 
 
-fun<C, V, P: KProperty1<C,V>> C.propToParams(p: P, conv: ((v: V) -> String) = { it.toString() } ) =
-        listOf("--daemon-" + p.name, conv(p.get(this)))
+open class PropExtractor<C, V, P: KProperty1<C, V>>(val dest: C,
+                                                    val prop: P,
+                                                    val name: String,
+                                                    val convert: ((v: V) -> String?) = { it.toString() },
+                                                    val skipIf: ((v: V) -> Boolean) = { false },
+                                                    val mergeWithDelimiter: String? = null)
+{
+    constructor(dest: C, prop: P, convert: ((v: V) -> String?) = { it.toString() }, skipIf: ((v: V) -> Boolean) = { false }) : this(dest, prop, prop.name, convert, skipIf)
+    open fun extract(prefix: String = COMPILE_DAEMON_CMDLINE_OPTIONS_PREFIX): List<String> =
+            when {
+                skipIf(prop.get(dest)) -> listOf<String>()
+                mergeWithDelimiter != null -> listOf(prefix + name + mergeWithDelimiter + convert(prop.get(dest))).filterNotNull()
+                else -> listOf(prefix + name, convert(prop.get(dest))).filterNotNull()
+            }
+}
 
-open class PropParser<C, V, P: KMutableProperty1<C, V>>(val dest: C, val prop: P, val parse: (s: String) -> V) {
+class BoolPropExtractor<C, P: KMutableProperty1<C, Boolean>>(dest: C, prop: P, name: String? = null)
+    : PropExtractor<C, Boolean, P>(dest, prop, name ?: prop.name, convert = { null }, skipIf = { !prop.get(dest) })
+
+class RestPropExtractor<C, P: KMutableProperty1<C, out MutableCollection<String>>>(dest: C, prop: P) : PropExtractor<C, MutableCollection<String>, P>(dest, prop, convert = { null }) {
+    override fun extract(prefix: String): List<String> = prop.get(dest).map { prefix + it }
+}
+
+
+open class PropParser<C, V, P: KMutableProperty1<C, V>>(val dest: C,
+                                                        val prop: P, alternativeNames: List<String>,
+                                                        val parse: (s: String) -> V,
+                                                        val allowMergedArg: Boolean = false) {
+    val names = listOf(prop.name) + alternativeNames
+    constructor(dest: C, prop: P, parse: (s: String) -> V, allowMergedArg: Boolean = false) : this(dest, prop, listOf(), parse, allowMergedArg)
     fun apply(s: String) = prop.set(dest, parse(s))
 }
 
 class BoolPropParser<C, P: KMutableProperty1<C, Boolean>>(dest: C, prop: P): PropParser<C, Boolean, P>(dest, prop, { true })
 
-fun Iterable<String>.propParseFilter(parsers: List<PropParser<*,*,*>>) : Iterable<String>  {
+class RestPropParser<C, P: KMutableProperty1<C, MutableCollection<String>>>(dest: C, prop: P): PropParser<C, MutableCollection<String>, P>(dest, prop, { arrayListOf() }) {
+    fun add(s: String) { prop.get(dest).add(s) }
+}
+
+
+fun Iterable<String>.filterSetProps(parsers: List<PropParser<*,*,*>>, prefix: String, restParser: RestPropParser<*,*>? = null) : Iterable<String>  {
     var currentParser: PropParser<*,*,*>? = null
-    return filter { param ->
+    var matchingOption = ""
+    val res = filter { param ->
         if (currentParser == null) {
-            currentParser = parsers.find { param.equals("--daemon-" + it.prop.name) }
-            if (currentParser != null) {
-                if (currentParser is BoolPropParser<*,*>) {
-                    currentParser!!.apply("")
-                    currentParser = null
+            val parser = parsers.find { it.names.any { name ->
+                if (param.startsWith(prefix + name)) { matchingOption = prefix + name; true }
+                else false } }
+            if (parser != null) {
+                val optionLength = matchingOption.length()
+                when {
+                    parser is BoolPropParser<*,*> ->
+                        if (param.length() > optionLength) throw IllegalArgumentException("Invalid switch option '$param', expecting $matchingOption without arguments")
+                        else parser.apply("")
+                    param.length() > optionLength ->
+                        if (param[optionLength] != '=') {
+                            if (parser.allowMergedArg) parser.apply(param.substring(optionLength))
+                            else throw IllegalArgumentException("Invalid option syntax '$param', expecting $matchingOption[= ]<arg>")
+                        }
+                        else parser.apply(param.substring(optionLength + 1))
+                    else -> currentParser = parser
                 }
                 false
             }
+            else if (restParser != null && param.startsWith(prefix)) {
+                restParser.add(param.removePrefix(prefix))
+                false
+            }
             else true
         }
         else {
@@ -62,6 +112,8 @@ fun Iterable<String>.propParseFilter(parsers: List<PropParser<*,*,*>>) : Iterabl
             false
         }
     }
+    if (currentParser != null) throw IllegalArgumentException("Expecting argument for the option $matchingOption")
+    return res
 }
 
 // TODO: find out how to create more generic variant using first constructor
@@ -70,25 +122,35 @@ fun Iterable<String>.propParseFilter(parsers: List<PropParser<*,*,*>>) : Iterabl
 //    kc.constructors.first().
 //}
 
+
+
 public interface CmdlineParams : Serializable {
-    public val asParams: Iterable<String>
+    public val extractors: List<PropExtractor<*,*,*>>
     public val parsers: List<PropParser<*,*,*>>
 }
 
-public fun Iterable<String>.propParseFilter(vararg cs: CmdlineParams) : Iterable<String> =
-    propParseFilter(cs.flatMap { it.parsers })
+public fun Iterable<String>.filterSetProps(vararg cs: CmdlineParams, prefix: String) : Iterable<String> =
+    filterSetProps(cs.flatMap { it.parsers }, prefix)
 
 
 public data class DaemonLaunchingOptions(
-        public var jvmParams: List<String> = listOf()
+        public var maxMemory: String = "",
+        public var maxPermSize: String = "",
+        public var reservedCodeCacheSize: String = "",
+        public var otherJvmParams: MutableCollection<String> = arrayListOf()
 ) : CmdlineParams {
 
-    override val asParams: Iterable<String>
-        get() =
-            propToParams(::jvmParams, { it.joinToString("##") }) // TODO: consider some other options rather than using potentially dangerous delimiter
+    override val extractors: List<PropExtractor<*,*,*>>
+        get() = listOf( PropExtractor(this, ::maxMemory, "Xmx", skipIf = { it.isEmpty() }, mergeWithDelimiter = ""),
+                        PropExtractor(this, ::maxPermSize, "XX:MaxPermSize", skipIf = { it.isEmpty() }, mergeWithDelimiter = "="),
+                        PropExtractor(this, ::reservedCodeCacheSize, "XX:ReservedCodeCacheSize", skipIf = { it.isEmpty() }, mergeWithDelimiter = "="),
+                        RestPropExtractor(this, ::otherJvmParams))
 
     override val parsers: List<PropParser<*,*,*>>
-        get() = listOf( PropParser(this, ::jvmParams, { it.split("##")})) // TODO: see appropriate comment in asParams
+        get() = listOf( PropParser(this, ::maxMemory, listOf("Xmx"), { it }, allowMergedArg = true),
+                        PropParser(this, ::maxPermSize, listOf("XX:MaxPermSize"), { it }, allowMergedArg = true),
+                        PropParser(this, ::reservedCodeCacheSize, listOf("XX:ReservedCodeCacheSize"), { it }, allowMergedArg = true))
+                        // otherJvmParams is missing here deliberately, it is used explicitly as a restParser param to filterSetProps
 }
 
 public data class DaemonOptions(
@@ -98,12 +160,11 @@ public data class DaemonOptions(
         public var startEcho: String = COMPILER_SERVICE_RMI_NAME
 ) : CmdlineParams {
 
-    override val asParams: Iterable<String>
-        get() =
-            propToParams(::port) +
-            propToParams(::autoshutdownMemoryThreshold) +
-            propToParams(::autoshutdownIdleSeconds) +
-            propToParams(::startEcho)
+    override val extractors: List<PropExtractor<*, *, *>>
+        get() = listOf( PropExtractor(this, ::port),
+                        PropExtractor(this, ::autoshutdownMemoryThreshold, skipIf = { it == 0L }),
+                        PropExtractor(this, ::autoshutdownIdleSeconds, skipIf = { it == 0 }),
+                        PropExtractor(this, ::startEcho))
 
     override val parsers: List<PropParser<*,*,*>>
             get() = listOf( PropParser(this, ::port, { it.toInt()}),
@@ -156,11 +217,10 @@ public data class CompilerId(
         // TODO: checksum
 ) : CmdlineParams {
 
-    override val asParams: Iterable<String>
-        get() =
-            propToParams(::compilerClasspath, { it.joinToString(File.pathSeparator) }) +
-            propToParams(::compilerDigest) +
-            propToParams(::compilerVersion)
+    override val extractors: List<PropExtractor<*, *, *>>
+        get() = listOf( PropExtractor(this, ::compilerClasspath, convert = { it.joinToString(File.pathSeparator) }),
+                        PropExtractor(this, ::compilerDigest),
+                        PropExtractor(this, ::compilerVersion, skipIf = { it.isEmpty() }))
 
     override val parsers: List<PropParser<*,*,*>>
         get() =
@@ -176,9 +236,37 @@ public data class CompilerId(
         public platformStatic fun makeCompilerId(vararg paths: File): CompilerId = makeCompilerId(paths.asIterable())
 
         public platformStatic fun makeCompilerId(paths: Iterable<File>): CompilerId =
-                // TODO consider reading version and calculating checksum here
+                // TODO consider reading version here
                 CompilerId(compilerClasspath = paths.map { it.absolutePath }, compilerDigest = paths.getFilesClasspathDigest())
     }
 }
 
 
+public fun isDaemonEnabled(): Boolean = System.getProperty(COMPILE_DAEMON_ENABLED_PROPERTY) != null
+
+
+public fun configureDaemonLaunchingOptions(opts: DaemonLaunchingOptions, inheritMemoryLimits: Boolean): DaemonLaunchingOptions {
+    // note: sequence matters, explicit override in COMPILE_DAEMON_JVM_OPTIONS_PROPERTY should be done after inputArguments processing
+    if (inheritMemoryLimits)
+        ManagementFactory.getRuntimeMXBean().inputArguments.filterSetProps(opts.parsers, "-")
+
+    System.getProperty(COMPILE_DAEMON_JVM_OPTIONS_PROPERTY)?.let {
+        opts.otherJvmParams.addAll( it.trim('"', '\'').split(",").filterSetProps(opts.parsers, "-", RestPropParser(opts, DaemonLaunchingOptions::otherJvmParams)))
+    }
+    return opts
+}
+
+public fun configureDaemonLaunchingOptions(inheritMemoryLimits: Boolean): DaemonLaunchingOptions =
+    configureDaemonLaunchingOptions(DaemonLaunchingOptions(), inheritMemoryLimits = inheritMemoryLimits)
+
+jvmOverloads public fun configureDaemonOptions(opts: DaemonOptions = DaemonOptions()): DaemonOptions {
+    System.getProperty(COMPILE_DAEMON_OPTIONS_PROPERTY)?.let {
+        val unrecognized = it.trim('"', '\'').split(",").filterSetProps(opts.parsers, "")
+        if (unrecognized.any())
+            throw IllegalArgumentException(
+                    "Unrecognized daemon options passed via property $COMPILE_DAEMON_OPTIONS_PROPERTY: " + unrecognized.joinToString(" ") +
+                    "\nSupported options: " + opts.extractors.joinToString(", ", transform = { it.name }))
+    }
+    return opts
+}
+

compiler/rmi/rmi-server/src/org/jetbrains/kotlin/rmi/service/CompileDaemon.kt

@@ -17,9 +17,10 @@
 package org.jetbrains.kotlin.rmi.service
 
 import org.jetbrains.kotlin.cli.jvm.K2JVMCompiler
+import org.jetbrains.kotlin.rmi.COMPILE_DAEMON_CMDLINE_OPTIONS_PREFIX
 import org.jetbrains.kotlin.rmi.CompilerId
 import org.jetbrains.kotlin.rmi.DaemonOptions
-import org.jetbrains.kotlin.rmi.propParseFilter
+import org.jetbrains.kotlin.rmi.filterSetProps
 import org.jetbrains.kotlin.service.CompileServiceImpl
 import java.io.OutputStream
 import java.io.PrintStream
@@ -95,7 +96,7 @@ public class CompileDaemon {
 
             val compilerId = CompilerId()
             val daemonOptions = DaemonOptions()
-            val filteredArgs = args.asIterable().propParseFilter(compilerId, daemonOptions)
+            val filteredArgs = args.asIterable().filterSetProps(compilerId, daemonOptions, prefix = COMPILE_DAEMON_CMDLINE_OPTIONS_PREFIX)
 
             if (filteredArgs.any()) {
                 val helpLine = "usage: <daemon> <compilerId options> <daemon options>"
@@ -115,7 +116,7 @@ public class CompileDaemon {
             val registry = LocateRegistry.createRegistry(daemonOptions.port);
             val compiler = K2JVMCompiler()
 
-            val server = CompileServiceImpl(registry, compiler, compilerId, daemonOptions)
+            CompileServiceImpl(registry, compiler, compilerId, daemonOptions)
 
             if (daemonOptions.startEcho.isNotEmpty())
                 println(daemonOptions.startEcho)

jps-plugin/src/org/jetbrains/kotlin/compilerRunner/KotlinCompilerRunner.java

@@ -32,10 +32,7 @@ import org.jetbrains.kotlin.cli.common.messages.MessageCollector;
 import org.jetbrains.kotlin.cli.common.messages.MessageCollectorUtil;
 import org.jetbrains.kotlin.config.CompilerSettings;
 import org.jetbrains.kotlin.load.kotlin.incremental.components.IncrementalCache;
-import org.jetbrains.kotlin.rmi.CompileService;
-import org.jetbrains.kotlin.rmi.CompilerId;
-import org.jetbrains.kotlin.rmi.DaemonLaunchingOptions;
-import org.jetbrains.kotlin.rmi.DaemonOptions;
+import org.jetbrains.kotlin.rmi.*;
 import org.jetbrains.kotlin.rmi.kotlinr.KotlinCompilerClient;
 import org.jetbrains.kotlin.utils.UtilsPackage;
 
@@ -133,14 +130,13 @@ public class KotlinCompilerRunner {
             String[] argsArray = ArrayUtil.toStringArray(argumentsList);
 
             // trying the daemon first
-            if (incrementalCaches != null && KotlinCompilerClient.Companion.isDaemonEnabled()) {
+            if (incrementalCaches != null && RmiPackage.isDaemonEnabled()) {
                 File libPath = CompilerRunnerUtil.getLibPath(environment.getKotlinPaths(), messageCollector);
                 // TODO: it may be a good idea to cache the compilerId, since making it means calculating digest over jar(s) and if \\
                 //    the lifetime of JPS process is small anyway, we can neglect the probability of changed compiler
                 CompilerId compilerId = CompilerId.makeCompilerId(new File(libPath, "kotlin-compiler.jar"));
-                DaemonOptions daemonOptions = new DaemonOptions();
-                DaemonLaunchingOptions daemonLaunchingOptions = new DaemonLaunchingOptions();
-                KotlinCompilerClient.Companion.configureDaemonLaunchingOptions(daemonLaunchingOptions);
+                DaemonOptions daemonOptions = RmiPackage.configureDaemonOptions();
+                DaemonLaunchingOptions daemonLaunchingOptions = RmiPackage.configureDaemonLaunchingOptions(true);
                 // TODO: find proper stream to report daemon connection progress
                 CompileService daemon = KotlinCompilerClient.Companion.connectToCompileService(compilerId, daemonLaunchingOptions, daemonOptions, System.out, true, true);
                 if (daemon != null) {