From d66f07a84ecc163a4225fc5639a8a754f47da546 Mon Sep 17 00:00:00 2001 From: Abduqodiri Qurbonzoda Date: Mon, 16 Sep 2019 20:34:13 +0300 Subject: [PATCH] CollectionsKt.windowed throws IllegalArgumentException --- .../common/src/generated/_Collections.kt | 3 +- .../stdlib/common/src/generated/_Strings.kt | 3 +- .../src/kotlin/collections/SlidingWindow.kt | 28 +++++++++++++++---- .../stdlib/test/collections/IterableTests.kt | 5 ++++ .../stdlib/test/collections/SequenceTest.kt | 15 ++++++++++ libraries/stdlib/test/text/StringTest.kt | 5 ++++ 6 files changed, 51 insertions(+), 8 deletions(-) diff --git a/libraries/stdlib/common/src/generated/_Collections.kt b/libraries/stdlib/common/src/generated/_Collections.kt index f6f39d52828..29a5b2b740d 100644 --- a/libraries/stdlib/common/src/generated/_Collections.kt +++ b/libraries/stdlib/common/src/generated/_Collections.kt @@ -2147,7 +2147,8 @@ public fun Iterable.windowed(size: Int, step: Int = 1, partialWindows: Bo checkWindowSizeStep(size, step) if (this is RandomAccess && this is List) { val thisSize = this.size - val result = ArrayList>((thisSize + step - 1) / step) + val resultCapacity = thisSize / step + if (thisSize % step == 0) 0 else 1 + val result = ArrayList>(resultCapacity) var index = 0 while (index < thisSize) { val windowSize = size.coerceAtMost(thisSize - index) diff --git a/libraries/stdlib/common/src/generated/_Strings.kt b/libraries/stdlib/common/src/generated/_Strings.kt index 58462562c64..71c2cd05bb7 100644 --- a/libraries/stdlib/common/src/generated/_Strings.kt +++ b/libraries/stdlib/common/src/generated/_Strings.kt @@ -1374,7 +1374,8 @@ public fun CharSequence.windowed(size: Int, step: Int = 1, partialWindows: Boole public fun CharSequence.windowed(size: Int, step: Int = 1, partialWindows: Boolean = false, transform: (CharSequence) -> R): List { checkWindowSizeStep(size, step) val thisSize = this.length - val result = ArrayList((thisSize + step - 1) / step) + val resultCapacity = thisSize / step + if (thisSize % step == 0) 0 else 1 + val result = ArrayList(resultCapacity) var index = 0 while (index < thisSize) { val end = index + size diff --git a/libraries/stdlib/src/kotlin/collections/SlidingWindow.kt b/libraries/stdlib/src/kotlin/collections/SlidingWindow.kt index 20be2a28453..f8321fcb842 100644 --- a/libraries/stdlib/src/kotlin/collections/SlidingWindow.kt +++ b/libraries/stdlib/src/kotlin/collections/SlidingWindow.kt @@ -22,9 +22,10 @@ internal fun Sequence.windowedSequence(size: Int, step: Int, partialWindo internal fun windowedIterator(iterator: Iterator, size: Int, step: Int, partialWindows: Boolean, reuseBuffer: Boolean): Iterator> { if (!iterator.hasNext()) return EmptyIterator return iterator> { + val bufferInitialCapacity = size.coerceAtMost(1024) val gap = step - size if (gap >= 0) { - var buffer = ArrayList(size) + var buffer = ArrayList(bufferInitialCapacity) var skip = 0 for (e in iterator) { if (skip > 0) { skip -= 1; continue } @@ -39,10 +40,12 @@ internal fun windowedIterator(iterator: Iterator, size: Int, step: Int, p if (partialWindows || buffer.size == size) yield(buffer) } } else { - val buffer = RingBuffer(size) + var buffer = RingBuffer(bufferInitialCapacity) for (e in iterator) { buffer.add(e) if (buffer.isFull()) { + if (buffer.size < size) { buffer = buffer.expanded(maxCapacity = size); continue } + yield(if (reuseBuffer) buffer else ArrayList(buffer)) buffer.removeFirst(step) } @@ -83,15 +86,18 @@ internal class MovingSubList(private val list: List) : AbstractList * * Buffer overflow is not allowed so [add] doesn't overwrite tail but raises an exception. */ -private class RingBuffer(val capacity: Int) : AbstractList(), RandomAccess { +private class RingBuffer(private val buffer: Array, filledSize: Int) : AbstractList(), RandomAccess { init { - require(capacity >= 0) { "ring buffer capacity should not be negative but it is $capacity" } + require(filledSize >= 0) { "ring buffer filled size should not be negative but it is $filledSize" } + require(filledSize <= buffer.size) { "ring buffer filled size: $filledSize cannot be larger than the buffer size: ${buffer.size}" } } - private val buffer = arrayOfNulls(capacity) + constructor(capacity: Int) : this(arrayOfNulls(capacity), 0) + + private val capacity = buffer.size private var startIndex: Int = 0 - override var size: Int = 0 + override var size: Int = filledSize private set override fun get(index: Int): T { @@ -149,6 +155,16 @@ private class RingBuffer(val capacity: Int) : AbstractList(), RandomAccess return toArray(arrayOfNulls(size)) } + /** + * Creates a new ring buffer with the capacity equal to the minimum of [maxCapacity] and 1.5 * [capacity]. + * The returned ring buffer contains the same elements as this ring buffer. + */ + fun expanded(maxCapacity: Int): RingBuffer { + val newCapacity = (capacity + (capacity shr 1) + 1).coerceAtMost(maxCapacity) + val newBuffer = if (startIndex == 0) buffer.copyOf(newCapacity) else toArray(arrayOfNulls(newCapacity)) + return RingBuffer(newBuffer, size) + } + /** * Add [element] to the buffer or fail with [IllegalStateException] if no free space available in the buffer */ diff --git a/libraries/stdlib/test/collections/IterableTests.kt b/libraries/stdlib/test/collections/IterableTests.kt index c9aa0ab240a..0b799f4513d 100644 --- a/libraries/stdlib/test/collections/IterableTests.kt +++ b/libraries/stdlib/test/collections/IterableTests.kt @@ -140,6 +140,11 @@ abstract class OrderedIterableTests>(createFrom: (Array assertEquals(expectedSingleChunk, data.chunked(size).single()) assertEquals(expectedSingleChunk, data.chunked(size + 3).single()) + assertEquals(expectedSingleChunk, data.chunked(Int.MAX_VALUE).single()) + } + + createFrom("a", "b").let { iterable -> + assertEquals(iterable.toList(), iterable.chunked(Int.MAX_VALUE).single()) } assertTrue(empty.chunked(3).isEmpty()) diff --git a/libraries/stdlib/test/collections/SequenceTest.kt b/libraries/stdlib/test/collections/SequenceTest.kt index 175f191be9f..1a6cc2a5ab6 100644 --- a/libraries/stdlib/test/collections/SequenceTest.kt +++ b/libraries/stdlib/test/collections/SequenceTest.kt @@ -230,6 +230,11 @@ public class SequenceTest { seq.toList().let { expectedSingleChunk -> assertEquals(expectedSingleChunk, seq.chunked(size).single()) assertEquals(expectedSingleChunk, seq.chunked(size + 3).single()) + assertEquals(expectedSingleChunk, seq.chunked(Int.MAX_VALUE).single()) + } + + infiniteSeq.take(2).let { seq2 -> + assertEquals(seq2.toList(), seq2.chunked(Int.MAX_VALUE).single()) } assertTrue(emptySequence().chunked(3).none()) @@ -251,6 +256,16 @@ public class SequenceTest { assertEquals((startElement until startElement + 5).toList(), window) } + infiniteSeq.take(3500).windowed(2000, 1000, partialWindows = true).forEachIndexed { windowIndex, window -> + val startElement = windowIndex * 1000 + val elementCount = when (windowIndex) { + 3 -> 500 + 2 -> 1500 + else -> 2000 + } + assertEquals((startElement until startElement + elementCount).toList(), window) + } + val size = 7 val seq = infiniteSeq.take(7) diff --git a/libraries/stdlib/test/text/StringTest.kt b/libraries/stdlib/test/text/StringTest.kt index f05639ae48e..2299011dce6 100644 --- a/libraries/stdlib/test/text/StringTest.kt +++ b/libraries/stdlib/test/text/StringTest.kt @@ -1068,6 +1068,11 @@ class StringTest { data.toString().let { expectedSingleChunk -> assertEquals(expectedSingleChunk, data.chunked(size).single()) assertEquals(expectedSingleChunk, data.chunked(size + 3).single()) + assertEquals(expectedSingleChunk, data.chunked(Int.MAX_VALUE).single()) + } + + arg1("ab").let { + assertEquals(it.toString(), it.chunked(Int.MAX_VALUE).single()) } assertTrue(arg1("").chunked(3).isEmpty())