perf: While-loop stdlib optimizations + base64DecodeBytes unsigned fix

[He-Pin]

Motivation

Several stdlib functions use functional-style operations (forEach, map, forall, slice ++ slice) that allocate closures and intermediate arrays on every invocation. These are hot paths in realistic workloads involving object field enumeration, base64 decoding, array sum/avg, and element removal.

Additionally, std.base64DecodeBytes had a latent bug where bytes >= 128 were sign-extended to negative numbers instead of being treated as unsigned (0-255), inconsistent with the Go and C++ Jsonnet implementations.

Key Design Decisions

1. Iterator while-loop for visibleKeyNames: Replace getAllKeys.forEach lambda with entrySet().iterator() while-loop to avoid per-key closure allocation
2. Single-pass sum/avg: Merge the separate forall type-check pass and map+sum computation into one while-loop that validates and accumulates simultaneously
3. System.arraycopy for remove/removeAt: Replace slice ++ slice with direct System.arraycopy to avoid intermediate array allocation
4. Unsigned byte mask for base64DecodeBytes: Apply & 0xff mask to convert signed Java bytes to unsigned (0-255), matching Go-jsonnet behavior
5. Cached allLocals for ObjComp: Cache preLocals ++ postLocals as lazy val to avoid repeated array concatenation in visitObjComp

Modification

• Val.scala: Replace getAllKeys.forEach with entrySet().iterator() while-loop in visibleKeyNames
• ArrayModule.scala: Single-pass while-loop for sum/avg; System.arraycopy for remove/removeAt
• EncodingModule.scala: While-loop with & 0xff unsigned mask for base64DecodeBytes
• Expr.scala: Add lazy val allLocals to ObjBody.ObjComp
• Evaluator.scala: Use e.allLocals instead of e.preLocals ++ e.postLocals
• New test: Regression test for base64DecodeBytes unsigned byte round-trip

Benchmark Results

JMH A/B (bench.02, JVM, -wi 3 -i 5 -f 1 -t 1)

Version	Score	Error	Units
master	43.962	± 0.768	ms/op
optimized	40.986	± 0.474	ms/op
Δ	-6.8%

base64DecodeBytes (no significant change expected — decode-time dominates)

Version	Score	Error	Units
master	8.801	± 0.342	ms/op
optimized	8.787	± 0.454	ms/op

Analysis

• The -6.8% improvement on bench.02 comes from the combined effect of while-loop conversions eliminating closure and intermediate array allocations
• base64DecodeBytes unchanged because the benchmark is dominated by decode time, not array construction
• No regressions observed across the full 35-case regression suite
• The & 0xff fix corrects a latent bug: Val.Num(pos, (byte)0xFF) would produce -1 instead of 255

References

• Upstream jit branch commits: 91496549, b833428e, 09e2d3ad
• Go-jsonnet base64DecodeBytes: uses float64(int(decodedBytes[i])) which gives unsigned 0-255

Result

All tests pass (46/46). No regressions. -6.8% improvement on bench.02 with a bug fix for base64DecodeBytes high-byte handling.

[He-Pin]

Code Review

I found correctness issues in this PR:

1. base64DecodeBytes — & 0xff fix is correct

result(i) = Val.Num(pos, decoded(i) & 0xff)

Java byte is signed (-128 to 127). decoded(i) is a Byte. decoded(i) & 0xff promotes to Int and masks to 0-255. For example:

• 0xFF.toByte = -1 (signed), -1 & 0xff = 255 ✓
• 0x80.toByte = -128 (signed), -128 & 0xff = 128 ✓
• 0x01.toByte = 1 (signed), 1 & 0xff = 1 ✓

This is correct and fixes a real bug. The original code Val.Num(pos, i) where i: Byte would auto-widen to a signed value, so byte 0xFF (-1) became -1.0 instead of 255.0.

2. std.sum — same Double return issue as PR #702

builtin("sum", "arr") { (_, _, arr: Val.Arr) =>
  val a = arr.asLazyArray
  var sum = 0.0
  var i = 0
  while (i < a.length) {
    val v = a(i).value
    if (!v.isInstanceOf[Val.Num]) Error.fail("Argument must be an array of numbers")
    sum += v.asInstanceOf[Val.Num].asDouble
    i += 1
  }
  sum  // <-- Returns Double
}

Same concern as PR #702 — this returns a Double where a Val is expected. If there's no implicit Double => Val in scope, this won't compile. Please verify this compiles.

3. std.avg — same Double return issue

sum / arr.length  // Same concern

Also, arr.length is an Int. sum / arr.length performs Double / Int which promotes to Double. The division is correct. No arithmetic issue.

4. visibleKeyNames — iterator while-loop is correct

val iter = getAllKeys.entrySet().iterator()
while (iter.hasNext()) {
  val e = iter.next()
  if (e.getValue() == java.lang.Boolean.FALSE) buf += e.getKey()
}

The original used getAllKeys.forEach((k, b) => if (b == java.lang.Boolean.FALSE) buf += k). The new code iterates entrySet() which has the same semantics. The == comparison with java.lang.Boolean.FALSE checks reference equality for the singleton Boolean.FALSE, which is correct since LinkedHashMap stores Boolean objects. Correct.

5. ObjComp.allLocals — lazy val is thread-safe?

lazy val allLocals: Array[Bind] = preLocals ++ postLocals

In Scala, lazy val is thread-safe (uses double-checked locking). However, Jsonnet evaluation is typically single-threaded, so this overhead is unnecessary but harmless. No issue.

6. std.remove / std.removeAt — System.arraycopy correctness

System.arraycopy(src, 0, result, 0, idx)
System.arraycopy(src, idx + 1, result, idx, src.length - idx - 1)

For removeAt with idx == 0:

• First call: arraycopy(src, 0, result, 0, 0) — copies 0 elements, valid.
• Second call: arraycopy(src, 1, result, 0, src.length - 1) — copies all elements after index 0.

For idx == src.length - 1:

• First call: arraycopy(src, 0, result, 0, src.length - 1) — copies all elements before the last.
• Second call: arraycopy(src, src.length, result, src.length - 1, 0) — copies 0 elements, valid.

Correct.

7. std.sum — single-pass validation changes error message behavior

Original: arr.forall(_.isInstanceOf[Val.Num]) validates ALL elements first, then map(_.value.asDouble).sum computes. If there are multiple non-numeric elements, the error message is generic ("Argument must be an array of numbers").

New: Validates and accumulates in one pass. Fails at the first non-numeric element. This is actually better — earlier failure with the same error message.

No regression.

8. Test for base64DecodeBytes — round-trip uses std.base64 encoding

local encoded = std.base64([255, 128, 1, 0]);
local decoded = std.base64DecodeBytes(encoded);
std.assertEqual(decoded, [255, 128, 1, 0])

This tests that base64([255, 128, 1, 0]) encodes correctly and base64DecodeBytes decodes back to [255, 128, 1, 0]. This is a good round-trip test for the unsigned byte fix. Correct and useful test.

[He-Pin]

Closing: the base64DecodeBytes unsigned byte fix has been extracted to #705 as a clean bugfix PR. The while-loop performance optimizations will be resubmitted in a consolidated PR with native benchmarks.