From 7815eec7c10e1e732a5853fc048a2ce34118719e Mon Sep 17 00:00:00 2001 From: Alexander Shabalin Date: Fri, 9 Jun 2023 12:04:19 +0000 Subject: [PATCH] [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 --- .../kotlin/backend/konan/BinaryOptions.kt | 27 +++++++++++++----- .../org/jetbrains/kotlin/backend/konan/GC.kt | 7 +++-- .../kotlin/backend/konan/GCSchedulerType.kt | 11 +++++--- .../kotlin/backend/konan/KonanConfig.kt | 24 ++++++++++------ .../backend/konan/KonanConfigurationKeys.kt | 1 - .../backend/konan/SetupConfiguration.kt | 28 +++++++++++++++---- .../backend.native/tests/build.gradle | 6 ++-- .../support/settings/TestProcessSettings.kt | 14 ++++++++-- 8 files changed, 82 insertions(+), 36 deletions(-) diff --git a/kotlin-native/backend.native/compiler/ir/backend.native/src/org/jetbrains/kotlin/backend/konan/BinaryOptions.kt b/kotlin-native/backend.native/compiler/ir/backend.native/src/org/jetbrains/kotlin/backend/konan/BinaryOptions.kt index 3af01236fff..0749bc1c0c3 100644 --- a/kotlin-native/backend.native/compiler/ir/backend.native/src/org/jetbrains/kotlin/backend/konan/BinaryOptions.kt +++ b/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() + val gc by option(shortcut = { it.shortcut }) + + val gcSchedulerType by option(hideValue = { it.deprecatedWithReplacement != null }) val gcMarkSingleThreaded by booleanOption() @@ -100,9 +102,9 @@ open class BinaryOptionRegistry { } } - protected inline fun > option(): PropertyDelegateProvider>> = + protected inline fun > option(noinline shortcut : (T) -> String? = { null }, noinline hideValue: (T) -> Boolean = { false }): PropertyDelegateProvider>> = PropertyDelegateProvider { _, property -> - val option = BinaryOption(property.name, EnumValueParser(enumValues().toList())) + val option = BinaryOption(property.name, EnumValueParser(enumValues().toList(), shortcut, hideValue)) register(option) ReadOnlyProperty { _, _ -> option.compilerConfigurationKey @@ -124,10 +126,21 @@ private object StringValueParser : BinaryOption.ValueParser { } @PublishedApi -internal class EnumValueParser>(val values: List) : BinaryOption.ValueParser { - // TODO: should we really ignore case here? - override fun parse(value: String): T? = values.firstOrNull { it.name.equals(value, ignoreCase = true) } +internal class EnumValueParser>( + val values: List, + val shortcut: (T) -> String?, + val hideValue: (T) -> Boolean, +) : BinaryOption.ValueParser { + 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("|") } diff --git a/kotlin-native/backend.native/compiler/ir/backend.native/src/org/jetbrains/kotlin/backend/konan/GC.kt b/kotlin-native/backend.native/compiler/ir/backend.native/src/org/jetbrains/kotlin/backend/konan/GC.kt index 56158e2517a..a32f8b76632 100644 --- a/kotlin-native/backend.native/compiler/ir/backend.native/src/org/jetbrains/kotlin/backend/konan/GC.kt +++ b/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, - CONCURRENT_MARK_AND_SWEEP + STOP_THE_WORLD_MARK_AND_SWEEP("stwms"), + PARALLEL_MARK_CONCURRENT_SWEEP("pmcs"), + // TODO: Bring back CONCURRENT_MARK_AND_SWEEP when we get concurrent mark } diff --git a/kotlin-native/backend.native/compiler/ir/backend.native/src/org/jetbrains/kotlin/backend/konan/GCSchedulerType.kt b/kotlin-native/backend.native/compiler/ir/backend.native/src/org/jetbrains/kotlin/backend/konan/GCSchedulerType.kt index df2befc0287..658fb57945e 100644 --- a/kotlin-native/backend.native/compiler/ir/backend.native/src/org/jetbrains/kotlin/backend/konan/GCSchedulerType.kt +++ b/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) { - DISABLED(0), - WITH_TIMER(1), - ON_SAFE_POINTS(2), +enum class GCSchedulerType(val value: Int, val deprecatedWithReplacement: GCSchedulerType? = null) { + MANUAL(0), + ADAPTIVE(1), AGGRESSIVE(3), + // Deprecated: + DISABLED(0, deprecatedWithReplacement = MANUAL), + WITH_TIMER(1, deprecatedWithReplacement = ADAPTIVE), + ON_SAFE_POINTS(2, deprecatedWithReplacement = ADAPTIVE), } diff --git a/kotlin-native/backend.native/compiler/ir/backend.native/src/org/jetbrains/kotlin/backend/konan/KonanConfig.kt b/kotlin-native/backend.native/compiler/ir/backend.native/src/org/jetbrains/kotlin/backend/konan/KonanConfig.kt index 7a30e81b539..39bea711bf1 100644 --- a/kotlin-native/backend.native/compiler/ir/backend.native/src/org/jetbrains/kotlin/backend/konan/KonanConfig.kt +++ b/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 - val gc: GC by lazy { - configuration.get(KonanConfigKeys.GARBAGE_COLLECTOR) ?: defaultGC - } + private val defaultGC get() = GC.PARALLEL_MARK_CONCURRENT_SWEEP + val gc: GC get() = configuration.get(BinaryOptions.gc) ?: 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") } } diff --git a/kotlin-native/backend.native/compiler/ir/backend.native/src/org/jetbrains/kotlin/backend/konan/KonanConfigurationKeys.kt b/kotlin-native/backend.native/compiler/ir/backend.native/src/org/jetbrains/kotlin/backend/konan/KonanConfigurationKeys.kt index 621361ab735..0f72b7e04b5 100644 --- a/kotlin-native/backend.native/compiler/ir/backend.native/src/org/jetbrains/kotlin/backend/konan/KonanConfigurationKeys.kt +++ b/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 = CompilerConfigurationKey.create("when to destroy runtime") - val GARBAGE_COLLECTOR: CompilerConfigurationKey = CompilerConfigurationKey.create("gc") val PROPERTY_LAZY_INITIALIZATION: CompilerConfigurationKey = CompilerConfigurationKey.create("lazy top level properties initialization") val WORKER_EXCEPTION_HANDLING: CompilerConfigurationKey = CompilerConfigurationKey.create("unhandled exception processing in Worker.executeAfter") diff --git a/kotlin-native/backend.native/compiler/ir/backend.native/src/org/jetbrains/kotlin/backend/konan/SetupConfiguration.kt b/kotlin-native/backend.native/compiler/ir/backend.native/src/org/jetbrains/kotlin/backend/konan/SetupConfiguration.kt index da2b506de7c..cf811d6d6c2 100644 --- a/kotlin-native/backend.native/compiler/ir/backend.native/src/org/jetbrains/kotlin/backend/konan/SetupConfiguration.kt +++ b/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 - "cms" -> GC.CONCURRENT_MARK_AND_SWEEP + "stms" -> GC.STOP_THE_WORLD_MARK_AND_SWEEP + "cms" -> GC.PARALLEL_MARK_CONCURRENT_SWEEP else -> { - report(ERROR, "Unsupported GC ${arguments.gc}") + val validValues = enumValues().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) diff --git a/kotlin-native/backend.native/tests/build.gradle b/kotlin-native/backend.native/tests/build.gradle index 8e137490aaa..bf477f82379 100644 --- a/kotlin-native/backend.native/tests/build.gradle +++ b/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') diff --git a/native/native.tests/tests/org/jetbrains/kotlin/konan/blackboxtest/support/settings/TestProcessSettings.kt b/native/native.tests/tests/org/jetbrains/kotlin/konan/blackboxtest/support/settings/TestProcessSettings.kt index c5428923955..00be59c5e49 100644 --- a/native/native.tests/tests/org/jetbrains/kotlin/konan/blackboxtest/support/settings/TestProcessSettings.kt +++ b/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"), - AGGRESSIVE("-Xbinary=gcSchedulerType=aggressive"); + ON_SAFE_POINTS("-Xbinary=gcSchedulerType=on_safe_points"); override fun toString() = compilerFlag?.let { "($it)" }.orEmpty() }