From 71381ec8e23f15e74cc3b5d4d66c4c8462455232 Mon Sep 17 00:00:00 2001 From: Abduqodiri Qurbonzoda Date: Fri, 7 Oct 2022 11:19:31 +0000 Subject: [PATCH] [K/N] HashMap/HashSet doesn't reclaim storage after removing elements #KT-53310 --- .../kotlin/collections/builders/MapBuilder.kt | 27 +++++++++++++++---- .../jvm/test/collections/MapBuilderTest.kt | 15 +++++++++++ .../src/kotlin/collections/HashMap.kt | 16 ++++++++--- 3 files changed, 50 insertions(+), 8 deletions(-) diff --git a/libraries/stdlib/jvm/src/kotlin/collections/builders/MapBuilder.kt b/libraries/stdlib/jvm/src/kotlin/collections/builders/MapBuilder.kt index a807cec9b4c..27c1fad36c1 100644 --- a/libraries/stdlib/jvm/src/kotlin/collections/builders/MapBuilder.kt +++ b/libraries/stdlib/jvm/src/kotlin/collections/builders/MapBuilder.kt @@ -10,11 +10,17 @@ import java.io.InvalidObjectException import java.io.NotSerializableException internal class MapBuilder private constructor( + // keys in insert order private var keysArray: Array, - private var valuesArray: Array?, // allocated only when actually used, always null in pure HashSet + // values in insert order, allocated only when actually used, always null in pure HashSet + private var valuesArray: Array?, + // hash of a key by its index, -1 if a key at that index was removed private var presenceArray: IntArray, + // (index + 1) of a key by its hash, 0 if there is no key with that hash, -1 if collision chain continues to the hash-1 private var hashArray: IntArray, + // max length of a collision chain private var maxProbeDistance: Int, + // index of the next key to be inserted private var length: Int ) : MutableMap, Serializable { private var hashShift: Int = computeShift(hashSize) @@ -165,7 +171,8 @@ internal class MapBuilder private constructor( // ---------------------------- private ---------------------------- - private val capacity: Int get() = keysArray.size + // Declared internal for testing + internal val capacity: Int get() = keysArray.size private val hashSize: Int get() = hashArray.size internal fun checkIsMutable() { @@ -173,7 +180,19 @@ internal class MapBuilder private constructor( } private fun ensureExtraCapacity(n: Int) { - ensureCapacity(length + n) + if (shouldCompact(extraCapacity = n)) { + rehash(hashSize) + } else { + ensureCapacity(length + n) + } + } + + private fun shouldCompact(extraCapacity: Int): Boolean { + val spareCapacity = this.capacity - length + val gaps = length - size + return spareCapacity < extraCapacity // there is no room for extraCapacity entries + && gaps + spareCapacity >= extraCapacity // removing gaps prevents capacity expansion + && gaps >= this.capacity / 4 // at least 25% of current capacity is occupied by gaps } private fun ensureCapacity(capacity: Int) { @@ -186,8 +205,6 @@ internal class MapBuilder private constructor( presenceArray = presenceArray.copyOf(newSize) val newHashSize = computeHashSize(newSize) if (newHashSize > hashSize) rehash(newHashSize) - } else if (length + capacity - size > this.capacity) { - rehash(hashSize) } } diff --git a/libraries/stdlib/jvm/test/collections/MapBuilderTest.kt b/libraries/stdlib/jvm/test/collections/MapBuilderTest.kt index bd9be54c8a0..e5a8c61d2fd 100644 --- a/libraries/stdlib/jvm/test/collections/MapBuilderTest.kt +++ b/libraries/stdlib/jvm/test/collections/MapBuilderTest.kt @@ -3,8 +3,11 @@ * Use of this source code is governed by the Apache 2.0 license that can be found in the license/LICENSE.txt file. */ +@file:Suppress("INVISIBLE_REFERENCE", "INVISIBLE_MEMBER") + package test.collections +import kotlin.collections.builders.MapBuilder import kotlin.test.assertEquals import kotlin.test.assertFails import kotlin.test.Test @@ -37,4 +40,16 @@ class MapBuilderTest { assertEquals(builderSize, size) } } + + // KT-53310 + @Test + fun reclaimStorage() { + val builder = MapBuilder() + val initialCapacity = builder.capacity + repeat(20) { + builder[it] = it + builder.remove(it) + } + assertEquals(initialCapacity, builder.capacity) + } } \ No newline at end of file diff --git a/libraries/stdlib/native-wasm/src/kotlin/collections/HashMap.kt b/libraries/stdlib/native-wasm/src/kotlin/collections/HashMap.kt index edfeaf4ed2e..7aae14ae2af 100644 --- a/libraries/stdlib/native-wasm/src/kotlin/collections/HashMap.kt +++ b/libraries/stdlib/native-wasm/src/kotlin/collections/HashMap.kt @@ -178,7 +178,19 @@ actual class HashMap private constructor( } private fun ensureExtraCapacity(n: Int) { - ensureCapacity(length + n) + if (shouldCompact(extraCapacity = n)) { + rehash(hashSize) + } else { + ensureCapacity(length + n) + } + } + + private fun shouldCompact(extraCapacity: Int): Boolean { + val spareCapacity = this.capacity - length + val gaps = length - size + return spareCapacity < extraCapacity // there is no room for extraCapacity entries + && gaps + spareCapacity >= extraCapacity // removing gaps prevents capacity expansion + && gaps >= this.capacity / 4 // at least 25% of current capacity is occupied by gaps } private fun ensureCapacity(capacity: Int) { @@ -191,8 +203,6 @@ actual class HashMap private constructor( presenceArray = presenceArray.copyOf(newSize) val newHashSize = computeHashSize(newSize) if (newHashSize > hashSize) rehash(newHashSize) - } else if (length + capacity - _size > this.capacity) { - rehash(hashSize) } }