From 111bb461a91cb13b1db256404b8e92e94524e05e Mon Sep 17 00:00:00 2001 From: Troels Bjerre Lund Date: Sat, 29 Apr 2023 07:55:23 +0200 Subject: [PATCH] CLI: Change kotlin reflection to java reflection The command line argument parser is using between 0.25s and 0.5s (depending on platform) on finding annotated properties. This fix replaces the slow kotlin reflection with java reflection, which is an order of magnitude faster. #KT-58183 Fixed --- .../compilerRunner/argumentsToStrings.kt | 4 +- .../idea/explicitDefaultSubstitutors.kt | 4 +- .../CompilerArgumentParsingTest.kt | 4 +- .../CompilerArgumentsImplementationTest.kt | 4 +- compiler/cli/build.gradle.kts | 1 + .../arguments/parseCommandLineArguments.kt | 52 +++++++++++-------- .../jetbrains/kotlin/cli/common/Usage.java | 7 ++- .../arguments/GenerateGradleOptions.kt | 11 ++-- .../CompilerArgumentsContentProspector.kt | 3 +- .../kotlin/config/KotlinFacetSettings.kt | 4 +- .../KotlinCompileArgumentsTest.kt | 6 +-- .../compiler/plugin/impl/errorReporting.kt | 4 +- 12 files changed, 61 insertions(+), 43 deletions(-) diff --git a/build-common/src/org/jetbrains/kotlin/compilerRunner/argumentsToStrings.kt b/build-common/src/org/jetbrains/kotlin/compilerRunner/argumentsToStrings.kt index dea13b22e36..9a43942908d 100644 --- a/build-common/src/org/jetbrains/kotlin/compilerRunner/argumentsToStrings.kt +++ b/build-common/src/org/jetbrains/kotlin/compilerRunner/argumentsToStrings.kt @@ -12,8 +12,8 @@ import org.jetbrains.kotlin.cli.common.arguments.CommonToolArguments import org.jetbrains.kotlin.cli.common.arguments.isAdvanced import org.jetbrains.kotlin.cli.common.arguments.resolvedDelimiter import kotlin.reflect.KClass -import kotlin.reflect.full.findAnnotation import kotlin.reflect.full.memberProperties +import kotlin.reflect.jvm.javaField @Suppress("UNCHECKED_CAST") @JvmOverloads @@ -33,7 +33,7 @@ internal fun toArgumentStrings( ): List = ArrayList().apply { val defaultArguments = type.newArgumentsInstance() type.memberProperties.forEach { property -> - val argumentAnnotation = property.findAnnotation() ?: return@forEach + val argumentAnnotation = property.javaField?.getAnnotation(Argument::class.java) ?: return@forEach val rawPropertyValue = property.get(thisArguments) val rawDefaultValue = property.get(defaultArguments) diff --git a/build-common/src/org/jetbrains/kotlin/idea/explicitDefaultSubstitutors.kt b/build-common/src/org/jetbrains/kotlin/idea/explicitDefaultSubstitutors.kt index ebaded2a5d4..946c459c06d 100644 --- a/build-common/src/org/jetbrains/kotlin/idea/explicitDefaultSubstitutors.kt +++ b/build-common/src/org/jetbrains/kotlin/idea/explicitDefaultSubstitutors.kt @@ -12,6 +12,7 @@ import org.jetbrains.kotlin.config.JvmTarget import kotlin.reflect.KClass import kotlin.reflect.KProperty1 import kotlin.reflect.full.findAnnotation +import kotlin.reflect.jvm.javaField //used by IJ facet import @SuppressWarnings("unused") @@ -24,7 +25,8 @@ sealed class ExplicitDefaultSubstitutor { abstract fun isSubstitutable(args: List): Boolean protected val argument: Argument by lazy { - substitutedProperty.findAnnotation() ?: error("Property \"${substitutedProperty.name}\" has no Argument annotation") + substitutedProperty.javaField?.getAnnotation(Argument::class.java) + ?: error("Property \"${substitutedProperty.name}\" has no Argument annotation") } } diff --git a/build-common/test/org/jetbrains/kotlin/compilerRunner/CompilerArgumentParsingTest.kt b/build-common/test/org/jetbrains/kotlin/compilerRunner/CompilerArgumentParsingTest.kt index d5bd6f59d32..f89161d2271 100644 --- a/build-common/test/org/jetbrains/kotlin/compilerRunner/CompilerArgumentParsingTest.kt +++ b/build-common/test/org/jetbrains/kotlin/compilerRunner/CompilerArgumentParsingTest.kt @@ -14,10 +14,10 @@ import org.junit.jupiter.params.provider.MethodSource import java.util.Base64.getEncoder import kotlin.random.Random import kotlin.reflect.* -import kotlin.reflect.full.findAnnotation import kotlin.reflect.full.isSubtypeOf import kotlin.reflect.full.memberProperties import kotlin.reflect.full.withNullability +import kotlin.reflect.jvm.javaField import kotlin.test.assertContentEquals import kotlin.test.fail @@ -72,7 +72,7 @@ class CompilerArgumentParsingTest { private fun assertEqualArguments(expected: CommonToolArguments, actual: CommonToolArguments) { if (expected::class != actual::class) fail("Expected class '${expected::class}', found: '${actual::class}'") expected::class.memberProperties - .filter { it.findAnnotation() != null } + .filter { it.javaField?.getAnnotation(Argument::class.java) != null } .ifEmpty { fail("No members with ${Argument::class} annotation") } .map { property -> @Suppress("UNCHECKED_CAST") diff --git a/build-common/test/org/jetbrains/kotlin/compilerRunner/CompilerArgumentsImplementationTest.kt b/build-common/test/org/jetbrains/kotlin/compilerRunner/CompilerArgumentsImplementationTest.kt index 3132e3b5fce..129f69b560e 100644 --- a/build-common/test/org/jetbrains/kotlin/compilerRunner/CompilerArgumentsImplementationTest.kt +++ b/build-common/test/org/jetbrains/kotlin/compilerRunner/CompilerArgumentsImplementationTest.kt @@ -11,8 +11,8 @@ import org.junit.jupiter.params.ParameterizedTest import org.junit.jupiter.params.provider.MethodSource import kotlin.reflect.KClass import kotlin.reflect.KVisibility -import kotlin.reflect.full.findAnnotation import kotlin.reflect.full.memberProperties +import kotlin.reflect.jvm.javaField import kotlin.test.fail @@ -22,7 +22,7 @@ class CompilerArgumentsImplementationTest { @MethodSource("implementations") fun `test - all properties with Argument annotation - are public`(implementation: KClass) { implementation.memberProperties.forEach { property -> - if (property.findAnnotation() != null) { + if (property.javaField?.getAnnotation(Argument::class.java) != null) { if (property.visibility != KVisibility.PUBLIC) { fail( "Property '${property.name}: ${property.returnType}' " + diff --git a/compiler/cli/build.gradle.kts b/compiler/cli/build.gradle.kts index c6f6636d8b6..4451a323e19 100644 --- a/compiler/cli/build.gradle.kts +++ b/compiler/cli/build.gradle.kts @@ -32,6 +32,7 @@ dependencies { compileOnly(toolsJarApi()) compileOnly(intellijCore()) compileOnly(commonDependency("org.jetbrains.intellij.deps:trove4j")) + compileOnly(commonDependency("org.jetbrains.kotlin:kotlin-reflect")) { isTransitive = false } testApi(project(":compiler:backend")) testApi(project(":compiler:cli")) diff --git a/compiler/cli/cli-common/src/org/jetbrains/kotlin/cli/common/arguments/parseCommandLineArguments.kt b/compiler/cli/cli-common/src/org/jetbrains/kotlin/cli/common/arguments/parseCommandLineArguments.kt index 5384afd0290..c72cc0e1b02 100644 --- a/compiler/cli/cli-common/src/org/jetbrains/kotlin/cli/common/arguments/parseCommandLineArguments.kt +++ b/compiler/cli/cli-common/src/org/jetbrains/kotlin/cli/common/arguments/parseCommandLineArguments.kt @@ -18,13 +18,13 @@ package org.jetbrains.kotlin.cli.common.arguments import org.jetbrains.kotlin.cli.common.CompilerSystemProperties import org.jetbrains.kotlin.konan.file.File +import org.jetbrains.kotlin.load.java.JvmAbi import org.jetbrains.kotlin.utils.SmartList +import java.lang.reflect.Method import kotlin.reflect.KClass -import kotlin.reflect.KMutableProperty1 import kotlin.reflect.cast -import kotlin.reflect.full.memberProperties -@Target(AnnotationTarget.PROPERTY) +@Target(AnnotationTarget.FIELD) annotation class Argument( val value: String, val shortName: String = "", @@ -120,13 +120,22 @@ private fun parsePreprocessedCommandLineArguments( errors: Lazy, overrideArguments: Boolean ) { - data class ArgumentField(val property: KMutableProperty1, val argument: Argument) + data class ArgumentField(val getter: Method, val setter: Method, val argument: Argument) - @Suppress("UNCHECKED_CAST") - val properties = result::class.memberProperties.mapNotNull { property -> - if (property !is KMutableProperty1<*, *>) return@mapNotNull null - val argument = property.annotations.firstOrNull { it is Argument } as Argument? ?: return@mapNotNull null - ArgumentField(property as KMutableProperty1, argument) + val superClasses = mutableListOf>(result::class.java) + while (superClasses.last() != Any::class.java) { + superClasses.add(superClasses.last().superclass) + } + + val resultClass = result::class.java + val properties = superClasses.flatMap { + it.declaredFields.mapNotNull { field -> + field.getAnnotation(Argument::class.java)?.let { argument -> + val getter = resultClass.getMethod(JvmAbi.getterName(field.name)) + val setter = resultClass.getMethod(JvmAbi.setterName(field.name), field.type) + ArgumentField(getter, setter, argument) + } + } } val visitedArgs = mutableSetOf() @@ -144,7 +153,7 @@ private fun parsePreprocessedCommandLineArguments( } if (argument.value == arg) { - if (argument.isAdvanced && property.returnType.classifier != Boolean::class) { + if (argument.isAdvanced && getter.returnType.kotlin != Boolean::class) { errors.value.extraArgumentsPassedInObsoleteForm.add(arg) } return true @@ -200,9 +209,9 @@ private fun parsePreprocessedCommandLineArguments( continue } - val (property, argument) = argumentField + val (getter, setter, argument) = argumentField val value: Any = when { - argumentField.property.returnType.classifier == Boolean::class -> { + getter.returnType.kotlin == Boolean::class -> { if (arg.startsWith(argument.value + "=")) { // Can't use toBooleanStrict yet because this part of the compiler is used in Gradle and needs API version 1.4. when (arg.substring(argument.value.length + 1)) { @@ -227,13 +236,12 @@ private fun parsePreprocessedCommandLineArguments( } } - if ((argumentField.property.returnType.classifier as? KClass<*>)?.java?.isArray == false - && !visitedArgs.add(argument.value) && value is String && property.get(result) != value + if (!getter.returnType.isArray && !visitedArgs.add(argument.value) && value is String && getter(result) != value ) { errors.value.duplicateArguments[argument.value] = value } - updateField(property, result, value, argument.resolvedDelimiter, overrideArguments) + updateField(getter, setter, result, value, argument.resolvedDelimiter, overrideArguments) } result.freeArgs += freeArgs @@ -257,25 +265,27 @@ private fun A.updateInternalArguments( } private fun updateField( - property: KMutableProperty1, + getter: Method, + setter: Method, result: A, value: Any, delimiter: String?, overrideArguments: Boolean ) { - when (property.returnType.classifier) { - Boolean::class, String::class -> property.set(result, value) + when (getter.returnType.kotlin) { + Boolean::class, String::class -> setter(result, value) Array::class -> { val newElements = if (delimiter.isNullOrEmpty()) { arrayOf(value as String) } else { (value as String).split(delimiter).toTypedArray() } + @Suppress("UNCHECKED_CAST") - val oldValue = property.get(result) as Array? - property.set(result, if (oldValue != null && !overrideArguments) arrayOf(*oldValue, *newElements) else newElements) + val oldValue = getter(result) as Array? + setter(result, if (oldValue != null && !overrideArguments) arrayOf(*oldValue, *newElements) else newElements) } - else -> throw IllegalStateException("Unsupported argument type: ${property.returnType}") + else -> throw IllegalStateException("Unsupported argument type: ${getter.returnType}") } } diff --git a/compiler/cli/src/org/jetbrains/kotlin/cli/common/Usage.java b/compiler/cli/src/org/jetbrains/kotlin/cli/common/Usage.java index 910fba361fb..5df6e5e8628 100644 --- a/compiler/cli/src/org/jetbrains/kotlin/cli/common/Usage.java +++ b/compiler/cli/src/org/jetbrains/kotlin/cli/common/Usage.java @@ -17,11 +17,11 @@ package org.jetbrains.kotlin.cli.common; import com.intellij.openapi.util.SystemInfo; -import com.intellij.util.containers.ContainerUtil; import kotlin.jvm.JvmClassMappingKt; import kotlin.reflect.KCallable; import kotlin.reflect.KClass; import kotlin.reflect.KProperty1; +import kotlin.reflect.jvm.ReflectJvmMapping; import kotlin.text.StringsKt; import org.jetbrains.annotations.NotNull; import org.jetbrains.kotlin.cli.common.arguments.Argument; @@ -29,6 +29,8 @@ import org.jetbrains.kotlin.cli.common.arguments.CommonToolArguments; import org.jetbrains.kotlin.cli.common.arguments.ParseCommandLineArgumentsKt; import org.jetbrains.kotlin.cli.common.arguments.PreprocessCommandLineArgumentsKt; +import java.lang.reflect.Field; + public class Usage { public static final String BAT_DELIMITER_CHARACTERS_NOTE = "Note: on Windows, arguments that contain delimiter characters (whitespace, =, ;, ,) need to be surrounded with double quotes (\")."; @@ -71,7 +73,8 @@ public class Usage { } private static void propertyUsage(@NotNull StringBuilder sb, @NotNull KProperty1 property, boolean extraHelp) { - Argument argument = ContainerUtil.findInstance(property.getAnnotations(), Argument.class); + Field field = ReflectJvmMapping.getJavaField(property); + Argument argument = field.getAnnotation(Argument.class); if (argument == null) return; if (extraHelp != ParseCommandLineArgumentsKt.isAdvanced(argument)) return; diff --git a/generators/tests/org/jetbrains/kotlin/generators/arguments/GenerateGradleOptions.kt b/generators/tests/org/jetbrains/kotlin/generators/arguments/GenerateGradleOptions.kt index efb79c67c7c..7219392a4f1 100644 --- a/generators/tests/org/jetbrains/kotlin/generators/arguments/GenerateGradleOptions.kt +++ b/generators/tests/org/jetbrains/kotlin/generators/arguments/GenerateGradleOptions.kt @@ -16,17 +16,18 @@ import kotlin.reflect.KClass import kotlin.reflect.KProperty1 import kotlin.reflect.full.declaredMemberProperties import kotlin.reflect.full.withNullability +import kotlin.reflect.jvm.javaField -// Additional properties that should be included in interface +// Additional properties that should be included @Suppress("unused") -interface AdditionalGradleProperties { +private class AdditionalGradleProperties { @GradleOption( value = DefaultValue.EMPTY_STRING_LIST_DEFAULT, gradleInputType = GradleInputTypes.INPUT, shouldGenerateDeprecatedKotlinOptions = true, ) @Argument(value = "", description = "A list of additional compiler arguments") - var freeCompilerArgs: List + var freeCompilerArgs = listOf() } private data class GeneratedOptions( @@ -869,7 +870,7 @@ private fun Printer.generateOptionDeprecation(property: KProperty1<*, *>) { } private fun Printer.generateDoc(property: KProperty1<*, *>) { - val description = property.findAnnotation()!!.description + val description = property.javaField!!.getAnnotation(Argument::class.java).description val possibleValues = property.gradleValues.possibleValues val defaultValue = property.gradleValues.defaultValue @@ -896,7 +897,7 @@ private fun generateMarkdown(properties: List>) { if (name == "includeRuntime") continue // This option has no effect in Gradle builds val renderName = listOfNotNull("`$name`", property.findAnnotation()?.let { "__(Deprecated)__" }) .joinToString(" ") - val description = property.findAnnotation()!!.description + val description = property.javaField!!.getAnnotation(Argument::class.java).description val possibleValues = property.gradleValues.possibleValues val defaultValue = when (property.gradleDefaultValue) { "null" -> "" diff --git a/jps/jps-common/src/org/jetbrains/kotlin/arguments/CompilerArgumentsContentProspector.kt b/jps/jps-common/src/org/jetbrains/kotlin/arguments/CompilerArgumentsContentProspector.kt index 92b01d4659c..789694404ef 100644 --- a/jps/jps-common/src/org/jetbrains/kotlin/arguments/CompilerArgumentsContentProspector.kt +++ b/jps/jps-common/src/org/jetbrains/kotlin/arguments/CompilerArgumentsContentProspector.kt @@ -9,6 +9,7 @@ import kotlin.reflect.KClass import kotlin.reflect.KProperty1 import kotlin.reflect.KType import kotlin.reflect.full.memberProperties +import kotlin.reflect.jvm.javaField object CompilerArgumentsContentProspector { private val argumentPropertiesCache: MutableMap, Collection>> = @@ -24,7 +25,7 @@ object CompilerArgumentsContentProspector { mutableMapOf() private fun getCompilerArgumentsProperties(kClass: KClass) = argumentPropertiesCache.getOrPut(kClass) { - kClass.memberProperties.filter { prop -> prop.annotations.any { it is Argument } } + kClass.memberProperties.filter { prop -> prop.javaField?.getAnnotation(Argument::class.java) != null } } private inline fun Collection>.filterByReturnType(predicate: (KType?) -> Boolean) = diff --git a/jps/jps-common/src/org/jetbrains/kotlin/config/KotlinFacetSettings.kt b/jps/jps-common/src/org/jetbrains/kotlin/config/KotlinFacetSettings.kt index 8fa5d30940d..0c3a7461d2c 100644 --- a/jps/jps-common/src/org/jetbrains/kotlin/config/KotlinFacetSettings.kt +++ b/jps/jps-common/src/org/jetbrains/kotlin/config/KotlinFacetSettings.kt @@ -15,7 +15,7 @@ import org.jetbrains.kotlin.platform.jvm.JvmPlatforms import org.jetbrains.kotlin.utils.DescriptionAware import org.jetbrains.kotlin.utils.addToStdlib.safeAs import kotlin.reflect.KProperty1 -import kotlin.reflect.full.findAnnotation +import kotlin.reflect.jvm.javaField @Deprecated("Use IdePlatformKind instead.", level = DeprecationLevel.ERROR) sealed class TargetPlatformKind( @@ -206,7 +206,7 @@ class KotlinFacetSettings { val isEnabledByCompilerArgument = compilerArguments?.safeAs()?.let(settingReference::get) if (isEnabledByCompilerArgument == true) return true val isEnabledByAdditionalSettings = run { - val stringArgumentName = settingReference.findAnnotation()?.value ?: return@run null + val stringArgumentName = settingReference.javaField?.getAnnotation(Argument::class.java)?.value ?: return@run null compilerSettings?.additionalArguments?.contains(stringArgumentName, ignoreCase = true) } return isEnabledByAdditionalSettings ?: false diff --git a/libraries/tools/kotlin-gradle-plugin/src/functionalTest/kotlin/org/jetbrains/kotlin/gradle/unitTests/compilerArgumetns/KotlinCompileArgumentsTest.kt b/libraries/tools/kotlin-gradle-plugin/src/functionalTest/kotlin/org/jetbrains/kotlin/gradle/unitTests/compilerArgumetns/KotlinCompileArgumentsTest.kt index deeb4dbaae8..825d3ccf76d 100644 --- a/libraries/tools/kotlin-gradle-plugin/src/functionalTest/kotlin/org/jetbrains/kotlin/gradle/unitTests/compilerArgumetns/KotlinCompileArgumentsTest.kt +++ b/libraries/tools/kotlin-gradle-plugin/src/functionalTest/kotlin/org/jetbrains/kotlin/gradle/unitTests/compilerArgumetns/KotlinCompileArgumentsTest.kt @@ -28,7 +28,7 @@ import org.jetbrains.kotlin.gradle.util.assertNotNull import org.jetbrains.kotlin.gradle.util.buildProjectWithJvm import org.jetbrains.kotlin.gradle.util.buildProjectWithMPP import org.jetbrains.kotlin.gradle.util.main -import kotlin.reflect.full.findAnnotation +import kotlin.reflect.jvm.javaField import kotlin.test.* @@ -78,7 +78,7 @@ class KotlinCompileArgumentsTest { val arguments = mainCompilationTask.createCompilerArguments(lenient) val argumentsString = ArgumentUtils.convertArgumentsToStringList(arguments) - val jvmTargetArgument = K2JVMCompilerArguments::jvmTarget.findAnnotation()!!.value + val jvmTargetArgument = K2JVMCompilerArguments::jvmTarget.javaField!!.getAnnotation(Argument::class.java)!!.value if (jvmTargetArgument !in argumentsString) fail("Missing '$jvmTargetArgument' in argument list") val indexOfJvmTargetArgument = argumentsString.indexOf(jvmTargetArgument) val jvmTargetTargetArgumentValue = argumentsString.getOrNull(indexOfJvmTargetArgument + 1) @@ -158,4 +158,4 @@ class KotlinCompileArgumentsTest { compileTask.createCompilerArguments(lenient).fragmentSources.orEmpty().toSet() ) } -} \ No newline at end of file +} diff --git a/plugins/scripting/scripting-compiler/src/org/jetbrains/kotlin/scripting/compiler/plugin/impl/errorReporting.kt b/plugins/scripting/scripting-compiler/src/org/jetbrains/kotlin/scripting/compiler/plugin/impl/errorReporting.kt index 93b756e4187..b2121234e88 100644 --- a/plugins/scripting/scripting-compiler/src/org/jetbrains/kotlin/scripting/compiler/plugin/impl/errorReporting.kt +++ b/plugins/scripting/scripting-compiler/src/org/jetbrains/kotlin/scripting/compiler/plugin/impl/errorReporting.kt @@ -17,8 +17,8 @@ import org.jetbrains.kotlin.diagnostics.impl.BaseDiagnosticsCollector import org.jetbrains.kotlin.diagnostics.rendering.RootDiagnosticRendererFactory import org.jetbrains.kotlin.psi import org.jetbrains.kotlin.scripting.definitions.MessageReporter -import org.jetbrains.kotlin.utils.addToStdlib.firstIsInstanceOrNull import kotlin.reflect.KMutableProperty1 +import kotlin.reflect.jvm.javaField import kotlin.script.experimental.api.ResultWithDiagnostics import kotlin.script.experimental.api.ScriptDiagnostic import kotlin.script.experimental.api.SourceCode @@ -194,7 +194,7 @@ private fun reportInvalidArguments( ): Boolean { val invalidArgKeys = toIgnore.mapNotNull { argProperty -> if (argProperty.get(arguments) != argProperty.get(reportingState.currentArguments)) { - argProperty.annotations.firstIsInstanceOrNull()?.value + argProperty.javaField?.getAnnotation(Argument::class.java)?.value ?: throw IllegalStateException("unknown compiler argument property: $argProperty: no Argument annotation found") } else null }