From b9067e6462f484116dd75d68c826eebe6d658e94 Mon Sep 17 00:00:00 2001 From: Alexander Udalov Date: Wed, 17 Aug 2016 15:56:08 +0300 Subject: [PATCH] Introduce reflection cache in KClassImpl and KPackageImpl Most of KClassImpl operations should be cached, but creating a lazy value for each operation would significantly increase memory footprint of a KClassImpl instance. Therefore we decide to store all lazy values under a single lazy value named 'data' which is stored in KClassImpl. There's a minor overhead of indirection on any operation now, however it'll allow us to substantially increase reflection performance by caching everything we can --- .../kotlin/code/ReflectionCodeSanityTest.kt | 75 ++++++++++++++++--- .../kotlin/reflect/jvm/internal/KClassImpl.kt | 52 ++++++------- .../jvm/internal/KDeclarationContainerImpl.kt | 11 ++- .../reflect/jvm/internal/KPackageImpl.kt | 45 +++++------ 4 files changed, 119 insertions(+), 64 deletions(-) diff --git a/compiler/tests/org/jetbrains/kotlin/code/ReflectionCodeSanityTest.kt b/compiler/tests/org/jetbrains/kotlin/code/ReflectionCodeSanityTest.kt index 8b9c702059b..1422b4ef5fe 100644 --- a/compiler/tests/org/jetbrains/kotlin/code/ReflectionCodeSanityTest.kt +++ b/compiler/tests/org/jetbrains/kotlin/code/ReflectionCodeSanityTest.kt @@ -20,28 +20,48 @@ import junit.framework.TestCase import org.jetbrains.kotlin.codegen.forTestCompile.ForTestCompileRuntime import org.jetbrains.kotlin.load.java.JvmAbi import java.lang.reflect.Field +import java.lang.reflect.Modifier +import kotlin.reflect.jvm.javaField class ReflectionCodeSanityTest : TestCase() { - fun testNoDelegatedPropertiesInKClassAndKProperties() { - val classLoader = ForTestCompileRuntime.runtimeAndReflectJarClassLoader() + private lateinit var classLoader: ClassLoader - val classesToCheck = linkedSetOf>() + override fun setUp() { + super.setUp() + classLoader = ForTestCompileRuntime.runtimeAndReflectJarClassLoader() + } + + override fun tearDown() { + ReflectionCodeSanityTest::classLoader.javaField!!.set(this, null) + super.tearDown() + } + + private fun loadClass(name: String): Class<*> = + classLoader.loadClass("kotlin.reflect.jvm.internal.$name") + + private fun collectClassesWithSupers(vararg names: String): Set> { + val result = linkedSetOf>() fun addClassToCheck(klass: Class<*>) { - if (classesToCheck.add(klass)) { - @Suppress("UNNECESSARY_SAFE_CALL") // https://youtrack.jetbrains.com/issue/KT-9294 - klass.superclass?.let { addClassToCheck(it) } + if (result.add(klass)) { + klass.superclass?.let(::addClassToCheck) } } - fun addClass(name: String) = addClassToCheck(classLoader.loadClass("kotlin.reflect.jvm.internal.$name")) + for (name in names) { + addClassToCheck(loadClass(name)) + } - addClass("KClassImpl") - addClass("KMutableProperty0Impl") - addClass("KMutableProperty1Impl") - addClass("KMutableProperty2Impl") + return result + } + fun testNoDelegatedPropertiesInKClassAndKProperties() { val badFields = linkedSetOf() - for (klass in classesToCheck) { + for (klass in collectClassesWithSupers( + "KClassImpl", + "KMutableProperty0Impl", + "KMutableProperty1Impl", + "KMutableProperty2Impl" + )) { badFields.addAll(klass.declaredFields.filter { it.name.endsWith(JvmAbi.DELEGATED_PROPERTY_NAME_SUFFIX) }) } @@ -54,4 +74,35 @@ class ReflectionCodeSanityTest : TestCase() { badFields.joinToString("\n")) } } + + fun testMaxAllowedFields() { + // The following classes are instantiated a lot in Kotlin applications and thus they should be optimized as best as possible. + // This test checks that these classes have not more fields than a predefined small number, which can usually be calculated as + // the number of constructor parameters (number of objects needed to initialize an instance) + 1 for 'data', the reflection cache. + val classesWithMaxAllowedFields = linkedMapOf( + "KClassImpl" to 2, // jClass, data + "KPackageImpl" to 3 // jClass, moduleName, data + ) + + val badClasses = linkedMapOf, Collection>() + for ((className, maxAllowedFields) in classesWithMaxAllowedFields) { + val klass = loadClass(className) + val fields = generateSequence(klass) { it.superclass } + .flatMap { it.declaredFields.asSequence() } + .filterNot { Modifier.isStatic(it.modifiers) } + .toList() + if (fields.size > maxAllowedFields) { + badClasses[klass] = fields + } + } + + if (badClasses.isNotEmpty()) { + fail("Some classes in reflection.jvm contain more fields than it is allowed. Please optimize storage in these classes:\n\n" + + badClasses.entries.joinToString("\n") { entry -> + val (klass, fields) = entry + "$klass has ${fields.size} fields but max allowed = ${classesWithMaxAllowedFields[klass.simpleName]}:\n" + + fields.joinToString("\n") { " $it" } + }) + } + } } diff --git a/core/reflection.jvm/src/kotlin/reflect/jvm/internal/KClassImpl.kt b/core/reflection.jvm/src/kotlin/reflect/jvm/internal/KClassImpl.kt index 8a469a9506e..df51b973ad4 100644 --- a/core/reflection.jvm/src/kotlin/reflect/jvm/internal/KClassImpl.kt +++ b/core/reflection.jvm/src/kotlin/reflect/jvm/internal/KClassImpl.kt @@ -35,18 +35,36 @@ import kotlin.reflect.* internal class KClassImpl(override val jClass: Class) : KDeclarationContainerImpl(), KClass, KClassifierImpl, KAnnotatedElementImpl { - private val descriptor_ = ReflectProperties.lazySoft { - val classId = classId + private inner class Data : KDeclarationContainerImpl.Data() { + val descriptor: ClassDescriptor by ReflectProperties.lazySoft { + val classId = classId + val moduleData = data().moduleData - val descriptor = - if (classId.isLocal) moduleData.deserialization.deserializeClass(classId) - else moduleData.module.findClassAcrossModuleDependencies(classId) + val descriptor = + if (classId.isLocal) moduleData.deserialization.deserializeClass(classId) + else moduleData.module.findClassAcrossModuleDependencies(classId) - descriptor ?: reportUnresolvedClass() + descriptor ?: reportUnresolvedClass() + } + + @Suppress("UNCHECKED_CAST") + val objectInstance: T? by ReflectProperties.lazy { + val descriptor = descriptor + if (descriptor.kind != ClassKind.OBJECT) return@lazy null + + val field = if (descriptor.isCompanionObject && !CompanionObjectMapping.isMappedIntrinsicCompanionObject(descriptor)) { + jClass.enclosingClass.getDeclaredField(descriptor.name.asString()) + } + else { + jClass.getDeclaredField(JvmAbi.INSTANCE_FIELD) + } + field.get(null) as T + } } - override val descriptor: ClassDescriptor - get() = descriptor_() + private val data = ReflectProperties.lazy { Data() } + + override val descriptor: ClassDescriptor get() = data().descriptor override val annotated: Annotated get() = descriptor @@ -70,7 +88,6 @@ internal class KClassImpl(override val jClass: Class) : return emptyList() } - @Suppress("UNCHECKED_CAST") override fun getProperties(name: Name): Collection = (memberScope.getContributedVariables(name, NoLookupLocation.FROM_REFLECTION) + staticScope.getContributedVariables(name, NoLookupLocation.FROM_REFLECTION)) @@ -123,22 +140,7 @@ internal class KClassImpl(override val jClass: Class) : jClass?.let { KClassImpl(it) } } - @Suppress("UNCHECKED_CAST") - private val objectInstance_ = ReflectProperties.lazy { - val descriptor = descriptor - if (descriptor.kind != ClassKind.OBJECT) return@lazy null - - val field = if (descriptor.isCompanionObject && !CompanionObjectMapping.isMappedIntrinsicCompanionObject(descriptor)) { - jClass.enclosingClass.getDeclaredField(descriptor.name.asString()) - } - else { - jClass.getDeclaredField(JvmAbi.INSTANCE_FIELD) - } - field.get(null) as T - } - - override val objectInstance: T? - get() = objectInstance_() + override val objectInstance: T? get() = data().objectInstance override fun isInstance(value: Any?): Boolean { // TODO: use Kotlin semantics for mutable/read-only collections once KT-11754 is supported (see TypeIntrinsics) diff --git a/core/reflection.jvm/src/kotlin/reflect/jvm/internal/KDeclarationContainerImpl.kt b/core/reflection.jvm/src/kotlin/reflect/jvm/internal/KDeclarationContainerImpl.kt index 36dafe2f949..e2ec95a015a 100644 --- a/core/reflection.jvm/src/kotlin/reflect/jvm/internal/KDeclarationContainerImpl.kt +++ b/core/reflection.jvm/src/kotlin/reflect/jvm/internal/KDeclarationContainerImpl.kt @@ -31,14 +31,13 @@ import kotlin.jvm.internal.ClassBasedDeclarationContainer import kotlin.reflect.KotlinReflectionInternalError internal abstract class KDeclarationContainerImpl : ClassBasedDeclarationContainer { - // Note: this is stored here on a soft reference to prevent GC from destroying the weak reference to it in the moduleByClassLoader cache - private val moduleData_ = ReflectProperties.lazySoft { - jClass.getOrCreateModule() + protected abstract inner class Data { + // This is stored here on a soft reference to prevent GC from destroying the weak reference to it in the moduleByClassLoader cache + val moduleData: RuntimeModuleData by ReflectProperties.lazySoft { + jClass.getOrCreateModule() + } } - val moduleData: RuntimeModuleData - get() = moduleData_() - protected open val methodOwner: Class<*> get() = jClass diff --git a/core/reflection.jvm/src/kotlin/reflect/jvm/internal/KPackageImpl.kt b/core/reflection.jvm/src/kotlin/reflect/jvm/internal/KPackageImpl.kt index 2837b5f5f6d..827180528ae 100644 --- a/core/reflection.jvm/src/kotlin/reflect/jvm/internal/KPackageImpl.kt +++ b/core/reflection.jvm/src/kotlin/reflect/jvm/internal/KPackageImpl.kt @@ -16,10 +16,7 @@ package kotlin.reflect.jvm.internal -import org.jetbrains.kotlin.descriptors.ConstructorDescriptor -import org.jetbrains.kotlin.descriptors.FunctionDescriptor -import org.jetbrains.kotlin.descriptors.PackageFragmentDescriptor -import org.jetbrains.kotlin.descriptors.PropertyDescriptor +import org.jetbrains.kotlin.descriptors.* import org.jetbrains.kotlin.incremental.components.NoLookupLocation import org.jetbrains.kotlin.load.java.lazy.descriptors.LazyJavaPackageFragment import org.jetbrains.kotlin.load.java.structure.reflect.classId @@ -31,27 +28,33 @@ import org.jetbrains.kotlin.serialization.deserialization.descriptors.Deserializ import kotlin.reflect.KCallable internal class KPackageImpl(override val jClass: Class<*>, val moduleName: String) : KDeclarationContainerImpl() { - private val descriptor = ReflectProperties.lazySoft { - with(moduleData) { - packageFacadeProvider.registerModule(moduleName) - module.getPackage(jClass.classId.packageFqName) + private inner class Data : KDeclarationContainerImpl.Data() { + val descriptor: PackageViewDescriptor by ReflectProperties.lazySoft { + with(moduleData) { + packageFacadeProvider.registerModule(moduleName) + module.getPackage(jClass.classId.packageFqName) + } + } + + val methodOwner: Class<*> by ReflectProperties.lazy { + val facadeName = ReflectKotlinClass.create(jClass)?.classHeader?.multifileClassName + // We need to check isNotEmpty because this is the value read from the annotation which cannot be null. + // The default value for 'xs' is empty string, as declared in kotlin.Metadata + // TODO: do not read ReflectKotlinClass multiple times, obtain facade name from descriptor + if (facadeName != null && facadeName.isNotEmpty()) { + jClass.classLoader.loadClass(facadeName.replace('/', '.')) + } + else { + jClass + } } } - override val methodOwner: Class<*> by lazy(LazyThreadSafetyMode.PUBLICATION) { - val facadeName = ReflectKotlinClass.create(jClass)?.classHeader?.multifileClassName - // We need to check isNotEmpty because this is the value read from the annotation which cannot be null. - // The default value for 'xs' is empty string, as declared in kotlin.Metadata - // TODO: do not read ReflectKotlinClass multiple times, obtain facade name from descriptor - if (facadeName != null && facadeName.isNotEmpty()) { - jClass.classLoader.loadClass(facadeName.replace('/', '.')) - } - else { - jClass - } - } + private val data = ReflectProperties.lazy { Data() } - internal val scope: MemberScope get() = descriptor().memberScope + override val methodOwner: Class<*> get() = data().methodOwner + + private val scope: MemberScope get() = data().descriptor.memberScope override val members: Collection> get() = getMembers(scope, declaredOnly = false, nonExtensions = true, extensions = true).filter { member ->