Use fields for spilled variables for lambda parameters as well

In other words, do not generate p$ fields if we can use L$ fields, which are being cleaned up. #KT-16222 Fixed
2020-08-11 18:51:14 +02:00
parent f5ab3a445f
commit bcbb050326
16 changed files with 192 additions and 96 deletions
@@ -43,7 +43,6 @@ import org.jetbrains.kotlin.resolve.jvm.jvmSignature.JvmMethodSignature
 import org.jetbrains.kotlin.serialization.DescriptorSerializer
 import org.jetbrains.kotlin.types.KotlinType
 import org.jetbrains.kotlin.types.typeUtil.makeNullable
-import org.jetbrains.kotlin.utils.addToStdlib.safeAs
 import org.jetbrains.kotlin.utils.sure
 import org.jetbrains.org.objectweb.asm.Label
 import org.jetbrains.org.objectweb.asm.MethodVisitor
@@ -104,9 +103,9 @@ abstract class AbstractCoroutineCodegen(
        ValueParameterDescriptorImpl(
            this, null, index, Annotations.EMPTY, name,
            type,
-            false, false,
-            false,
-            null, SourceElement.NO_SOURCE
+            declaresDefaultValue = false, isCrossinline = false,
+            isNoinline = false,
+            varargElementType = null, source = SourceElement.NO_SOURCE
        )

    override fun generateConstructor(): Method {
@@ -157,7 +156,7 @@ abstract class AbstractCoroutineCodegen(
        return constructor
    }

-    abstract protected val passArityToSuperClass: Boolean
+    protected abstract val passArityToSuperClass: Boolean
 }

 class CoroutineCodegenForLambda private constructor(
@@ -187,6 +186,23 @@ class CoroutineCodegenForLambda private constructor(

    private val endLabel = Label()

+    private val varsCountByType = hashMapOf<Type, Int>()
+
+    private val fieldsForParameters: Map<ParameterDescriptor, FieldInfo> = createFieldsForParameters()
+
+    private fun createFieldsForParameters(): Map<ParameterDescriptor, FieldInfo> {
+        val result = hashMapOf<ParameterDescriptor, FieldInfo>()
+        for (parameter in allFunctionParameters()) {
+            if (parameter.isUnused()) continue
+            val type = state.typeMapper.mapType(parameter.type)
+            val normalizedType = type.normalize()
+            val index = varsCountByType[normalizedType]?.plus(1) ?: 0
+            varsCountByType[normalizedType] = index
+            result[parameter] = createHiddenFieldInfo(parameter.type, "${normalizedType.descriptor[0]}$$index")
+        }
+        return result
+    }
+
    private fun getCreateFunction(): SimpleFunctionDescriptor = SimpleFunctionDescriptorImpl.create(
        funDescriptor.containingDeclaration,
        Annotations.EMPTY,
@@ -232,8 +248,7 @@ class CoroutineCodegenForLambda private constructor(

    override fun generateClosureBody() {
        for (parameter in allFunctionParameters()) {
-            if (parameter.isUnused()) continue
-            val fieldInfo = parameter.getFieldInfoForCoroutineLambdaParameter()
+            val fieldInfo = fieldsForParameters[parameter] ?: continue
            v.newField(
                OtherOrigin(parameter),
                Opcodes.ACC_PRIVATE + Opcodes.ACC_SYNTHETIC,
@@ -257,7 +272,7 @@ class CoroutineCodegenForLambda private constructor(
        super.generateBody()

        if (doNotGenerateInvokeBridge) {
-            v.serializationBindings.put<FunctionDescriptor, Method>(
+            v.serializationBindings.put(
                METHOD_FOR_FUNCTION,
                originalSuspendFunctionDescriptor,
                typeMapper.mapAsmMethod(erasedInvokeFunction)
@@ -320,7 +335,7 @@ class CoroutineCodegenForLambda private constructor(
            newarray(AsmTypes.OBJECT_TYPE)
            // 0 - this
            // 1..22 - parameters
-            // 23 - first empy slot
+            // 23 - first empty slot
            val arraySlot = 23
            store(arraySlot, AsmTypes.OBJECT_TYPE)
            for ((varIndex, type) in parameterTypes.withVariableIndices()) {
@@ -405,8 +420,8 @@ class CoroutineCodegenForLambda private constructor(
            // Pass lambda parameters to 'invoke' call on newly constructed object
            var index = 1
            for (parameter in allFunctionParameters()) {
-                val fieldInfoForCoroutineLambdaParameter = parameter.getFieldInfoForCoroutineLambdaParameter()
-                if (!parameter.isUnused()) {
+                val fieldInfoForCoroutineLambdaParameter = fieldsForParameters[parameter]
+                if (fieldInfoForCoroutineLambdaParameter != null) {
                    if (isBigArity) {
                        load(cloneIndex, fieldInfoForCoroutineLambdaParameter.ownerType)
                        load(1, AsmTypes.OBJECT_TYPE)
@@ -442,7 +457,7 @@ class CoroutineCodegenForLambda private constructor(
                        )
                    }
                }
-                index += if (isBigArity || generateErasedCreate) 1 else fieldInfoForCoroutineLambdaParameter.fieldType.size
+                index += if (isBigArity || generateErasedCreate) 1 else state.typeMapper.mapType(parameter.type).size
            }

            load(cloneIndex, AsmTypes.OBJECT_TYPE)
@@ -452,17 +467,17 @@ class CoroutineCodegenForLambda private constructor(

    private fun ExpressionCodegen.initializeCoroutineParameters() {
        for (parameter in allFunctionParameters()) {
-            if (parameter.isUnused()) continue
-            val fieldStackValue =
-                StackValue.field(
-                    parameter.getFieldInfoForCoroutineLambdaParameter(), generateThisOrOuter(context.thisDescriptor, false)
-                )
+            val fieldForParameter = fieldsForParameters[parameter] ?: continue
+            val fieldStackValue = StackValue.field(fieldForParameter, generateThisOrOuter(context.thisDescriptor, false))

-            val mappedType = typeMapper.mapType(parameter.type)
-            fieldStackValue.put(mappedType, v)
+            val originalType = typeMapper.mapType(parameter.type)
+            val normalizedType = originalType.normalize()
+            fieldStackValue.put(normalizedType, v)

-            val newIndex = myFrameMap.enter(parameter, mappedType)
-            v.store(newIndex, mappedType)
+            val newIndex = myFrameMap.enter(parameter, originalType)
+            // FIXME: Coerce int
+            StackValue.coerce(normalizedType, originalType, v)
+            v.store(newIndex, originalType)

            val name =
                if (parameter is ReceiverParameterDescriptor)
@@ -471,7 +486,7 @@ class CoroutineCodegenForLambda private constructor(
                    (getNameForDestructuredParameterOrNull(parameter as ValueParameterDescriptor) ?: parameter.name.asString())
            val label = Label()
            v.mark(label)
-            v.visitLocalVariable(name, mappedType.descriptor, null, label, endLabel, newIndex)
+            v.visitLocalVariable(name, originalType.descriptor, null, label, endLabel, newIndex)
        }

        initializeVariablesForDestructuredLambdaParameters(
@@ -485,13 +500,10 @@ class CoroutineCodegenForLambda private constructor(
        originalSuspendFunctionDescriptor.extensionReceiverParameter.let(::listOfNotNull) +
                originalSuspendFunctionDescriptor.valueParameters

-    private fun ParameterDescriptor.getFieldInfoForCoroutineLambdaParameter() =
-        createHiddenFieldInfo(type, COROUTINE_LAMBDA_PARAMETER_PREFIX + (this.safeAs<ValueParameterDescriptor>()?.index ?: ""))
-
    private fun createHiddenFieldInfo(type: KotlinType, name: String) =
        FieldInfo.createForHiddenField(
            typeMapper.mapClass(closureContext.thisDescriptor),
-            typeMapper.mapType(type),
+            typeMapper.mapType(type).normalize(),
            type,
            name
        )
@@ -514,7 +526,8 @@ class CoroutineCodegenForLambda private constructor(
                        isForNamedFunction = false,
                        languageVersionSettings = languageVersionSettings,
                        disableTailCallOptimizationForFunctionReturningUnit = false,
-                        useOldSpilledVarTypeAnalysis = state.configuration.getBoolean(JVMConfigurationKeys.USE_OLD_SPILLED_VAR_TYPE_ANALYSIS)
+                        useOldSpilledVarTypeAnalysis = state.configuration.getBoolean(JVMConfigurationKeys.USE_OLD_SPILLED_VAR_TYPE_ANALYSIS),
+                        initialVarsCountByType = varsCountByType
                    )
                    val maybeWithForInline = if (forInline)
                        SuspendForInlineCopyingMethodVisitor(stateMachineBuilder, access, name, desc, functionCodegen::newMethod)
@@ -758,16 +771,11 @@ class CoroutineCodegenForNamedFunction private constructor(
            declaration: KtFunction
        ): CoroutineCodegenForNamedFunction {
            val bindingContext = expressionCodegen.state.bindingContext
-            val closure =
-                bindingContext[
-                        CodegenBinding.CLOSURE,
-                        bindingContext[CodegenBinding.CLASS_FOR_CALLABLE, originalSuspendDescriptor]
-                ].sure { "There must be a closure defined for $originalSuspendDescriptor" }
+            val closure = bindingContext[CLOSURE, bindingContext[CodegenBinding.CLASS_FOR_CALLABLE, originalSuspendDescriptor]]
+                .sure { "There must be a closure defined for $originalSuspendDescriptor" }

-            val suspendFunctionView =
-                bindingContext[
-                        CodegenBinding.SUSPEND_FUNCTION_TO_JVM_VIEW, originalSuspendDescriptor
-                ].sure { "There must be a jvm view defined for $originalSuspendDescriptor" }
+            val suspendFunctionView = bindingContext[CodegenBinding.SUSPEND_FUNCTION_TO_JVM_VIEW, originalSuspendDescriptor]
+                .sure { "There must be a jvm view defined for $originalSuspendDescriptor" }

            if (suspendFunctionView.dispatchReceiverParameter != null) {
                closure.setNeedsCaptureOuterClass()
@@ -785,10 +793,8 @@ class CoroutineCodegenForNamedFunction private constructor(
    }
 }

-private const val COROUTINE_LAMBDA_PARAMETER_PREFIX = "p$"
-
 private object FailingFunctionGenerationStrategy : FunctionGenerationStrategy() {
-    override fun skipNotNullAssertionsForParameters(): kotlin.Boolean {
+    override fun skipNotNullAssertionsForParameters(): Boolean {
        error("This functions must not be called")
    }

@@ -64,7 +64,9 @@ class CoroutineTransformerMethodVisitor(
    // JVM_IR backend generates $completion, while old backend does not
    private val putContinuationParameterToLvt: Boolean = true,
    // New SourceInterpreter-less analyser can be somewhat unstable, disable it
-    private val useOldSpilledVarTypeAnalysis: Boolean = false
+    private val useOldSpilledVarTypeAnalysis: Boolean = false,
+    // Parameters of suspend lambda are put to the same fields as spilled variables
+    private val initialVarsCountByType: Map<Type, Int> = emptyMap()
 ) : TransformationMethodVisitor(delegate, access, name, desc, signature, exceptions) {

    private val classBuilderForCoroutineState: ClassBuilder by lazy(obtainClassBuilderForCoroutineState)
@@ -200,7 +202,6 @@ class CoroutineTransformerMethodVisitor(
    // In this case the variables are uninitialized, initialize them
    private fun initializeFakeInlinerVariables(methodNode: MethodNode, stateLabels: List<LabelNode>) {
        for (stateLabel in stateLabels) {
-            val unitializedFakeInlinerVariables = arrayListOf<LocalVariableNode>()
            for (record in methodNode.localVariables) {
                if (isFakeLocalVariableForInline(record.name) &&
                    methodNode.instructions.indexOf(record.start) < methodNode.instructions.indexOf(stateLabel) &&
@@ -597,6 +598,14 @@ class CoroutineTransformerMethodVisitor(
        fun AbstractInsnNode.index() = instructions.indexOf(this)

        val maxVarsCountByType = mutableMapOf<Type, Int>()
+        var initialSpilledVariablesCount = 0
+        for ((type, count) in initialVarsCountByType) {
+            if (type == AsmTypes.OBJECT_TYPE) {
+                initialSpilledVariablesCount = count
+            }
+            maxVarsCountByType[type] = count
+        }
+
        val livenessFrames = analyzeLiveness(methodNode)

        // References shall be cleaned up after uspill (during spill in next suspension point) to prevent memory leaks,
@@ -703,7 +712,8 @@ class CoroutineTransformerMethodVisitor(
            val currentSpilledReferencesCount = countVariablesToSpill(suspensionPointIndex)
            val preds = predSuspensionPoints[suspensionPoint]
            val predSpilledReferencesCount =
-                if (preds.isNullOrEmpty()) 0 else preds.maxOf { countVariablesToSpill(suspensionPoints.indexOf(it)) }
+                if (preds.isNullOrEmpty()) initialSpilledVariablesCount
+                else preds.maxOf { countVariablesToSpill(suspensionPoints.indexOf(it)) }
            referencesToCleanBySuspensionPointIndex += currentSpilledReferencesCount to predSpilledReferencesCount
        }

@@ -779,7 +789,7 @@ class CoroutineTransformerMethodVisitor(

        for (entry in maxVarsCountByType) {
            val (type, maxIndex) = entry
-            for (index in 0..maxIndex) {
+            for (index in (initialVarsCountByType[type]?.plus(1) ?: 0)..maxIndex) {
                classBuilderForCoroutineState.newField(
                    JvmDeclarationOrigin.NO_ORIGIN, AsmUtil.NO_FLAG_PACKAGE_PRIVATE,
                    type.fieldNameForVar(index), type.descriptor, null, null
@@ -835,7 +845,7 @@ class CoroutineTransformerMethodVisitor(
    private val SuspensionPoint.tryCatchBlockEndLabelAfterSuspensionCall: LabelNode
        get() {
            assert(suspensionCallEnd.next is LabelNode) {
-                "Next instruction after ${this} should be a label, but " +
+                "Next instruction after $this should be a label, but " +
                        "${suspensionCallEnd.next::class.java}/${suspensionCallEnd.next.opcode} was found"
            }

@@ -936,9 +946,11 @@ class CoroutineTransformerMethodVisitor(
    private fun nextDefinitelyHitLineNumber(suspension: SuspensionPoint): LineNumberNode? {
        var next = suspension.suspensionCallEnd.next
        while (next != null) {
-            if (next.isBranchOrCall) return null
-            else if (next is LineNumberNode) return next
-            else next = next.next
+            when {
+                next.isBranchOrCall -> return null
+                next is LineNumberNode -> return next
+                else -> next = next.next
+            }
        }
        return next
    }
@@ -1072,7 +1084,7 @@ inline fun withInstructionAdapter(block: InstructionAdapter.() -> Unit): InsnLis
    return tmpMethodNode.instructions
 }

-private fun Type.normalize() =
+internal fun Type.normalize() =
    when (sort) {
        Type.ARRAY, Type.OBJECT -> AsmTypes.OBJECT_TYPE
        else -> this