From c1cfb5ca8844d1b6019a18c71d24f1199fc44e67 Mon Sep 17 00:00:00 2001 From: He-Pin Date: Wed, 29 Jul 2026 15:25:50 +0800 Subject: [PATCH 1/3] fix: cache importstr and importbin values Motivation: Repeated raw imports called ResolvedFile readers and rebuilt Jsonnet values on every visit. Large importstr inputs could be read from disk repeatedly, while importbin eagerly allocated one Eval entry per byte. This also failed to keep a filename-stable raw-value snapshot for an evaluator. Modification: Cache importstr and importbin values independently by resolved path for the lifetime of an Evaluator. Clone importer-owned binary data before storing it in the compact ByteArr representation. Add snapshot, aliasing, cross-kind cache tests, and controlled JMH workloads. Result: Stable importstr output remains identical across sjsonnet, go-jsonnet, and jrsonnet; direct importbin output remains identical across sjsonnet and go-jsonnet. Repeated 256 KiB importbin x300 improves from 1062.890 to 0.199 ms/op, and repeated >1 MiB importstr x200 improves from 25.394 to 0.232 ms/op in the local JMH runs. Caches remain evaluator-local, so independent Interpreter instances do not share raw imported values. References: - https://github.com/databricks/sjsonnet/pull/1094 - https://jsonnet.org/ref/spec.html --- .../sjsonnet/bench/RawImportBenchmark.scala | 81 +++++++++++++++++++ sjsonnet/src/sjsonnet/Evaluator.scala | 23 +++--- .../EvaluatorRawImportCacheTests.scala | 73 +++++++++++++++++ 3 files changed, 163 insertions(+), 14 deletions(-) create mode 100644 bench/src/sjsonnet/bench/RawImportBenchmark.scala create mode 100644 sjsonnet/test/src/sjsonnet/EvaluatorRawImportCacheTests.scala diff --git a/bench/src/sjsonnet/bench/RawImportBenchmark.scala b/bench/src/sjsonnet/bench/RawImportBenchmark.scala new file mode 100644 index 00000000..9b511378 --- /dev/null +++ b/bench/src/sjsonnet/bench/RawImportBenchmark.scala @@ -0,0 +1,81 @@ +package sjsonnet.bench + +import org.openjdk.jmh.annotations.* +import org.openjdk.jmh.infra.Blackhole +import sjsonnet.* + +import java.util.concurrent.TimeUnit + +@BenchmarkMode(Array(Mode.AverageTime)) +@Fork(1) +@Warmup(iterations = 5) +@Measurement(iterations = 10) +@OutputTimeUnit(TimeUnit.MILLISECONDS) +@State(Scope.Benchmark) +class RawImportBenchmark { + private var root: os.Path = _ + private var importBinProgram: String = _ + private var importStrProgram: String = _ + private var repeatedSmallImportStrProgram: String = _ + private var singleImportBinProgram: String = _ + private var singleImportStrProgram: String = _ + + @Setup(Level.Trial) + def setup(): Unit = { + root = os.temp.dir(prefix = "sjsonnet-raw-import-benchmark-") + os.write.over(root / "payload.bin", Array.tabulate[Byte](256 * 1024)(_.toByte)) + os.write.over(root / "payload.txt", "x" * (1024 * 1024 + 1)) + os.write.over(root / "small.txt", "small payload") + importBinProgram = """std.sum(std.makeArray( + | 300, + | function(_) std.length(importbin "payload.bin") + |))""".stripMargin + importStrProgram = """std.sum(std.makeArray( + | 200, + | function(_) std.length(importstr "payload.txt") + |))""".stripMargin + repeatedSmallImportStrProgram = """std.sum(std.makeArray( + | 300, + | function(_) std.length(importstr "small.txt") + |))""".stripMargin + singleImportBinProgram = """std.length(importbin "payload.bin")""" + singleImportStrProgram = """std.length(importstr "payload.txt")""" + } + + @TearDown(Level.Trial) + def teardown(): Unit = os.remove.all(root) + + private def evaluate(program: String): ujson.Value = { + val interpreter = new Interpreter( + Map.empty, + Map.empty, + OsPath(root), + new SjsonnetMainBase.SimpleImporter(Seq.empty), + parseCache = new DefaultParseCache + ) + interpreter.interpret(program, OsPath(root / "main.jsonnet")) match { + case Right(value) => value + case Left(error) => throw new RuntimeException(error) + } + } + + @Benchmark + def repeatedImportBin(blackhole: Blackhole): Unit = + blackhole.consume(evaluate(importBinProgram)) + + @Benchmark + def repeatedImportStr(blackhole: Blackhole): Unit = + blackhole.consume(evaluate(importStrProgram)) + + @Benchmark + def repeatedSmallImportStr(blackhole: Blackhole): Unit = + blackhole.consume(evaluate(repeatedSmallImportStrProgram)) + + @Benchmark + def singleImportBin(blackhole: Blackhole): Unit = + blackhole.consume(evaluate(singleImportBinProgram)) + + @Benchmark + def singleImportStr(blackhole: Blackhole): Unit = + blackhole.consume(evaluate(singleImportStrProgram)) +} diff --git a/sjsonnet/src/sjsonnet/Evaluator.scala b/sjsonnet/src/sjsonnet/Evaluator.scala index 5f99b43b..92a2c59b 100644 --- a/sjsonnet/src/sjsonnet/Evaluator.scala +++ b/sjsonnet/src/sjsonnet/Evaluator.scala @@ -65,6 +65,8 @@ class Evaluator( def materialize(v: Val): Value = Materializer.apply(v) val cachedImports: collection.mutable.HashMap[Path, Val] = collection.mutable.HashMap.empty[Path, Val] + private val cachedImportStrings = collection.mutable.HashMap.empty[Path, Val.Str] + private val cachedImportBinaries = collection.mutable.HashMap.empty[Path, Val.Arr] // Hot path: top 7 types cover 96.1% of all visitExpr calls across benchmarks. // Order matches the empirically-measured frequency for C1 monomorphic dispatch parity. @@ -1266,23 +1268,16 @@ class Evaluator( } def visitImportStr(e: ImportStr): Val.Str = { - Val.Str( - e.pos, - importer.resolveAndReadOrFail(e.value, e.pos, binaryData = false)._2.readString() - ) + val (path, file) = importer.resolveAndReadOrFail(e.value, e.pos, binaryData = false) + cachedImportStrings.getOrElseUpdate(path, Val.Str(e.pos, file.readString())) } def visitImportBin(e: ImportBin): Val.Arr = { - val rawBytes = - importer.resolveAndReadOrFail(e.value, e.pos, binaryData = true)._2.readRawBytes() - val len = rawBytes.length - val arr = new Array[Eval](len) - var i = 0 - while (i < len) { - arr(i) = Val.cachedNum(e.pos, (rawBytes(i) & 0xff).toDouble) - i += 1 - } - Val.Arr(e.pos, arr) + val (path, file) = importer.resolveAndReadOrFail(e.value, e.pos, binaryData = true) + cachedImportBinaries.getOrElseUpdate( + path, + Val.Arr.fromBytes(e.pos, file.readRawBytes().clone()) + ) } def visitImport(e: Import): Val = { diff --git a/sjsonnet/test/src/sjsonnet/EvaluatorRawImportCacheTests.scala b/sjsonnet/test/src/sjsonnet/EvaluatorRawImportCacheTests.scala new file mode 100644 index 00000000..9b561407 --- /dev/null +++ b/sjsonnet/test/src/sjsonnet/EvaluatorRawImportCacheTests.scala @@ -0,0 +1,73 @@ +package sjsonnet + +import utest._ + +object EvaluatorRawImportCacheTests extends TestSuite { + def tests: Tests = Tests { + test("importstr and importbin values are cached independently by path") { + var stringReads = 0 + var binaryReads = 0 + val binaryContent = Array[Byte](1) + + val importer = new Importer { + def resolve(docBase: Path, importName: String): Option[Path] = + Some(DummyPath(importName)) + + def read(path: Path, binaryData: Boolean): Option[ResolvedFile] = + if (binaryData) { + Some(new ResolvedFile { + def getParserInput(): fastparse.ParserInput = + throw new NotImplementedError("not used by importbin") + def readString(): String = + throw new NotImplementedError("not used by importbin") + def contentHash(): String = "binary" + def readRawBytes(): Array[Byte] = { + binaryReads += 1 + binaryContent + } + }) + } else { + Some(new ResolvedFile { + def getParserInput(): fastparse.ParserInput = + throw new NotImplementedError("not used by importstr") + def readString(): String = { + stringReads += 1 + s"text-$stringReads" + } + def contentHash(): String = "string" + def readRawBytes(): Array[Byte] = + throw new NotImplementedError("not used by importstr") + }) + } + } + + val interpreter = new Interpreter( + Map.empty, + Map.empty, + DummyPath("root"), + importer, + parseCache = new DefaultParseCache + ) + val result = interpreter.interpret( + """[ + | importstr "same", + | importstr "same", + | importbin "same", + | importbin "same", + |]""".stripMargin, + DummyPath("root", "main.jsonnet") + ) + + result ==> Right(ujson.Arr("text-1", "text-1", ujson.Arr(1), ujson.Arr(1))) + stringReads ==> 1 + binaryReads ==> 1 + + binaryContent(0) = 9 + interpreter.interpret( + """importbin "same"""", + DummyPath("root", "second.jsonnet") + ) ==> Right(ujson.Arr(1)) + binaryReads ==> 1 + } + } +} From 3de68714da284d0619a870b03a15dd4c006053c2 Mon Sep 17 00:00:00 2001 From: He-Pin Date: Wed, 29 Jul 2026 17:43:27 +0800 Subject: [PATCH 2/3] docs: add immutability contract to ByteArr.rawBytes Motivation: The import cache shares ByteArr instances across all import sites. rawBytes exposes the internal backing array by reference, so any mutation would corrupt the cached value for all subsequent imports. Modification: Add CONTRACT comments to both ByteArr and LinearModByteArr rawBytes overrides documenting that callers must not mutate the returned array. Result: Future contributors are warned about the shared-state invariant. --- sjsonnet/src/sjsonnet/Val.scala | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/sjsonnet/src/sjsonnet/Val.scala b/sjsonnet/src/sjsonnet/Val.scala index e7b39ac5..a101b89a 100644 --- a/sjsonnet/src/sjsonnet/Val.scala +++ b/sjsonnet/src/sjsonnet/Val.scala @@ -1530,7 +1530,8 @@ object Val { // After materialization arr becomes non-null; delegate to parent Arr logic. @inline private def isMaterialized: Boolean = arr ne null - /** Raw byte backing data for zero-copy extraction (e.g. base64 encode). Always non-null. */ + /** Raw byte backing data for zero-copy extraction (e.g. base64 encode). Always non-null. + * CONTRACT: callers MUST NOT mutate the returned array — it may be shared via import cache. */ override def rawBytes: Array[Byte] = byteData override def value(i: Int): Val = { @@ -1624,6 +1625,7 @@ object Val { } } + /** CONTRACT: callers MUST NOT mutate the returned array. */ override def rawBytes: Array[Byte] = { val current = byteData if (current != null) current From 3ceea1ef6946a850a7886faaf075e236fba4842b Mon Sep 17 00:00:00 2001 From: He-Pin Date: Wed, 29 Jul 2026 18:25:05 +0800 Subject: [PATCH 3/3] style: fix scalafmt formatting in Val.scala Motivation: CI checkFormat failed on all platforms due to scaladoc comment style. Modification: Reformat multi-line scaladoc to match scalafmt 3.11.5 rules. Result: checkFormat passes on JVM/JS/Wasm/Native. --- sjsonnet/src/sjsonnet/Val.scala | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/sjsonnet/src/sjsonnet/Val.scala b/sjsonnet/src/sjsonnet/Val.scala index a101b89a..039a441a 100644 --- a/sjsonnet/src/sjsonnet/Val.scala +++ b/sjsonnet/src/sjsonnet/Val.scala @@ -1530,8 +1530,10 @@ object Val { // After materialization arr becomes non-null; delegate to parent Arr logic. @inline private def isMaterialized: Boolean = arr ne null - /** Raw byte backing data for zero-copy extraction (e.g. base64 encode). Always non-null. - * CONTRACT: callers MUST NOT mutate the returned array — it may be shared via import cache. */ + /** + * Raw byte backing data for zero-copy extraction (e.g. base64 encode). Always non-null. + * CONTRACT: callers MUST NOT mutate the returned array — it may be shared via import cache. + */ override def rawBytes: Array[Byte] = byteData override def value(i: Int): Val = {