From 220403b6f634558b24c73ef76368908e887b24a5 Mon Sep 17 00:00:00 2001 From: Dmitry Jemerov Date: Wed, 3 Jun 2015 14:48:06 +0200 Subject: [PATCH] avoid unnecessary creation of empty containers --- .../kotlin/resolve/lazy/LazyFileScope.kt | 6 +-- .../kotlin/resolve/lazy/LazyImportScope.kt | 8 +-- .../resolve/scopes/WritableScopeImpl.kt | 36 ++++--------- .../AbstractJvmRuntimeDescriptorLoaderTest.kt | 5 +- .../kotlin/types/JetTypeCheckerTest.java | 2 +- .../kotlin/resolve/scopes/ChainedScope.kt | 15 +++--- .../jetbrains/kotlin/util/collectionUtils.kt | 54 +++++++++++++++++++ 7 files changed, 81 insertions(+), 45 deletions(-) create mode 100644 core/descriptors/src/org/jetbrains/kotlin/util/collectionUtils.kt diff --git a/compiler/frontend/src/org/jetbrains/kotlin/resolve/lazy/LazyFileScope.kt b/compiler/frontend/src/org/jetbrains/kotlin/resolve/lazy/LazyFileScope.kt index a296cdf7075..471036e54b0 100644 --- a/compiler/frontend/src/org/jetbrains/kotlin/resolve/lazy/LazyFileScope.kt +++ b/compiler/frontend/src/org/jetbrains/kotlin/resolve/lazy/LazyFileScope.kt @@ -33,13 +33,13 @@ import org.jetbrains.kotlin.descriptors.Visibilities import org.jetbrains.kotlin.resolve.scopes.receivers.ReceiverValue import java.util.ArrayList -class LazyFileScope private( - private val scopeChain: List, +class LazyFileScope private constructor( + scopeChain: List, private val aliasImportResolver: LazyImportResolver, private val allUnderImportResolver: LazyImportResolver, containingDeclaration: PackageFragmentDescriptor, debugName: String -) : ChainedScope(containingDeclaration, debugName, *scopeChain.copyToArray()) { +) : ChainedScope(containingDeclaration, debugName, *scopeChain.toTypedArray()) { public fun forceResolveAllImports() { aliasImportResolver.forceResolveAllContents() diff --git a/compiler/frontend/src/org/jetbrains/kotlin/resolve/lazy/LazyImportScope.kt b/compiler/frontend/src/org/jetbrains/kotlin/resolve/lazy/LazyImportScope.kt index 6d82659dccd..915e4ce9b64 100644 --- a/compiler/frontend/src/org/jetbrains/kotlin/resolve/lazy/LazyImportScope.kt +++ b/compiler/frontend/src/org/jetbrains/kotlin/resolve/lazy/LazyImportScope.kt @@ -31,6 +31,7 @@ import org.jetbrains.kotlin.resolve.scopes.DescriptorKindFilter import org.jetbrains.kotlin.resolve.scopes.JetScope import org.jetbrains.kotlin.resolve.scopes.receivers.ReceiverValue import org.jetbrains.kotlin.utils.Printer +import org.jetbrains.kotlin.util.collectionUtils.concat import java.util.HashSet import java.util.LinkedHashSet import kotlin.properties.Delegates @@ -192,17 +193,18 @@ class LazyImportResolver( descriptorsSelector: (JetScope, Name) -> Collection ): Collection { return resolveSession.getStorageManager().compute { - val descriptors = HashSet() + var descriptors: Collection? = null for (directive in indexedImports.importsForName(name)) { if (directive == directiveUnderResolve) { // This is the recursion in imports analysis throw IllegalStateException("Recursion while resolving many imports: " + directive.getText()) } - descriptors.addAll(descriptorsSelector(getImportScope(directive, lookupMode), name)) + val descriptorsForImport = descriptorsSelector(getImportScope(directive, lookupMode), name) + descriptors = descriptors.concat(descriptorsForImport) } - descriptors + descriptors ?: emptySet() } } diff --git a/compiler/frontend/src/org/jetbrains/kotlin/resolve/scopes/WritableScopeImpl.kt b/compiler/frontend/src/org/jetbrains/kotlin/resolve/scopes/WritableScopeImpl.kt index 2e11f8b9192..1a7ace5ec4c 100644 --- a/compiler/frontend/src/org/jetbrains/kotlin/resolve/scopes/WritableScopeImpl.kt +++ b/compiler/frontend/src/org/jetbrains/kotlin/resolve/scopes/WritableScopeImpl.kt @@ -24,6 +24,7 @@ import org.jetbrains.kotlin.utils.Printer import java.util.* import com.intellij.util.SmartList +import org.jetbrains.kotlin.util.collectionUtils.concatInOrder // Reads from: // 1. Maps @@ -110,12 +111,8 @@ public class WritableScopeImpl(override val workerScope: JetScope, val labelsToDescriptors = getLabelsToDescriptors() val name = descriptor.getName() - var declarationDescriptors = labelsToDescriptors[name] - if (declarationDescriptors == null) { - declarationDescriptors = ArrayList() - labelsToDescriptors.put(name, declarationDescriptors!!) - } - declarationDescriptors!!.add(descriptor) + var declarationDescriptors = labelsToDescriptors.getOrPut(name) { ArrayList() } + declarationDescriptors.add(descriptor) } private fun getVariableOrClassDescriptors(): MutableMap { @@ -125,13 +122,6 @@ public class WritableScopeImpl(override val workerScope: JetScope, return variableOrClassDescriptors!! } - private fun getPackageAliases(): MutableMap { - if (packageAliases == null) { - packageAliases = HashMap() - } - return packageAliases!! - } - override fun addVariableDescriptor(variableDescriptor: VariableDescriptor) { addVariableDescriptor(variableDescriptor, false) } @@ -157,17 +147,14 @@ public class WritableScopeImpl(override val workerScope: JetScope, checkMayRead() val propertyGroupsByName = propertyGroups?.get(name) ?: return workerScope.getProperties(name) - - val result = Sets.newLinkedHashSet(propertyGroupsByName) - result.addAll(workerScope.getProperties(name)) - return result + return concatInOrder(propertyGroupsByName, workerScope.getProperties(name)) } override fun getLocalVariable(name: Name): VariableDescriptor? { checkMayRead() - val descriptor = getVariableOrClassDescriptors()[name] - if (descriptor is VariableDescriptor && !getPropertyGroups()[name].contains(descriptor)) { + val descriptor = variableOrClassDescriptors?.get(name) + if (descriptor is VariableDescriptor && propertyGroups?.get(name)?.contains(descriptor) != true) { return descriptor } @@ -198,11 +185,8 @@ public class WritableScopeImpl(override val workerScope: JetScope, override fun getFunctions(name: Name): Collection { checkMayRead() - val functionGroupByName = functionGroups?.get(name) ?: return workerScope.getFunctions(name) - - val result = Sets.newLinkedHashSet(functionGroupByName) - result.addAll(workerScope.getFunctions(name)) - return result + val functionGroupByName = functionGroups?.get(name) + return concatInOrder(functionGroupByName, workerScope.getFunctions(name)) } override fun addClassifierDescriptor(classifierDescriptor: ClassifierDescriptor) { @@ -238,14 +222,14 @@ public class WritableScopeImpl(override val workerScope: JetScope, override fun getClassifier(name: Name): ClassifierDescriptor? { checkMayRead() - return getVariableOrClassDescriptors()[name] as? ClassifierDescriptor + return variableOrClassDescriptors?.get(name) as? ClassifierDescriptor ?: workerScope.getClassifier(name) } override fun getPackage(name: Name): PackageViewDescriptor? { checkMayRead() - return getPackageAliases().get(name) + return packageAliases?.get(name) ?: workerScope.getPackage(name) } diff --git a/compiler/tests/org/jetbrains/kotlin/jvm/runtime/AbstractJvmRuntimeDescriptorLoaderTest.kt b/compiler/tests/org/jetbrains/kotlin/jvm/runtime/AbstractJvmRuntimeDescriptorLoaderTest.kt index e5207d3b0fd..a43b9380497 100644 --- a/compiler/tests/org/jetbrains/kotlin/jvm/runtime/AbstractJvmRuntimeDescriptorLoaderTest.kt +++ b/compiler/tests/org/jetbrains/kotlin/jvm/runtime/AbstractJvmRuntimeDescriptorLoaderTest.kt @@ -161,8 +161,7 @@ public abstract class AbstractJvmRuntimeDescriptorLoaderTest : TestCaseWithTmpdi // Since runtime package view descriptor doesn't support getAllDescriptors(), we construct a synthetic package view here. // It has in its scope descriptors for all the classes and top level members generated by the compiler - val actual = SyntheticPackageViewForTest(module, packageScopes, classes) - return actual + return SyntheticPackageViewForTest(module, packageScopes, classes) } private fun addRuntimeRetentionToKotlinSource(text: String): String { @@ -188,7 +187,7 @@ public abstract class AbstractJvmRuntimeDescriptorLoaderTest : TestCaseWithTmpdi val writableScope = WritableScopeImpl(JetScope.Empty, this, RedeclarationHandler.THROW_EXCEPTION, "runtime descriptor loader test") classes.forEach { writableScope.addClassifierDescriptor(it) } writableScope.changeLockLevel(WritableScope.LockLevel.READING) - scope = ChainedScope(null, "", *(listOf(writableScope) + packageScopes).toTypedArray()) + scope = ChainedScope(this, "synthetic package view for test", writableScope, *packageScopes.toTypedArray()) } override fun getFqName() = LoadDescriptorUtil.TEST_PACKAGE_FQNAME diff --git a/compiler/tests/org/jetbrains/kotlin/types/JetTypeCheckerTest.java b/compiler/tests/org/jetbrains/kotlin/types/JetTypeCheckerTest.java index 14784611076..17892b67441 100644 --- a/compiler/tests/org/jetbrains/kotlin/types/JetTypeCheckerTest.java +++ b/compiler/tests/org/jetbrains/kotlin/types/JetTypeCheckerTest.java @@ -625,7 +625,7 @@ public class JetTypeCheckerTest extends JetLiteFixture { } scopeChain.add(module.getPackage(FqName.ROOT).getMemberScope()); writableScope.changeLockLevel(WritableScope.LockLevel.BOTH); - return new ChainedScope(scope.getContainingDeclaration(), "", scopeChain.toArray(new JetScope[scopeChain.size()])); + return new ChainedScope(scope.getContainingDeclaration(), "JetTypeCheckerTest.addImports scope with imports", scopeChain.toArray(new JetScope[scopeChain.size()])); } private JetType makeType(String typeStr) { diff --git a/core/descriptors/src/org/jetbrains/kotlin/resolve/scopes/ChainedScope.kt b/core/descriptors/src/org/jetbrains/kotlin/resolve/scopes/ChainedScope.kt index 729e165e52f..62b9938c8ca 100644 --- a/core/descriptors/src/org/jetbrains/kotlin/resolve/scopes/ChainedScope.kt +++ b/core/descriptors/src/org/jetbrains/kotlin/resolve/scopes/ChainedScope.kt @@ -19,6 +19,7 @@ package org.jetbrains.kotlin.resolve.scopes import org.jetbrains.kotlin.descriptors.* import org.jetbrains.kotlin.name.Name import org.jetbrains.kotlin.utils.Printer +import org.jetbrains.kotlin.util.collectionUtils.concat import java.util.* public open class ChainedScope( @@ -38,15 +39,11 @@ public open class ChainedScope( return null } - private inline fun getFromAllScopes(callback: (JetScope) -> Collection): Set { + private inline fun getFromAllScopes(callback: (JetScope) -> Collection): Collection { if (scopeChain.isEmpty()) return emptySet() - var result: MutableSet? = null + var result: Collection? = null for (scope in scopeChain) { - val fromScope = callback(scope) - if (result == null) { - result = LinkedHashSet() - } - result.addAll(fromScope) + result = result.concat(callback(scope)) } return result ?: emptySet() } @@ -57,13 +54,13 @@ public open class ChainedScope( override fun getPackage(name: Name): PackageViewDescriptor? = getFirstMatch { it.getPackage(name) } - override fun getProperties(name: Name): Set + override fun getProperties(name: Name): Collection = getFromAllScopes { it.getProperties(name) } override fun getLocalVariable(name: Name): VariableDescriptor? = getFirstMatch { it.getLocalVariable(name) } - override fun getFunctions(name: Name): Set + override fun getFunctions(name: Name): Collection = getFromAllScopes { it.getFunctions(name) } override fun getImplicitReceiversHierarchy(): List { diff --git a/core/descriptors/src/org/jetbrains/kotlin/util/collectionUtils.kt b/core/descriptors/src/org/jetbrains/kotlin/util/collectionUtils.kt new file mode 100644 index 00000000000..41032c460f5 --- /dev/null +++ b/core/descriptors/src/org/jetbrains/kotlin/util/collectionUtils.kt @@ -0,0 +1,54 @@ +/* + * Copyright 2010-2015 JetBrains s.r.o. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.jetbrains.kotlin.util.collectionUtils + +import java.util.* + +/** + * Concatenates the contents of this collection with the given collection, avoiding allocations if possible. + * Can modify `this` if it is a mutable collection. + */ +fun Collection?.concat(collection: Collection): Collection? { + if (collection.isEmpty()) { + return this + } + if (this == null) { + return collection + } + if (this is LinkedHashSet<*>) { + addAll(collection) + return this + } + + val result = LinkedHashSet(this) + result.addAll(collection) + return result +} + +fun concatInOrder(c1: Collection?, c2: Collection?): Collection { + val result = if (c1 == null || c1.isEmpty()) + c2 + else if (c2 == null || c2.isEmpty()) + c1 + else { + val result = LinkedHashSet() + result.addAll(c1) + result.addAll(c2) + result + } + return result ?: emptySet() +}