Override .slice in ArrayOps to use arraycopy.#5250
Conversation
This makes it ~10x faster when copying large chunks arround.
My benchmark:
def bm(duration: Long)(f: => Unit): Int = {
val end = System.currentTimeMillis + duration
var count = 0
while(System.currentTimeMillis < end) {
f
count += 1
}
count
}
def measure(seconds: Int)(f: => Unit) = (1 to seconds).map { _ => bm(1000)(f) }.sum / seconds
val array = scala.util.Random.alphanumeric.take(1000).toArray
measure(20) { array.slice(100, 500) }
// ~5 million
measure(20) { scala.collection.WrappedArray(array).slice(100, 500) }
// ~300K
25209d6 to
add83b8
Compare
| override def slice(from: Int, until: Int): Array[T] = { | ||
| val lo = math.max(from, 0) | ||
| val hi = math.min(math.max(until, 0), repr.length) | ||
| val size = math.max(hi-lo, 0) |
There was a problem hiding this comment.
We usually delimit operators like - with whitespaces.
|
Thanks for your contribution! LGTM on my side. Please verify if there is a test for |
|
@dimatkach for test coverage, there is at least https://github.com/scala/scala/blob/2.12.x/test/files/run/slices.scala. If you'd like to add to it, that test could be converted to a JUnit test (see https://github.com/scala/scala/tree/2.12.x/test/junit/scala/collection). |
|
@lrytz I am not sure I understand - why/where do I need to "add" it, if it's already there? :) |
|
Oh, i was just pointing out the existing test, but you already know about it then. My question is if you think it is comprehensive enough. If you know of more corner cases that should be tested, you could add them to the test, and if you work on the test you could consider converting it to JUnit. JUnit tests run more efficiently. Also, it would be nice to address Alex's code style remarks. For setting up your local dev env, see https://github.com/scala/scala#get-ready-to-contribute. |
|
LGTM |
This makes it ~10x faster when copying large chunks arround.
My benchmark: