diff --git a/bench/resources/sjsonnet_suite/array_copy_views.jsonnet b/bench/resources/sjsonnet_suite/array_copy_views.jsonnet new file mode 100644 index 00000000..cfc0b7da --- /dev/null +++ b/bench/resources/sjsonnet_suite/array_copy_views.jsonnet @@ -0,0 +1,24 @@ +local block = std.range(0, 4095); +local pair = block + block; +local mixed = [ + std.repeat(block, 64), + pair, + block[100:4000:2], + std.repeat(pair, 16), +]; +local flattened = std.flattenArrays(mixed); +local joined = std.join([], mixed); +local len = std.length(flattened); +local probes = std.range(0, 39999); +{ + len: len, + joinedLen: std.length(joined), + first: flattened[0], + middle: flattened[std.floor(len / 2)], + last: flattened[len - 1], + checksum: std.foldl( + function(acc, i) acc + flattened[(i * 7919) % len], + probes, + 0 + ), +} diff --git a/bench/resources/sjsonnet_suite/array_copy_views.jsonnet.golden b/bench/resources/sjsonnet_suite/array_copy_views.jsonnet.golden new file mode 100644 index 00000000..7aa0e8a5 --- /dev/null +++ b/bench/resources/sjsonnet_suite/array_copy_views.jsonnet.golden @@ -0,0 +1,8 @@ +{ + "checksum": 81774965, + "first": 0, + "joinedLen": 403358, + "last": 4095, + "len": 403358, + "middle": 975 +} diff --git a/sjsonnet/src/sjsonnet/Val.scala b/sjsonnet/src/sjsonnet/Val.scala index 9b9290d3..7b287969 100644 --- a/sjsonnet/src/sjsonnet/Val.scala +++ b/sjsonnet/src/sjsonnet/Val.scala @@ -582,6 +582,45 @@ object Val { if (!isConcatView && !_reversed && (arr ne null)) arr.asInstanceOf[Array[Eval]] else null + private[sjsonnet] def copyEvalTo(out: mutable.ArrayBuilder.ofRef[Eval]): Unit = { + if (isConcatView) { + _concatLeft.copyEvalTo(out) + _concatRight.copyEvalTo(out) + } else { + val direct = directBackingArray + if (direct != null) out ++= direct + else { + val len = length + var i = 0 + while (i < len) { + out += eval(i) + i += 1 + } + } + } + } + + private[sjsonnet] def copyEvalTo(dest: Array[Eval], offset: Int): Int = { + if (isConcatView) { + val next = _concatLeft.copyEvalTo(dest, offset) + _concatRight.copyEvalTo(dest, next) + } else { + val direct = directBackingArray + if (direct != null) { + System.arraycopy(direct, 0, dest, offset, direct.length) + offset + direct.length + } else { + val len = length + var i = 0 + while (i < len) { + dest(offset + i) = eval(i) + i += 1 + } + offset + len + } + } + } + /** * If both this and other are ConcatViews sharing the same left array, return the shared prefix * length. Otherwise return 0. Used by compare/equal to skip identical prefix elements entirely, @@ -630,15 +669,8 @@ object Val { * contents and the concat references are released for GC. */ private def materialize(): Unit = { - val left = _concatLeft - val right = _concatRight - val lArr = left.asLazyArray - val rArr = right.asLazyArray - val lLen = lArr.length - val rLen = rArr.length - val result = new Array[Eval](lLen + rLen) - System.arraycopy(lArr, 0, result, 0, lLen) - System.arraycopy(rArr, 0, result, lLen, rLen) + val result = new Array[Eval](_length) + copyEvalTo(result, 0) arr = result _concatLeft = null _concatRight = null @@ -671,11 +703,9 @@ object Val { } else { // Eager path: allocate + arraycopy (also used when either side is a concat view // to flatten and prevent deep nesting) - val lArr = this.asLazyArray - val rArr = rhs.asLazyArray val result = new Array[Eval](totalLen) - System.arraycopy(lArr, 0, result, 0, leftLen) - System.arraycopy(rArr, 0, result, leftLen, rhsLen) + this.copyEvalTo(result, 0) + rhs.copyEvalTo(result, leftLen) Arr(newPos, result) } } @@ -863,6 +893,28 @@ object Val { result } + override private[sjsonnet] def copyEvalTo(out: mutable.ArrayBuilder.ofRef[Eval]): Unit = { + val sourceArr = source.asLazyArray + var i = 0 + while (i < count) { + out ++= sourceArr + i += 1 + } + } + + override private[sjsonnet] def copyEvalTo(dest: Array[Eval], offset: Int): Int = { + val sourceArr = source.asLazyArray + val sourceLen = this.sourceLen + val len = _length + var current = offset + val end = offset + len + while (current < end) { + System.arraycopy(sourceArr, 0, dest, current, sourceLen) + current += sourceLen + } + end + } + override def reversed(newPos: Position): Arr = new RepeatedArr(newPos, source.reversed(newPos), count) } @@ -891,14 +943,35 @@ object Val { else { val len = _length val result = new Array[Eval](len) + copyEvalTo(result, 0) + arr = result + source = null + result + } + } + + override private[sjsonnet] def copyEvalTo(out: mutable.ArrayBuilder.ofRef[Eval]): Unit = { + if ((arr ne null) || isConcatView) super.copyEvalTo(out) + else { + val len = _length var i = 0 while (i < len) { - result(i) = source.eval(sourceIndex(i)) + out += source.eval(sourceIndex(i)) i += 1 } - arr = result - source = null - result + } + } + + override private[sjsonnet] def copyEvalTo(dest: Array[Eval], offset: Int): Int = { + if ((arr ne null) || isConcatView) super.copyEvalTo(dest, offset) + else { + val len = _length + var i = 0 + while (i < len) { + dest(offset + i) = source.eval(sourceIndex(i)) + i += 1 + } + offset + len } } @@ -1046,13 +1119,33 @@ object Val { else { val len = _length val result = new Array[Eval](len) + copyEvalTo(result, 0) + arr = result + result + } + } + + override private[sjsonnet] def copyEvalTo(out: mutable.ArrayBuilder.ofRef[Eval]): Unit = { + if ((arr ne null) || isConcatView) super.copyEvalTo(out) + else { + var i = _length - 1 + while (i >= 0) { + out += source.eval(i) + i -= 1 + } + } + } + + override private[sjsonnet] def copyEvalTo(dest: Array[Eval], offset: Int): Int = { + if ((arr ne null) || isConcatView) super.copyEvalTo(dest, offset) + else { + val len = _length var i = 0 while (i < len) { - result(i) = source.eval(len - 1 - i) + dest(offset + i) = source.eval(len - 1 - i) i += 1 } - arr = result - result + offset + len } } @@ -1181,6 +1274,31 @@ object Val { super.asLazyArray } + override private[sjsonnet] def copyEvalTo(out: mutable.ArrayBuilder.ofRef[Eval]): Unit = { + if (isMaterialized || isConcatView) super.copyEvalTo(out) + else { + val len = _length + var i = 0 + while (i < len) { + out += Val.cachedNum(pos, doubleAt(i)) + i += 1 + } + } + } + + override private[sjsonnet] def copyEvalTo(dest: Array[Eval], offset: Int): Int = { + if (isMaterialized || isConcatView) super.copyEvalTo(dest, offset) + else { + val len = _length + var i = 0 + while (i < len) { + dest(offset + i) = Val.cachedNum(pos, doubleAt(i)) + i += 1 + } + offset + len + } + } + override def reversed(newPos: Position): Arr = { if (isMaterialized || isConcatView) { super.reversed(newPos) @@ -1200,15 +1318,8 @@ object Val { /** Materialize this range into a flat Val.Num array. */ private def materializeRange(): Unit = { val len = _length - val from = rangeFrom - val rev = _reversed - val p = pos val result = new Array[Eval](len) - var i = 0 - while (i < len) { - result(i) = Val.cachedNum(p, if (rev) from - i else from + i) - i += 1 - } + copyEvalTo(result, 0) arr = result _reversed = false // materialized in correct order } @@ -1250,6 +1361,35 @@ object Val { super.asLazyArray } + override private[sjsonnet] def copyEvalTo(out: mutable.ArrayBuilder.ofRef[Eval]): Unit = { + if (isMaterialized || isConcatView) super.copyEvalTo(out) + else { + val bytes = byteData + val len = bytes.length + val p = pos + var i = 0 + while (i < len) { + out += Val.cachedNum(p, (bytes(i) & 0xff).toDouble) + i += 1 + } + } + } + + override private[sjsonnet] def copyEvalTo(dest: Array[Eval], offset: Int): Int = { + if (isMaterialized || isConcatView) super.copyEvalTo(dest, offset) + else { + val bytes = byteData + val len = bytes.length + val p = pos + var i = 0 + while (i < len) { + dest(offset + i) = Val.cachedNum(p, (bytes(i) & 0xff).toDouble) + i += 1 + } + offset + len + } + } + // DO NOT CHANGE // WHY: reversed() materializes first instead of flipping a _reversed flag. This keeps // ByteArr simple — value()/eval()/rawBytes never need to handle reversed indexing. @@ -1264,13 +1404,8 @@ object Val { private def materializeByteData(): Unit = { val bytes = byteData val len = bytes.length - val p = pos val result = new Array[Eval](len) - var i = 0 - while (i < len) { - result(i) = Val.cachedNum(p, (bytes(i) & 0xff).toDouble) - i += 1 - } + copyEvalTo(result, 0) arr = result } } diff --git a/sjsonnet/src/sjsonnet/stdlib/ArrayModule.scala b/sjsonnet/src/sjsonnet/stdlib/ArrayModule.scala index 26cea699..fc115380 100644 --- a/sjsonnet/src/sjsonnet/stdlib/ArrayModule.scala +++ b/sjsonnet/src/sjsonnet/stdlib/ArrayModule.scala @@ -387,11 +387,7 @@ object ArrayModule extends AbstractFunctionModule { for (x <- arr) { x.value match { case v: Val.Arr => - var i = 0 - while (i < v.length) { - out += v.eval(i) - i += 1 - } + v.copyEvalTo(out) case x => Error.fail("binary operator + requires matching types, got array and " + x.prettyName) } @@ -820,12 +816,7 @@ object ArrayModule extends AbstractFunctionModule { while (i < subArrays.length) { val sub = subArrays(i) if (sub != null) { - var j = 0 - while (j < sub.length) { - result(offset + j) = sub.eval(j) - j += 1 - } - offset += sub.length + offset = sub.copyEvalTo(result, offset) } i += 1 } diff --git a/sjsonnet/src/sjsonnet/stdlib/StringModule.scala b/sjsonnet/src/sjsonnet/stdlib/StringModule.scala index eb6e69f9..c1a6d17e 100644 --- a/sjsonnet/src/sjsonnet/stdlib/StringModule.scala +++ b/sjsonnet/src/sjsonnet/stdlib/StringModule.scala @@ -418,15 +418,7 @@ object StringModule extends AbstractFunctionModule { */ private object Join extends Val.Builtin2("join", "sep", "arr") { private def appendArray(out: mutable.ArrayBuilder.ofRef[Eval], arr: Val.Arr): Unit = { - val direct = arr.directBackingArray - if (direct != null) out ++= direct - else { - var i = 0 - while (i < arr.length) { - out += arr.eval(i) - i += 1 - } - } + arr.copyEvalTo(out) } def evalRhs(sep: Eval, _arr: Eval, ev: EvalScope, pos: Position): Val = { diff --git a/sjsonnet/test/src/sjsonnet/ValArrayViewTests.scala b/sjsonnet/test/src/sjsonnet/ValArrayViewTests.scala index 069c1e18..e37a0f1c 100644 --- a/sjsonnet/test/src/sjsonnet/ValArrayViewTests.scala +++ b/sjsonnet/test/src/sjsonnet/ValArrayViewTests.scala @@ -5,7 +5,7 @@ import utest._ object ValArrayViewTests extends TestSuite { private val testPos = new Position(null, -1) - private final class NoBulkArr(size: Int) extends Val.Arr(testPos, Val.Arr.EMPTY_EVAL_ARRAY) { + private final class NoBulkArr(size: Int) extends Val.Arr(testPos, null) { var bulkCalls: Int = 0 override def length: Int = size @@ -43,5 +43,21 @@ object ValArrayViewTests extends TestSuite { materialized(2).value.asDouble ==> 106.0 source.bulkCalls ==> 0 } + + test("copyEvalToAvoidsBulkMaterializingConcatChildren") { + val left = new NoBulkArr(5000) + val right = new NoBulkArr(5000) + val concat = left.concat(testPos, right) + + val result = new Array[Eval](concat.length) + concat.copyEvalTo(result, 0) ==> 10000 + + result(0).value.asDouble ==> 0.0 + result(4999).value.asDouble ==> 4999.0 + result(5000).value.asDouble ==> 0.0 + result(9999).value.asDouble ==> 4999.0 + left.bulkCalls ==> 0 + right.bulkCalls ==> 0 + } } }