From d22e4495de18c4ad01e3213e46c872337b5afb0a Mon Sep 17 00:00:00 2001 From: Zalim Bashorov Date: Tue, 21 Jan 2020 17:45:47 +0300 Subject: [PATCH] [JS IR DCE] Improve handling overrides Introduce the notion of contagious for overridable declarations. So, now overriding uncontagious declaration will not lead to becoming a declaration reachable. By default, all declarations treated as contagious, it's not the most efficient, but it's safest. In case when we access a declaration through a fake-override declaration, the original (real) one will not be marked as contagious, so, later, other overrides will not be processed unconditionally only because it overrides a reachable declaration. --- .../kotlin/backend/common/ir/IrUtils.kt | 8 ++- .../org/jetbrains/kotlin/ir/backend/js/Dce.kt | 57 ++++++++++++------- 2 files changed, 45 insertions(+), 20 deletions(-) diff --git a/compiler/ir/backend.common/src/org/jetbrains/kotlin/backend/common/ir/IrUtils.kt b/compiler/ir/backend.common/src/org/jetbrains/kotlin/backend/common/ir/IrUtils.kt index 12e6bd7c42b..32e5d80fd81 100644 --- a/compiler/ir/backend.common/src/org/jetbrains/kotlin/backend/common/ir/IrUtils.kt +++ b/compiler/ir/backend.common/src/org/jetbrains/kotlin/backend/common/ir/IrUtils.kt @@ -1,5 +1,5 @@ /* - * Copyright 2010-2019 JetBrains s.r.o. and Kotlin Programming Language contributors. + * Copyright 2010-2020 JetBrains s.r.o. and Kotlin Programming Language contributors. * Use of this source code is governed by the Apache 2.0 license that can be found in the license/LICENSE.txt file. */ @@ -106,6 +106,12 @@ val IrSimpleFunction.isOverridable: Boolean val IrSimpleFunction.isOverridableOrOverrides: Boolean get() = isOverridable || overriddenSymbols.isNotEmpty() +val IrDeclaration.isMemberOfOpenClass: Boolean + get() { + val parentClass = this.parent as? IrClass ?: return false + return !parentClass.isFinalClass + } + fun IrReturnTarget.returnType(context: CommonBackendContext) = when (this) { is IrConstructor -> context.irBuiltIns.unitType diff --git a/compiler/ir/backend.js/src/org/jetbrains/kotlin/ir/backend/js/Dce.kt b/compiler/ir/backend.js/src/org/jetbrains/kotlin/ir/backend/js/Dce.kt index ef5f9abfe39..c0cd1096b3c 100644 --- a/compiler/ir/backend.js/src/org/jetbrains/kotlin/ir/backend/js/Dce.kt +++ b/compiler/ir/backend.js/src/org/jetbrains/kotlin/ir/backend/js/Dce.kt @@ -5,6 +5,7 @@ package org.jetbrains.kotlin.ir.backend.js +import org.jetbrains.kotlin.backend.common.ir.isMemberOfOpenClass import org.jetbrains.kotlin.ir.IrElement import org.jetbrains.kotlin.ir.backend.js.export.isExported import org.jetbrains.kotlin.ir.backend.js.utils.getJsName @@ -97,6 +98,7 @@ private fun removeUselessDeclarations(module: IrModuleFragment, usefulDeclaratio } } +// TODO refactor it, the function became too big. Please contact me (Zalim) before doing it. fun usefulDeclarations(roots: Iterable, context: JsIrBackendContext): Set { val printReachabilityInfo = context.configuration.getBoolean(JSConfigurationKeys.PRINT_REACHABILITY_INFO) || @@ -105,31 +107,48 @@ fun usefulDeclarations(roots: Iterable, context: JsIrBackendConte val queue = ArrayDeque() val result = hashSetOf() + + // This collection contains declarations whose reachability should be propagated to overrides. + // Overriding uncontagious declaration will not lead to becoming a declaration reachable. + // By default, all declarations treated as contagious, it's not the most efficient, but it's safest. + // In case when we access a declaration through a fake-override declaration, the original (real) one will not be marked as contagious, + // so, later, other overrides will not be processed unconditionally only because it overrides a reachable declaration. + // + // The collection must be a subset of [result] set. + val contagiousReachableDeclarations = hashSetOf>() val constructedClasses = hashSetOf() fun IrDeclaration.enqueue( from: IrDeclaration?, description: String?, + isContagious: Boolean = true, altFromFqn: String? = null ) { - if (this is IrProperty && !this.isExternal || - this in result - ) { - return - } + // Ignore non-external IrProperty because we don't want to generate code for them and codegen doesn't support it. + if (this is IrProperty && !this.isExternal) return + + // TODO check that this is overridable + // it requires fixing how functions with default arguments is handled + val isContagiousOverridableDeclaration = isContagious && this is IrOverridableDeclaration<*> && this.isMemberOfOpenClass if (printReachabilityInfo) { val fromFqn = (from as? IrDeclarationWithName)?.fqNameWhenAvailable?.asString() ?: altFromFqn ?: "" - val toFqn = (this as? IrDeclarationWithName)?.fqNameWhenAvailable?.asString() ?: "" - val v = "\"$fromFqn\" -> \"$toFqn\"" + (if (description.isNullOrBlank()) "" else " // $description") + val comment = (description ?: "") + (if (isContagiousOverridableDeclaration) "[CONTAGIOUS!]" else "") + val v = "\"$fromFqn\" -> \"$toFqn\"" + (if (comment.isBlank()) "" else " // $comment") reachabilityInfo.add(v) } - result.add(this) - queue.addLast(this) + if (isContagiousOverridableDeclaration) { + contagiousReachableDeclarations.add(this as IrOverridableDeclaration<*>) + } + + if (this !in result) { + result.add(this) + queue.addLast(this) + } } // use withInitialIr to avoid ConcurrentModificationException in dce-driven lowering when adding roots' nested declarations (members) @@ -175,8 +194,8 @@ fun usefulDeclarations(roots: Iterable, context: JsIrBackendConte // TODO remove? stageController.lazyLower(declaration) - fun IrDeclaration.enqueue(description: String) { - enqueue(declaration, description) + fun IrDeclaration.enqueue(description: String, isContagious: Boolean = true) { + enqueue(declaration, description, isContagious) } if (declaration is IrClass) { @@ -186,7 +205,7 @@ fun usefulDeclarations(roots: Iterable, context: JsIrBackendConte // TODO find out how `doResume` gets removed if (declaration.symbol == context.ir.symbols.coroutineImpl) { - declaration.declarations.forEach { + declaration.declarations.toList().forEach { if (it is IrSimpleFunction && it.name.asString() == "doResume") { it.enqueue("hack for CoroutineImpl::doResume") } @@ -195,7 +214,7 @@ fun usefulDeclarations(roots: Iterable, context: JsIrBackendConte } if (declaration is IrSimpleFunction && declaration.isFakeOverride) { - declaration.resolveFakeOverride()?.enqueue("real overridden fun") + declaration.resolveFakeOverride()?.enqueue("real overridden fun", isContagious = false) } // Collect instantiated classes. @@ -281,13 +300,13 @@ fun usefulDeclarations(roots: Iterable, context: JsIrBackendConte }) } - fun IrOverridableDeclaration<*>.findOverriddenUsefulDeclaration(): IrOverridableDeclaration<*>? { + fun IrOverridableDeclaration<*>.findOverriddenContagiousDeclaration(): IrOverridableDeclaration<*>? { for (overriddenSymbol in this.overriddenSymbols) { val overriddenDeclaration = overriddenSymbol.owner as? IrOverridableDeclaration<*> ?: continue - if (overriddenDeclaration in result) return overriddenDeclaration + if (overriddenDeclaration in contagiousReachableDeclarations) return overriddenDeclaration - overriddenDeclaration.findOverriddenUsefulDeclaration()?.let { + overriddenDeclaration.findOverriddenContagiousDeclaration()?.let { return it } } @@ -301,7 +320,7 @@ fun usefulDeclarations(roots: Iterable, context: JsIrBackendConte if (declaration in result) continue if (declaration is IrOverridableDeclaration<*>) { - declaration.findOverriddenUsefulDeclaration()?.let { + declaration.findOverriddenContagiousDeclaration()?.let { declaration.enqueue(it, "overrides useful declaration") } } @@ -324,7 +343,7 @@ fun usefulDeclarations(roots: Iterable, context: JsIrBackendConte // TODO this is a hack. if (declaration is IrProperty) { fun IrSimpleFunction.enqueue(description: String) { - findOverriddenUsefulDeclaration()?.let { enqueue(it, description) } + findOverriddenContagiousDeclaration()?.let { enqueue(it, description) } } declaration.getter?.enqueue("(getter) overrides useful declaration") @@ -349,4 +368,4 @@ fun usefulDeclarations(roots: Iterable, context: JsIrBackendConte } return result -} \ No newline at end of file +}