[K/N] Rework GC switches with better naming

Additionally, deprecate -Xgc in favour of a new binary option "gc".
This will allow setting gc right in gradle.properties

Merge-request: KT-MR-10498
Merged-by: Alexander Shabalin <Alexander.Shabalin@jetbrains.com>

kotlin-native/backend.native/compiler/ir/backend.native/src/org/jetbrains/kotlin/backend/konan/BinaryOptions.kt

@@ -33,7 +33,9 @@ object BinaryOptions : BinaryOptionRegistry() {
 
     val objcExportIgnoreInterfaceMethodCollisions by booleanOption()
 
-    val gcSchedulerType by option<GCSchedulerType>()
+    val gc by option<GC>(shortcut = { it.shortcut })
+
+    val gcSchedulerType by option<GCSchedulerType>(hideValue = { it.deprecatedWithReplacement != null })
 
     val gcMarkSingleThreaded by booleanOption()
 
@@ -100,9 +102,9 @@ open class BinaryOptionRegistry {
                 }
             }
 
-    protected inline fun <reified T : Enum<T>> option(): PropertyDelegateProvider<Any?, ReadOnlyProperty<Any?, CompilerConfigurationKey<T>>> =
+    protected inline fun <reified T : Enum<T>> option(noinline shortcut : (T) -> String? = { null }, noinline hideValue: (T) -> Boolean = { false }): PropertyDelegateProvider<Any?, ReadOnlyProperty<Any?, CompilerConfigurationKey<T>>> =
             PropertyDelegateProvider { _, property ->
-                val option = BinaryOption(property.name, EnumValueParser(enumValues<T>().toList()))
+                val option = BinaryOption(property.name, EnumValueParser(enumValues<T>().toList(), shortcut, hideValue))
                 register(option)
                 ReadOnlyProperty { _, _ ->
                     option.compilerConfigurationKey
@@ -124,10 +126,21 @@ private object StringValueParser : BinaryOption.ValueParser<String> {
 }
 
 @PublishedApi
-internal class EnumValueParser<T : Enum<T>>(val values: List<T>) : BinaryOption.ValueParser<T> {
+internal class EnumValueParser<T : Enum<T>>(
-    // TODO: should we really ignore case here?
+    val values: List<T>,
-    override fun parse(value: String): T? = values.firstOrNull { it.name.equals(value, ignoreCase = true) }
+    val shortcut: (T) -> String?,
+    val hideValue: (T) -> Boolean,
+) : BinaryOption.ValueParser<T> {
+    override fun parse(value: String): T? = values.firstOrNull {
+        // TODO: should we really ignore case here?
+        it.name.equals(value, ignoreCase = true) || (shortcut(it)?.equals(value, ignoreCase = true) ?: false)
+    }
 
     override val validValuesHint: String?
-        get() = values.joinToString("|")
+        get() = values.filter { !hideValue(it) }.map {
+            val fullName = "$it".lowercase()
+            shortcut(it)?.let { short ->
+                "$fullName (or: $short)"
+            } ?: fullName
+        }.joinToString("|")
 }

kotlin-native/backend.native/compiler/ir/backend.native/src/org/jetbrains/kotlin/backend/konan/GC.kt

@@ -5,8 +5,9 @@
 
 package org.jetbrains.kotlin.backend.konan
 
-enum class GC {
+enum class GC(val shortcut: String? = null) {
     NOOP,
-    SAME_THREAD_MARK_AND_SWEEP,
+    STOP_THE_WORLD_MARK_AND_SWEEP("stwms"),
-    CONCURRENT_MARK_AND_SWEEP
+    PARALLEL_MARK_CONCURRENT_SWEEP("pmcs"),
+    // TODO: Bring back CONCURRENT_MARK_AND_SWEEP when we get concurrent mark
 }

kotlin-native/backend.native/compiler/ir/backend.native/src/org/jetbrains/kotlin/backend/konan/GCSchedulerType.kt

@@ -6,9 +6,12 @@
 package org.jetbrains.kotlin.backend.konan
 
 // Must match `GCSchedulerType` in CompilerConstants.hpp
-enum class GCSchedulerType(val value: Int) {
+enum class GCSchedulerType(val value: Int, val deprecatedWithReplacement: GCSchedulerType? = null) {
-    DISABLED(0),
+    MANUAL(0),
-    WITH_TIMER(1),
+    ADAPTIVE(1),
-    ON_SAFE_POINTS(2),
     AGGRESSIVE(3),
+    // Deprecated:
+    DISABLED(0, deprecatedWithReplacement = MANUAL),
+    WITH_TIMER(1, deprecatedWithReplacement = ADAPTIVE),
+    ON_SAFE_POINTS(2, deprecatedWithReplacement = ADAPTIVE),
 }

kotlin-native/backend.native/compiler/ir/backend.native/src/org/jetbrains/kotlin/backend/konan/KonanConfig.kt

@@ -88,10 +88,8 @@ class KonanConfig(val project: Project, val configuration: CompilerConfiguration
             configuration.report(CompilerMessageSeverity.STRONG_WARNING, "-Xdestroy-runtime-mode switch is deprecated and will be removed in a future release.")
         }
     }.let { DestroyRuntimeMode.ON_SHUTDOWN }
-    private val defaultGC get() = GC.CONCURRENT_MARK_AND_SWEEP
+    private val defaultGC get() = GC.PARALLEL_MARK_CONCURRENT_SWEEP
-    val gc: GC by lazy {
+    val gc: GC get() = configuration.get(BinaryOptions.gc) ?: defaultGC
-        configuration.get(KonanConfigKeys.GARBAGE_COLLECTOR) ?: defaultGC
-    }
     val runtimeAssertsMode: RuntimeAssertsMode get() = configuration.get(BinaryOptions.runtimeAssertionsMode) ?: RuntimeAssertsMode.IGNORE
     private val defaultDisableMmap get() = target.family == Family.MINGW
     val disableMmap: Boolean by lazy {
@@ -130,10 +128,18 @@ class KonanConfig(val project: Project, val configuration: CompilerConfiguration
                 ?: SourceInfoType.CORESYMBOLICATION.takeIf { debug && target.supportsCoreSymbolication() }
                 ?: SourceInfoType.NOOP
 
-    val defaultGCSchedulerType get() = GCSchedulerType.WITH_TIMER
+    val defaultGCSchedulerType get() =
+        when (gc) {
+            GC.NOOP -> GCSchedulerType.MANUAL
+            else -> GCSchedulerType.ADAPTIVE
+        }
 
     val gcSchedulerType: GCSchedulerType by lazy {
-        configuration.get(BinaryOptions.gcSchedulerType) ?: defaultGCSchedulerType
+        val arg = configuration.get(BinaryOptions.gcSchedulerType) ?: defaultGCSchedulerType
+        arg.deprecatedWithReplacement?.let { replacement ->
+            configuration.report(CompilerMessageSeverity.WARNING, "Binary option gcSchedulerType=$arg is deprecated. Use gcSchedulerType=$replacement instead")
+            replacement
+        } ?: arg
     }
 
     val gcMarkSingleThreaded: Boolean
@@ -243,7 +249,7 @@ class KonanConfig(val project: Project, val configuration: CompilerConfiguration
                 }
             }
             AllocationMode.CUSTOM -> {
-                if (gc == GC.CONCURRENT_MARK_AND_SWEEP) {
+                if (gc == GC.PARALLEL_MARK_CONCURRENT_SWEEP) {
                     AllocationMode.CUSTOM
                 } else {
                     configuration.report(CompilerMessageSeverity.STRONG_WARNING,
@@ -263,13 +269,13 @@ class KonanConfig(val project: Project, val configuration: CompilerConfiguration
         } else {
             add("experimental_memory_manager.bc")
             when (gc) {
-                GC.SAME_THREAD_MARK_AND_SWEEP -> {
+                GC.STOP_THE_WORLD_MARK_AND_SWEEP -> {
                     add("same_thread_ms_gc.bc")
                 }
                 GC.NOOP -> {
                     add("noop_gc.bc")
                 }
-                GC.CONCURRENT_MARK_AND_SWEEP -> {
+                GC.PARALLEL_MARK_CONCURRENT_SWEEP -> {
                     add("concurrent_ms_gc.bc")
                 }
             }

kotlin-native/backend.native/compiler/ir/backend.native/src/org/jetbrains/kotlin/backend/konan/KonanConfigurationKeys.kt

@@ -151,7 +151,6 @@ class KonanConfigKeys {
                 = CompilerConfigurationKey.create("override konan.properties values")
         val DESTROY_RUNTIME_MODE: CompilerConfigurationKey<DestroyRuntimeMode>
                 = CompilerConfigurationKey.create("when to destroy runtime")
-        val GARBAGE_COLLECTOR: CompilerConfigurationKey<GC> = CompilerConfigurationKey.create("gc")
         val PROPERTY_LAZY_INITIALIZATION: CompilerConfigurationKey<Boolean>
                 = CompilerConfigurationKey.create("lazy top level properties initialization")
         val WORKER_EXCEPTION_HANDLING: CompilerConfigurationKey<WorkerExceptionHandling> = CompilerConfigurationKey.create("unhandled exception processing in Worker.executeAfter")

kotlin-native/backend.native/compiler/ir/backend.native/src/org/jetbrains/kotlin/backend/konan/SetupConfiguration.kt

@@ -210,16 +210,32 @@ fun CompilerConfiguration.setupFromArguments(arguments: K2NativeCompilerArgument
             null
         }
     })
-    putIfNotNull(GARBAGE_COLLECTOR, when (arguments.gc) {
+
+    val gcFromArgument = when (arguments.gc) {
         null -> null
         "noop" -> GC.NOOP
-        "stms" -> GC.SAME_THREAD_MARK_AND_SWEEP
+        "stms" -> GC.STOP_THE_WORLD_MARK_AND_SWEEP
-        "cms" -> GC.CONCURRENT_MARK_AND_SWEEP
+        "cms" -> GC.PARALLEL_MARK_CONCURRENT_SWEEP
         else -> {
-            report(ERROR, "Unsupported GC ${arguments.gc}")
+            val validValues = enumValues<GC>().map {
+                val fullName = "$it".lowercase()
+                it.shortcut?.let { short ->
+                    "$fullName (or: $short)"
+                } ?: fullName
+            }.joinToString("|")
+            report(ERROR, "Unsupported argument -Xgc=${arguments.gc}. Use -Xbinary=gc= with values ${validValues}")
             null
         }
-    })
+    }
+    if (gcFromArgument != null) {
+        val newValue = gcFromArgument.shortcut ?: "$gcFromArgument".lowercase()
+        report(WARNING, "-Xgc=${arguments.gc} compiler argument is deprecated. Use -Xbinary=gc=${newValue} instead")
+    }
+    // TODO: revise priority and/or report conflicting values.
+    if (get(BinaryOptions.gc) == null) {
+        putIfNotNull(BinaryOptions.gc, gcFromArgument)
+    }
+
     putIfNotNull(PROPERTY_LAZY_INITIALIZATION, when (arguments.propertyLazyInitialization) {
         null -> null
         "enable" -> true
@@ -302,7 +318,7 @@ internal fun CompilerConfiguration.setupCommonOptionsForCaches(konanConfig: Kona
     put(PROPERTY_LAZY_INITIALIZATION, konanConfig.propertyLazyInitialization)
     put(BinaryOptions.stripDebugInfoFromNativeLibs, !konanConfig.useDebugInfoInNativeLibs)
     put(ALLOCATION_MODE, konanConfig.allocationMode)
-    put(GARBAGE_COLLECTOR, konanConfig.gc)
+    put(BinaryOptions.gc, konanConfig.gc)
     put(BinaryOptions.gcSchedulerType, konanConfig.gcSchedulerType)
     put(BinaryOptions.runtimeAssertionsMode, konanConfig.runtimeAssertsMode)
     put(LAZY_IR_FOR_CACHES, konanConfig.lazyIrForCaches)

kotlin-native/backend.native/tests/build.gradle

@@ -91,9 +91,9 @@ tasks.withType(KonanCompileNativeBinary.class).configureEach {
     enableTwoStageCompilation = twoStageEnabled
 }
 
-ext.isNoopGC = project.globalTestArgs.contains("-Xgc=noop")
+ext.isNoopGC = project.globalTestArgs.contains("-Xbinary=gc=noop") || project.globalTestArgs.contains("-Xgc=noop")
+ext.isSTWMSGC = project.globalTestArgs.contains("-Xbinary=gc=stwms") || project.globalTestArgs.contains("-Xgc=stms")
 ext.isAggressiveGC = project.globalTestArgs.contains("-Xbinary=gcSchedulerType=aggressive")
-ext.isCmsGC = project.globalTestArgs.contains("-Xgc=cms") || !project.globalTestArgs.any { it.startsWith("-Xgc=") }
 ext.runtimeAssertionsPanic = false
 
 // TODO: It also makes sense to test -g without asserts, and also to test -opt with asserts.
@@ -5927,7 +5927,7 @@ if (HostManager.@Companion.hostIsMac) {
 }
 
 fileCheckTest("filecheck_redundant_safepoints_removal") {
-    enabled = isCmsGC &&
+    enabled = !isNoopGC && !isSTWMSGC &&
             project.globalTestArgs.contains("-opt") &&
             !needSmallBinary(project)
     annotatedSource = project.file('filecheck/redundant_safepoints.kt')

native/native.tests/tests/org/jetbrains/kotlin/konan/blackboxtest/support/settings/TestProcessSettings.kt

@@ -141,7 +141,11 @@ internal enum class Sanitizer(val compilerFlag: String?) {
  */
 internal enum class GCType(val compilerFlag: String?) {
     UNSPECIFIED(null),
-    NOOP("-Xgc=noop"),
+    NOOP("-Xbinary=gc=noop"),
+    STWMS("-Xbinary=gc=stwms"),
+    PMCS("-Xbinary=gc=pmcs"),
+
+    // TODO: Remove these deprecated GC options.
     STMS("-Xgc=stms"),
     CMS("-Xgc=cms");
 
@@ -150,10 +154,14 @@ internal enum class GCType(val compilerFlag: String?) {
 
 internal enum class GCScheduler(val compilerFlag: String?) {
     UNSPECIFIED(null),
+    MANUAL("-Xbinary=gcSchedulerType=manual"),
+    ADAPTIVE("-Xbinary=gcSchedulerType=adaptive"),
+    AGGRESSIVE("-Xbinary=gcSchedulerType=aggressive"),
+
+    // TODO: Remove these deprecated GC scheduler options.
     DISABLED("-Xbinary=gcSchedulerType=disabled"),
     WITH_TIMER("-Xbinary=gcSchedulerType=with_timer"),
-    ON_SAFE_POINTS("-Xbinary=gcSchedulerType=on_safe_points"),
+    ON_SAFE_POINTS("-Xbinary=gcSchedulerType=on_safe_points");
-    AGGRESSIVE("-Xbinary=gcSchedulerType=aggressive");
 
     override fun toString() = compilerFlag?.let { "($it)" }.orEmpty()
 }