Vectors smaller than 32 allocate or reuse small arrays when possible #7743
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
joshlemer added WIP performance 
joshlemer changed the title 
viktorklang reviewed Feb 12, 2019
joshlemer force-pushed the small-vectors branch 3 times, most recently from ef7b1bd to f8c01a6 Compare 
joshlemer commented Feb 14, 2019
| Unfortunately I am seeing about a 40% slowdown in this code: @Benchmark def appended0To33(bh: Blackhole): Unit = { var v: Vector[Int] = Vector.empty[Int] var i = 0 while (i < 33) { v = v.appended(i) i += 1 } bh.consume(v) }hoping to avoid that.. |
| ❗ (tentatively) Fixed it, turned that around to a 50-70% speedup over 2.13.x, and on par with ArraySeq in the more general case... @Param(Array("1", "10", "100", "1000", "10000")) var size: Int = _ @Benchmark def appended0ToN(bh: Blackhole): Unit = { var v: Vector[Int] = Vector.empty[Int] var i = 0 while (i < size) { v = v.appended(i) i += 1 } bh.consume(v) } |
joshlemer force-pushed the small-vectors branch 4 times, most recently from 821065a to 07f352f Compare joshlemer commented Feb 15, 2019
| s.display0(lo) = value.asInstanceOf[AnyRef] | ||
| val thisLength = length | ||
| val result = | ||
| if (depth == 1 && thisLength < 32) { |
There was a problem hiding this comment.
This branch and the Vector.single branch are the only changed branches, the reset of this method is just indentation
joshlemer commented Feb 15, 2019
| } else { | ||
| val shift = startIndex & ~((1 << (5 * (depth - 1))) - 1) | ||
| val shiftBlocks = startIndex >>> (5 * (depth - 1)) | ||
| } else if (thisLength > 0) { |
There was a problem hiding this comment.
Mostly indentation, plus changing condition to have a bit more obvious of logic:
old:
if (startIndex != endIndex)
new:
if (thisLength > 0) // thisLength already computed anyways
joshlemer changed the title joshlemer force-pushed the small-vectors branch from 07f352f to 2277048 Compare joshlemer force-pushed the small-vectors branch from 2277048 to 0ca94c6 Compare 
retronym reviewed Mar 27, 2019
retronym reviewed Mar 27, 2019
retronym reviewed Mar 27, 2019
retronym reviewed Mar 27, 2019
| System.arraycopy(display0, startIndex, newDisplay0, 1, thisLength) | ||
| newDisplay0(0) = value.asInstanceOf[AnyRef] | ||
| s.display0 = newDisplay0 | ||
| s |
There was a problem hiding this comment.
👍 already Covered by the releaseFence below.
retronym reviewed Mar 27, 2019
| System.arraycopy(display0, startIndex, newDisplay0, 0, thisLength) | ||
| newDisplay0(thisLength) = value.asInstanceOf[AnyRef] | ||
| s.display0 = newDisplay0 | ||
| s |
There was a problem hiding this comment.
👍 already Covered by the releaseFence below.
retronym reviewed Mar 27, 2019
| Just pushed a commit with the fences |
retronym approved these changes Mar 27, 2019
…ating from varargs, reuse underlying array
SethTisue force-pushed the small-vectors branch from 67752df to fe80a23 Compare 
| rebased (trivial merge conflict involving imports), let's merge once CI likes it |
yeah, it's awesome this got in, thanks Josh |
SethTisue changed the title Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
6 participants
Add this suggestion to a batch that can be applied as a single commit. This suggestion is invalid because no changes were made to the code. Suggestions cannot be applied while the pull request is closed. Suggestions cannot be applied while viewing a subset of changes. Only one suggestion per line can be applied in a batch. Add this suggestion to a batch that can be applied as a single commit. Applying suggestions on deleted lines is not supported. You must change the existing code in this line in order to create a valid suggestion. Outdated suggestions cannot be applied. This suggestion has been applied or marked resolved. Suggestions cannot be applied from pending reviews. Suggestions cannot be applied on multi-line comments. Suggestions cannot be applied while the pull request is queued to merge. Suggestion cannot be applied right now. Please check back later.
One critique often made against Vectors is that small Vectors consume a lot of memory (See Li Haoyi's post here ). Previous to this PR, Vectors of sizes in range [1, 31] would always round up and allocate an Array of size 32. Now, when creating a Vector with known size < 32, only exactly that size of an Array is allocated.
As well, when creating from varargs of
AnyRefs (collection.immutable.ArraySeq[T <: AnyRef]) of size <= 32, we reuse the underlyingArray[AnyRef].Performance of creating a Vector from varargs is greatly improved, for both AnyRef and AnyVal types.
Benchmarks
Code:
GC-profiled benchmark:
Edit:
Some benchmark for
Vector.from(ArrayBuffer)