From f2028650807970171883a48773fd7348737a500c Mon Sep 17 00:00:00 2001 From: Alexander Korepanov Date: Mon, 19 Jun 2023 16:39:40 +0200 Subject: [PATCH] [JS IR] Improve InternalStringMap iterators & Extend InternalMap interface ^KT-59001 --- .../js/src/kotlin/collections/HashMap.kt | 4 +- .../src/kotlin/collections/InternalHashMap.kt | 62 ++--- .../js/src/kotlin/collections/InternalMap.kt | 37 ++- .../kotlin/collections/InternalStringMap.kt | 224 +++++++++++++----- 4 files changed, 222 insertions(+), 105 deletions(-) diff --git a/libraries/stdlib/js/src/kotlin/collections/HashMap.kt b/libraries/stdlib/js/src/kotlin/collections/HashMap.kt index 90ef925072d..1c1c1311ed8 100644 --- a/libraries/stdlib/js/src/kotlin/collections/HashMap.kt +++ b/libraries/stdlib/js/src/kotlin/collections/HashMap.kt @@ -30,7 +30,7 @@ public actual open class HashMap : AbstractMutableMap, MutableMap): Boolean = this@HashMap.containsEntry(element) - override operator fun iterator(): MutableIterator> = internalMap.iterator() + override operator fun iterator(): MutableIterator> = internalMap.entriesIterator() override fun removeEntry(element: Map.Entry): Boolean { if (contains(element)) { @@ -107,7 +107,7 @@ public actual open class HashMap : AbstractMutableMap, MutableMap>? = null actual override val entries: MutableSet> diff --git a/libraries/stdlib/js/src/kotlin/collections/InternalHashMap.kt b/libraries/stdlib/js/src/kotlin/collections/InternalHashMap.kt index 9b068e37d58..2555531e30e 100644 --- a/libraries/stdlib/js/src/kotlin/collections/InternalHashMap.kt +++ b/libraries/stdlib/js/src/kotlin/collections/InternalHashMap.kt @@ -87,7 +87,7 @@ internal class InternalHashMap private constructor( fun isEmpty(): Boolean = _size == 0 fun containsKey(key: K): Boolean = findKey(key) >= 0 - fun containsValue(value: V): Boolean = findValue(value) >= 0 + override fun containsValue(value: V): Boolean = findValue(value) >= 0 override operator fun get(key: K): V? { val index = findKey(key) @@ -113,7 +113,7 @@ internal class InternalHashMap private constructor( } } - fun putAll(from: Map) { + override fun putAll(from: Map) { checkIsMutable() putAllEntries(from.entries) } @@ -143,10 +143,6 @@ internal class InternalHashMap private constructor( length = 0 } - override fun iterator(): MutableIterator> { - return entriesIterator() - } - val keys: MutableSet get() { val cur = keysView @@ -211,7 +207,7 @@ internal class InternalHashMap private constructor( private val capacity: Int get() = keysArray.size private val hashSize: Int get() = hashArray.size - internal fun checkIsMutable() { + override fun checkIsMutable() { if (isReadOnly) throw UnsupportedOperationException() } @@ -425,13 +421,18 @@ internal class InternalHashMap private constructor( } } - internal fun containsEntry(entry: Map.Entry): Boolean { + override fun containsEntry(entry: Map.Entry): Boolean { val index = findKey(entry.key) if (index < 0) return false return valuesArray!![index] == entry.value } - internal fun getEntry(entry: Map.Entry): MutableMap.MutableEntry? { + override fun containsOtherEntry(entry: Map.Entry<*, *>): Boolean { + @Suppress("UNCHECKED_CAST") + return containsEntry(entry as Map.Entry) + } + + override fun getEntry(entry: Map.Entry): MutableMap.MutableEntry? { val index = findKey(entry.key) return if (index < 0 || valuesArray!![index] != entry.value) { null @@ -451,21 +452,6 @@ internal class InternalHashMap private constructor( private fun contentEquals(other: Map<*, *>): Boolean = _size == other.size && containsAllEntries(other.entries) - internal fun containsAllEntries(m: Collection<*>): Boolean { - val it = m.iterator() - while (it.hasNext()) { - val entry = it.next() - try { - @Suppress("UNCHECKED_CAST") // todo: get rid of unchecked cast here somehow - if (entry == null || !containsEntry(entry as Map.Entry)) - return false - } catch (e: ClassCastException) { - return false - } - } - return true - } - private fun putEntry(entry: Map.Entry): Boolean { val index = addKey(entry.key) val valuesArray = allocateValuesArray() @@ -493,7 +479,7 @@ internal class InternalHashMap private constructor( return updated } - internal fun removeEntry(entry: Map.Entry): Boolean { + override fun removeEntry(entry: Map.Entry): Boolean { checkIsMutable() val index = findKey(entry.key) if (index < 0) return false @@ -502,7 +488,7 @@ internal class InternalHashMap private constructor( return true } - internal fun removeValue(element: V): Boolean { + override fun removeValue(element: V): Boolean { checkIsMutable() val index = findValue(element) if (index < 0) return false @@ -510,9 +496,9 @@ internal class InternalHashMap private constructor( return true } - internal fun keysIterator() = KeysItr(this) - internal fun valuesIterator() = ValuesItr(this) - internal fun entriesIterator() = EntriesItr(this) + override fun keysIterator() = KeysItr(this) + override fun valuesIterator() = ValuesItr(this) + override fun entriesIterator() = EntriesItr(this) private companion object { private const val MAGIC = -1640531527 // 2654435769L.toInt(), golden ratio @@ -638,16 +624,16 @@ internal class InternalHashMap private constructor( } internal class HashMapKeys internal constructor( - private val backing: InternalHashMap, + private val backing: InternalMap, ) : MutableSet, AbstractMutableSet() { override val size: Int get() = backing.size - override fun isEmpty(): Boolean = backing.isEmpty() - override fun contains(element: E): Boolean = backing.containsKey(element) + override fun isEmpty(): Boolean = backing.size == 0 + override fun contains(element: E): Boolean = backing.contains(element) override fun clear() = backing.clear() override fun add(element: E): Boolean = throw UnsupportedOperationException() override fun addAll(elements: Collection): Boolean = throw UnsupportedOperationException() - override fun remove(element: E): Boolean = backing.removeKey(element) >= 0 + override fun remove(element: E): Boolean = backing.remove(element) != null override fun iterator(): MutableIterator = backing.keysIterator() override fun removeAll(elements: Collection): Boolean { @@ -662,11 +648,11 @@ internal class HashMapKeys internal constructor( } internal class HashMapValues internal constructor( - val backing: InternalHashMap<*, V>, + private val backing: InternalMap<*, V>, ) : MutableCollection, AbstractMutableCollection() { override val size: Int get() = backing.size - override fun isEmpty(): Boolean = backing.isEmpty() + override fun isEmpty(): Boolean = backing.size == 0 override fun contains(element: V): Boolean = backing.containsValue(element) override fun add(element: V): Boolean = throw UnsupportedOperationException() override fun addAll(elements: Collection): Boolean = throw UnsupportedOperationException() @@ -700,11 +686,11 @@ internal class HashMapValues internal constructor( * See also [KT-42248](https://youtrack.jetbrains.com/issue/KT-42428). */ internal abstract class HashMapEntrySetBase> internal constructor( - val backing: InternalHashMap, + val backing: InternalMap, ) : MutableSet, AbstractMutableSet() { override val size: Int get() = backing.size - override fun isEmpty(): Boolean = backing.isEmpty() + override fun isEmpty(): Boolean = backing.size == 0 override fun contains(element: E): Boolean = backing.containsEntry(element) protected abstract fun getEntry(element: Map.Entry): E? override fun clear() = backing.clear() @@ -725,7 +711,7 @@ internal abstract class HashMapEntrySetBase> internal } internal class HashMapEntrySet internal constructor( - backing: InternalHashMap, + backing: InternalMap, ) : HashMapEntrySetBase>(backing) { override fun getEntry(element: Map.Entry): MutableMap.MutableEntry? = backing.getEntry(element) diff --git a/libraries/stdlib/js/src/kotlin/collections/InternalMap.kt b/libraries/stdlib/js/src/kotlin/collections/InternalMap.kt index 24ac3840222..e60fee65d06 100644 --- a/libraries/stdlib/js/src/kotlin/collections/InternalMap.kt +++ b/libraries/stdlib/js/src/kotlin/collections/InternalMap.kt @@ -6,22 +6,35 @@ package kotlin.collections /** - * The common interface of [InternalStringMap] and [InternalHashCodeMap]. + * The common interface of [InternalStringMap] and [InternalHashMap]. */ -internal interface InternalMap : MutableIterable> { +internal interface InternalMap { val size: Int - operator fun contains(key: K): Boolean - operator fun get(key: K): V? fun put(key: K, value: V): V? - fun remove(key: K): V? - fun clear(): Unit + fun putAll(from: Map) - fun createJsMap(): dynamic { - val result = js("Object.create(null)") - // force to switch object representation to dictionary mode - result["foo"] = 1 - jsDeleteProperty(result, "foo") - return result + operator fun get(key: K): V? + fun getEntry(entry: Map.Entry): MutableMap.MutableEntry? + + operator fun contains(key: K): Boolean + fun containsValue(value: V): Boolean + fun containsEntry(entry: Map.Entry): Boolean + fun containsOtherEntry(entry: Map.Entry<*, *>): Boolean + + fun remove(key: K): V? + fun removeValue(value: V): Boolean + fun removeEntry(entry: Map.Entry): Boolean + + fun clear() + + fun keysIterator(): MutableIterator + fun valuesIterator(): MutableIterator + fun entriesIterator(): MutableIterator> + + fun checkIsMutable() {} + + fun containsAllEntries(m: Collection>): Boolean { + return m.all(this::containsOtherEntry) } } diff --git a/libraries/stdlib/js/src/kotlin/collections/InternalStringMap.kt b/libraries/stdlib/js/src/kotlin/collections/InternalStringMap.kt index 32a4bd9ecca..78cd153496c 100644 --- a/libraries/stdlib/js/src/kotlin/collections/InternalStringMap.kt +++ b/libraries/stdlib/js/src/kotlin/collections/InternalStringMap.kt @@ -10,6 +10,26 @@ package kotlin.collections import kotlin.collections.MutableMap.MutableEntry +internal external interface JsRawArray { + fun push(element: E) + fun pop(): E + + var length: Int +} + +private inline fun JsRawArray.getElement(index: Int): E { + return (this.asDynamic()[index]).unsafeCast() +} + +private inline fun JsRawArray.setElement(index: Int, element: E) { + this.asDynamic()[index] = element +} + +private inline fun JsRawArray.replaceElementAtWithLast(index: Int) { + setElement(index, getElement(length - 1)) + pop() +} + /** * A simple wrapper around JavaScript Map for key type is string. * @@ -17,19 +37,25 @@ import kotlin.collections.MutableMap.MutableEntry * because we want to have it erased to Any? in order not to generate type-safe override bridges for * [get], [contains], [remove] etc, if they ever are generated. */ -internal class InternalStringMap : InternalMap { +internal open class InternalStringMap : InternalMap { + private fun createJsMap(): dynamic { + val result = js("Object.create(null)") + // force to switch object representation to dictionary mode + result["foo"] = 1 + jsDeleteProperty(result.unsafeCast(), "foo") + return result + } + + private fun createJsArray(): JsRawArray { + return js("[]").unsafeCast>() + } private var backingMap: dynamic = createJsMap() - override var size: Int = 0 - private set + private var values = createJsArray() + private var keys = createJsArray() -// /** -// * A mod count to track 'value' replacements in map to ensure that the 'value' that we have in the -// * iterator entry is guaranteed to be still correct. -// * This is to optimize for the common scenario where the values are not modified during -// * iterations where the entries are never stale. -// */ -// private var valueMod: Int = 0 + override val size: Int + get() = keys.length override operator fun contains(key: K): Boolean { if (key !is String) return false @@ -38,77 +64,169 @@ internal class InternalStringMap : InternalMap { override operator fun get(key: K): V? { if (key !is String) return null - val value = backingMap[key] - return if (value !== undefined) value.unsafeCast() else null + val index = backingMap[key] + return if (index !== undefined) values.getElement(index.unsafeCast()) else null } + override fun getEntry(entry: Map.Entry): MutableEntry? { + val key = entry.key as? String ?: return null + val index = backingMap[key] + if (index === undefined) { + return null + } + val value = values.getElement(index.unsafeCast()) + if (value == entry.value) { + return EntryRef(entry.key, value, this) + } + return null + } + + override fun containsValue(value: V): Boolean { + return values.unsafeCast>().contains(value) + } + + override fun containsEntry(entry: Map.Entry): Boolean { + return getEntry(entry) != null + } + + override fun containsOtherEntry(entry: Map.Entry<*, *>): Boolean { + @Suppress("UNCHECKED_CAST") + return containsEntry(entry as Map.Entry) + } + + override fun removeEntry(entry: Map.Entry): Boolean { + val key = getEntry(entry)?.key ?: return false + return remove(key) != null + } + + override fun removeValue(value: V): Boolean { + val index = values.unsafeCast>().indexOf(value) + if (index < 0) { + return false + } + removeKeyIndex(keys.getElement(index), index) + return true + } override fun put(key: K, value: V): V? { require(key is String) - val oldValue = backingMap[key] - backingMap[key] = value + val index = backingMap[key] + if (index !== undefined) { + val i = index.unsafeCast() + val oldValue = values.getElement(i) + values.setElement(i, value) + return oldValue + } - if (oldValue === undefined) { - size++ -// structureChanged(host) - return null - } else { -// valueMod++ - return oldValue.unsafeCast() + backingMap[key] = size + keys.push(key) + values.push(value) + return null + } + + override fun putAll(from: Map) { + for ((key, value) in from) { + put(key, value) } } override fun remove(key: K): V? { if (key !is String) return null - val value = backingMap[key] - if (value !== undefined) { - jsDeleteProperty(backingMap, key) - size-- -// structureChanged(host) - return value.unsafeCast() - } else { -// valueMod++ + val index = backingMap[key] + if (index === undefined) { return null } + + val i = index.unsafeCast() + val removedValue = values.getElement(i) + + removeKeyIndex(key, i) + return removedValue } + internal fun removeKeyIndex(key: K, index: Int) { + jsDeleteProperty(backingMap.unsafeCast(), key as Any) + + if (index + 1 == size) { + keys.pop() + values.pop() + } else { + keys.replaceElementAtWithLast(index) + values.replaceElementAtWithLast(index) + backingMap[keys.getElement(index)] = index + } + } override fun clear() { backingMap = createJsMap() - size = 0 + keys = createJsArray() + values = createJsArray() } + override fun keysIterator(): MutableIterator = KeysItr(this) + override fun valuesIterator(): MutableIterator = ValuesItr(this) + override fun entriesIterator(): MutableIterator> = EntriesItr(this) - override fun iterator(): MutableIterator> { - return object : MutableIterator> { - private val keys: Array = js("Object").keys(backingMap) - private val iterator = keys.iterator() - private var lastKey: String? = null + private abstract class BaseItr(protected val map: InternalStringMap) { + protected var lastIndex = -1 + protected var index = 0 - override fun hasNext(): Boolean = iterator.hasNext() - - override fun next(): MutableEntry { - val key = iterator.next() - lastKey = key - @Suppress("UNCHECKED_CAST") - return newMapEntry(key as K) + protected fun goNext() { + if (index >= map.size) { + throw NoSuchElementException() } + lastIndex = index++ + } - override fun remove() { - @Suppress("UNCHECKED_CAST") - this@InternalStringMap.remove(checkNotNull(lastKey) as K) - } + fun hasNext(): Boolean = index < map.size + + fun remove() { + map.removeKeyIndex(map.keys.getElement(lastIndex), lastIndex) + lastIndex = -1 } } - private fun newMapEntry(key: K): MutableEntry = object : MutableEntry { - override val key: K get() = key - override val value: V get() = this@InternalStringMap[key].unsafeCast() + private abstract class Itr( + private val iterableArray: JsRawArray, + map: InternalStringMap, + ) : MutableIterator, BaseItr(map) { + override fun next(): I { + goNext() + return iterableArray.getElement(lastIndex) + } + } - override fun setValue(newValue: V): V = this@InternalStringMap.put(key, newValue).unsafeCast() + private class KeysItr(map: InternalStringMap) : Itr(map.keys, map) + private class ValuesItr(map: InternalStringMap) : Itr(map.values, map) - override fun hashCode(): Int = AbstractMap.entryHashCode(this) - override fun toString(): String = AbstractMap.entryToString(this) - override fun equals(other: Any?): Boolean = AbstractMap.entryEquals(this, other) + private class EntriesItr(map: InternalStringMap) : MutableIterator>, BaseItr(map) { + override fun next(): MutableEntry { + goNext() + val key = map.keys.getElement(lastIndex) + val value = map.values.getElement(lastIndex) + return EntryRef(key, value, map) + } + } + + private class EntryRef( + override val key: K, + override var value: V, + private val map: InternalStringMap, + ) : MutableEntry { + override fun setValue(newValue: V): V { + val prevValue = value + map.put(key, newValue) + value = newValue + return prevValue + } + + override fun equals(other: Any?): Boolean = + other is Map.Entry<*, *> && + other.key == key && + other.value == value + + override fun hashCode(): Int = key.hashCode() xor value.hashCode() + + override fun toString(): String = "$key=$value" } }