From 23d2654afd36c9927338378802a4fb8836a7dcff Mon Sep 17 00:00:00 2001 From: Ilya Gorbunov Date: Fri, 26 Aug 2016 22:25:56 +0300 Subject: [PATCH] Refactor Map-InternalMap to provide specialized linked string map and set --- .../src/core/collections/HashMap.kt | 16 +++++----- .../core/collections/InternalHashCodeMap.kt | 12 +++---- .../src/core/collections/InternalMap.kt | 1 + .../src/core/collections/InternalStringMap.kt | 2 +- .../src/core/collections/LinkedHashMap.kt | 31 ++++++++++--------- .../src/core/collections/LinkedHashSet.kt | 8 ++++- libraries/stdlib/test/js/MapJsTest.kt | 15 ++++++++- libraries/stdlib/test/js/SetJsTest.kt | 12 +++++++ libraries/stdlib/test/js/StubsJVM.kt | 2 ++ 9 files changed, 67 insertions(+), 32 deletions(-) diff --git a/js/js.libraries/src/core/collections/HashMap.kt b/js/js.libraries/src/core/collections/HashMap.kt index fb06e6a530b..59f3c3f6ff7 100644 --- a/js/js.libraries/src/core/collections/HashMap.kt +++ b/js/js.libraries/src/core/collections/HashMap.kt @@ -100,26 +100,26 @@ open class HashMap : AbstractMap { */ private val internalMap: InternalMap - internal val equality: EqualityComparator + private val equality: EqualityComparator // /** // * A map of Strings onto values. // */ // @Transient private var stringMap: InternalStringMap? = null // - internal constructor(internalMapProvider: (HashMap) -> InternalMap, equality: EqualityComparator = EqualityComparator.HashCode) : super() { - this.equality = equality - this.internalMap = internalMapProvider(this) + internal constructor(internalMap: InternalMap) : super() { + this.internalMap = internalMap + this.equality = internalMap.equality } - constructor() : this( { m -> InternalHashCodeMap(m) }) + constructor() : this(InternalHashCodeMap(EqualityComparator.HashCode)) // init { // reset() // } - constructor(capacity: Int, loadFactor: Float = 0f) : this() { + constructor(initialCapacity: Int, loadFactor: Float = 0f) : this() { // This implementation of HashMap has no need of load factors or capacities. - require(capacity >= 0) { "Negative initial capacity" } + require(initialCapacity >= 0) { "Negative initial capacity" } require(loadFactor >= 0) { "Non-positive load factor" } } @@ -273,5 +273,5 @@ open class HashMap : AbstractMap { public fun stringMapOf(vararg pairs: Pair): HashMap { - return HashMap({ m -> InternalStringMap(m) }).apply { putAll(pairs) } + return HashMap(InternalStringMap(EqualityComparator.HashCode)).apply { putAll(pairs) } } \ No newline at end of file diff --git a/js/js.libraries/src/core/collections/InternalHashCodeMap.kt b/js/js.libraries/src/core/collections/InternalHashCodeMap.kt index 29b2a198972..8b88063c45a 100644 --- a/js/js.libraries/src/core/collections/InternalHashCodeMap.kt +++ b/js/js.libraries/src/core/collections/InternalHashCodeMap.kt @@ -35,14 +35,14 @@ import kotlin.collections.AbstractMap.SimpleEntry * have the same hash, each value in hashCodeMap is actually an array containing all entries whose * keys share the same hash. */ -internal class InternalHashCodeMap(private val host: HashMap) : InternalMap { +internal class InternalHashCodeMap(override val equality: EqualityComparator) : InternalMap { private var backingMap: dynamic = js("Object.create(null)") override var size: Int = 0 private set override fun put(key: K, value: V): V? { - val hashCode = host.equality.getHashCode(key) + val hashCode = equality.getHashCode(key) val chain = getChainOrNull(hashCode) if (chain == null) { // This is a new chain, put it to the map. @@ -62,11 +62,11 @@ internal class InternalHashCodeMap(private val host: HashMap) : Inte } override fun remove(key: K): V? { - val hashCode = host.equality.getHashCode(key) + val hashCode = equality.getHashCode(key) val chain = getChainOrNull(hashCode) ?: return null for (index in 0..chain.size-1) { val entry = chain[index] - if (host.equality.equals(key, entry.key)) { + if (equality.equals(key, entry.key)) { if (chain.size == 1) { chain.asDynamic().length = 0 // remove the whole array @@ -94,10 +94,10 @@ internal class InternalHashCodeMap(private val host: HashMap) : Inte override fun get(key: K): V? = getEntry(key)?.value private fun getEntry(key: K): MutableEntry? = - getChainOrNull(host.equality.getHashCode(key))?.findEntryInChain(key) + getChainOrNull(equality.getHashCode(key))?.findEntryInChain(key) private fun Array>.findEntryInChain(key: K): MutableEntry? = - firstOrNull { entry -> host.equality.equals(entry.key, key) } + firstOrNull { entry -> equality.equals(entry.key, key) } override fun iterator(): MutableIterator> { diff --git a/js/js.libraries/src/core/collections/InternalMap.kt b/js/js.libraries/src/core/collections/InternalMap.kt index 2dfa38c5da3..607053975aa 100644 --- a/js/js.libraries/src/core/collections/InternalMap.kt +++ b/js/js.libraries/src/core/collections/InternalMap.kt @@ -17,6 +17,7 @@ package kotlin.collections internal interface InternalMap : MutableIterable> { + val equality: EqualityComparator val size: Int operator fun contains(key: K): Boolean operator fun get(key: K): V? diff --git a/js/js.libraries/src/core/collections/InternalStringMap.kt b/js/js.libraries/src/core/collections/InternalStringMap.kt index b3422659ec0..f3c87f76fa4 100644 --- a/js/js.libraries/src/core/collections/InternalStringMap.kt +++ b/js/js.libraries/src/core/collections/InternalStringMap.kt @@ -24,7 +24,7 @@ import kotlin.collections.MutableMap.MutableEntry /** * A simple wrapper around JavaScript Map for key type is string. */ -internal class InternalStringMap(private val host: HashMap) : InternalMap { +internal class InternalStringMap(override val equality: EqualityComparator) : InternalMap { private var backingMap: dynamic = js("Object.create(null)") override var size: Int = 0 diff --git a/js/js.libraries/src/core/collections/LinkedHashMap.kt b/js/js.libraries/src/core/collections/LinkedHashMap.kt index 7b541d08c79..abb141f7ceb 100644 --- a/js/js.libraries/src/core/collections/LinkedHashMap.kt +++ b/js/js.libraries/src/core/collections/LinkedHashMap.kt @@ -162,35 +162,31 @@ open class LinkedHashMap : HashMap, Map { * duplicate the key here to eliminate changes to HashMap and minimize the * code here, at the expense of additional space. */ - private val map = HashMap() + private val map: HashMap - constructor() { - resetChainEntries() + constructor() : super() { + map = HashMap() } - constructor(ignored: Int, alsoIgnored: Float = 0f) : super(ignored, alsoIgnored) { - resetChainEntries() + internal constructor(backingMap: HashMap) : super() { + map = backingMap as HashMap } -// constructor(ignored: Int, alsoIgnored: Float, accessOrder: Boolean) : super(ignored, alsoIgnored) { -// this.accessOrder = accessOrder -// resetChainEntries() -// } + constructor(initialCapacity: Int, loadFactor: Float = 0f) : super(initialCapacity, loadFactor) { + map = HashMap() + } constructor(original: Map) { - resetChainEntries() + map = HashMap() this.putAll(original) } override fun clear() { map.clear() - resetChainEntries() - } - - private fun resetChainEntries() { head = null } -// + + // override fun clone(): Any { // return LinkedHashMap(this) // } @@ -265,3 +261,8 @@ open class LinkedHashMap : HashMap, Map { // } } } + + +public fun linkedStringMapOf(vararg pairs: Pair): LinkedHashMap { + return LinkedHashMap(stringMapOf()).apply { putAll(pairs) } +} \ No newline at end of file diff --git a/js/js.libraries/src/core/collections/LinkedHashSet.kt b/js/js.libraries/src/core/collections/LinkedHashSet.kt index 4ef18eb3d86..ee9f3847e0f 100644 --- a/js/js.libraries/src/core/collections/LinkedHashSet.kt +++ b/js/js.libraries/src/core/collections/LinkedHashSet.kt @@ -22,16 +22,22 @@ package kotlin.collections open class LinkedHashSet : HashSet { + internal constructor(map: LinkedHashMap) : super(map) + constructor() : super(LinkedHashMap()) constructor(c: Collection) : super(LinkedHashMap()) { addAll(c) } - constructor(capacity: Int, loadFactor: Float = 0.0f) : super(LinkedHashMap(capacity, loadFactor)) + constructor(initialCapacity: Int, loadFactor: Float = 0.0f) : super(LinkedHashMap(initialCapacity, loadFactor)) // public override fun clone(): Any { // return LinkedHashSet(this) // } } + +public fun linkedStringSetOf(vararg elements: String): LinkedHashSet { + return LinkedHashSet(linkedStringMapOf()).apply { addAll(elements) } +} diff --git a/libraries/stdlib/test/js/MapJsTest.kt b/libraries/stdlib/test/js/MapJsTest.kt index ceaac411287..c7bd5fa6640 100644 --- a/libraries/stdlib/test/js/MapJsTest.kt +++ b/libraries/stdlib/test/js/MapJsTest.kt @@ -55,7 +55,7 @@ class PrimitiveMapJsTest : MapJsTest() { } } -class LinkedHashMapTest : MapJsTest() { +class LinkedHashMapJsTest : MapJsTest() { @test override fun constructors() { LinkedHashMap() LinkedHashMap(3) @@ -72,6 +72,19 @@ class LinkedHashMapTest : MapJsTest() { override fun emptyMutableMapWithNullableKeyValue(): MutableMap = LinkedHashMap() } +class LinkedPrimitiveMapJsTest : MapJsTest() { + @test override fun constructors() { + val map = createTestMap() + + assertEquals(KEYS.toNormalizedList(), map.keys.toNormalizedList()) + assertEquals(VALUES.toNormalizedList(), map.values.toNormalizedList()) + } + + override fun > Collection.toNormalizedList(): List = this.toList() + override fun emptyMutableMap(): MutableMap = linkedStringMapOf() + override fun emptyMutableMapWithNullableKeyValue(): MutableMap = LinkedHashMap() +} + abstract class MapJsTest { val KEYS = listOf("zero", "one", "two", "three") val VALUES = arrayOf(0, 1, 2, 3).toList() diff --git a/libraries/stdlib/test/js/SetJsTest.kt b/libraries/stdlib/test/js/SetJsTest.kt index 8ce3319dbb4..e573d6918f7 100644 --- a/libraries/stdlib/test/js/SetJsTest.kt +++ b/libraries/stdlib/test/js/SetJsTest.kt @@ -72,6 +72,18 @@ class LinkedHashSetJsTest : SetJsTest() { } } +class LinkedPrimitiveSetJsTest : SetJsTest() { + override fun createEmptyMutableSet(): MutableSet = linkedStringSetOf() + override fun createEmptyMutableSetWithNullableValues(): MutableSet = LinkedHashSet() + @Test + override fun constructors() { + val orderedData = data.toList() + val set = linkedStringSetOf(*orderedData.toTypedArray()) + + assertEquals(orderedData, set.toList()) + } +} + abstract class SetJsTest { val data: Set = createTestMutableSet() val empty: Set = createEmptyMutableSet() diff --git a/libraries/stdlib/test/js/StubsJVM.kt b/libraries/stdlib/test/js/StubsJVM.kt index b86bbf8ad71..1322a4077ab 100644 --- a/libraries/stdlib/test/js/StubsJVM.kt +++ b/libraries/stdlib/test/js/StubsJVM.kt @@ -19,4 +19,6 @@ package test.collections.js import java.util.* public fun stringMapOf(vararg pairs: Pair): HashMap = hashMapOf(*pairs) +public fun linkedStringMapOf(vararg pairs: Pair): LinkedHashMap = linkedMapOf(*pairs) public fun stringSetOf(vararg elements: String): HashSet = hashSetOf(*elements) +public fun linkedStringSetOf(vararg elements: String): LinkedHashSet = linkedSetOf(*elements)