Skip to content

Commit 4e7f3c6

Browse files
Revert last changes that caused performance regressions (#351)
I don't know why the `Make evalutator a simple val` made a difference, but it did. We are back to the performance level of the scala3 migration (which is better than the 0.4 series) ``` # Run progress: 0.00% complete, ETA 00:02:00 # Fork: 1 of 1 Java HotSpot(TM) 64-Bit Server VM warning: -XX:ThreadPriorityPolicy=1 may require system level permission, e.g., being the root user. If the necessary permission is not possessed, changes to priority will be silently ignored. # Warmup Iteration 1: 318.899 ms/op # Warmup Iteration 2: 219.683 ms/op Iteration 1: 218.122 ms/op Iteration 2: 217.369 ms/op Iteration 3: 216.576 ms/op Iteration 4: 216.465 ms/op Iteration 5: 215.468 ms/op Iteration 6: 215.459 ms/op Iteration 7: 215.341 ms/op Iteration 8: 214.828 ms/op Iteration 9: 215.898 ms/op Iteration 10: 215.108 ms/op Result "com.databricks.jsonnet.bundle.test.InternalJsonnetBuilderBenchmark.main": 216.063 ±(99.9%) 1.600 ms/op [Average] (min, avg, max) = (214.828, 216.063, 218.122), stdev = 1.058 CI (99.9%): [214.464, 217.663] (assumes normal distribution) # Run complete. Total time: 00:02:02 REMEMBER: The numbers below are just data. To gain reusable insights, you need to follow up on why the numbers are the way they are. Use profilers (see -prof, -lprof), design factorial experiments, perform baseline and negative tests that provide experimental control, make sure the benchmarking environment is safe on JVM/OS/HW level, ask for reviews from the domain experts. Do not assume the numbers tell you what you want them to tell. Benchmark Mode Cnt Score Error Units InternalJsonnetBuilderBenchmark.main avgt 10 216.063 ± 1.600 ms/op ```
1 parent 4d1ce40 commit 4e7f3c6

5 files changed

Lines changed: 14 additions & 12 deletions

File tree

sjsonnet/src/sjsonnet/Interpreter.scala

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -91,7 +91,7 @@ class Interpreter(queryExtVar: String => Option[ExternalVariable[_]],
9191
varResolver.parse(wd / Util.wrapInLessThanGreaterThan(k), StaticResolvedFile(v))(evaluator).fold(throw _, _._1)
9292
}
9393

94-
val evaluator: Evaluator = createEvaluator(
94+
lazy val evaluator: Evaluator = createEvaluator(
9595
resolver,
9696
// parse extVars lazily, because they can refer to each other and be recursive
9797
k => queryExtVar(k).map(v => evaluateVar("ext", k, v)),
@@ -100,6 +100,8 @@ class Interpreter(queryExtVar: String => Option[ExternalVariable[_]],
100100
warn
101101
)
102102

103+
evaluator // force the lazy val
104+
103105
def formatError(e: Error): String = {
104106
val s = new StringWriter()
105107
val p = new PrintWriter(s)

sjsonnet/src/sjsonnet/StaticOptimizer.scala

Lines changed: 0 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -43,15 +43,6 @@ class StaticOptimizer(
4343
case b2 @ BinaryOp(pos, lhs: Val.Str, BinaryOp.OP_%, rhs) =>
4444
try ApplyBuiltin1(pos, new Format.PartialApplyFmt(lhs.value), rhs, tailstrict = false)
4545
catch { case _: Exception => b2 }
46-
case Or(_, Val.False(_), rhs) => transform(rhs)
47-
case Or(pos, Val.True(_), _) => Val.True(pos)
48-
case Or(pos, _, Val.True(_)) => Val.True(pos)
49-
case And(_, Val.True(_), rhs) => transform(rhs)
50-
case And(pos, Val.False(_), _) => Val.False(pos)
51-
case And(pos, _, Val.False(_)) => Val.False(pos)
52-
case UnaryOp(pos, UnaryOp.OP_!, Val.True(_)) => Val.False(pos)
53-
case UnaryOp(pos, UnaryOp.OP_!, Val.False(_)) => Val.True(pos)
54-
case UnaryOp(_, UnaryOp.OP_!, UnaryOp(_, UnaryOp.OP_!, expr)) => expr
5546

5647
case e @ Id(pos, name) =>
5748
scope.get(name) match {

sjsonnet/test/src-js/sjsonnet/FileTests.scala

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -77,6 +77,7 @@ object FileTests extends TestSuite {
7777
test("lazy_operator2") - checkFail(
7878
"""sjsonnet.Error: should happen
7979
| at [Error].((memory):1:9)
80+
| at [And].((memory):1:6)
8081
|""".stripMargin)
8182
test("merge") - check()
8283
test("null") - check()

sjsonnet/test/src-jvm/sjsonnet/FileTests.scala

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -51,7 +51,11 @@ object FileTests extends TestSuite{
5151
test("local") - check()
5252
test("lazy") - checkGolden()
5353
test("lazy_operator1") - checkGolden()
54-
test("lazy_operator2") - checkFail("sjsonnet.Error: should happen\n at [Error].(lazy_operator2.jsonnet:1:9)\n")
54+
test("lazy_operator2") - checkFail(
55+
"""sjsonnet.Error: should happen
56+
| at [Error].(lazy_operator2.jsonnet:1:9)
57+
| at [And].(lazy_operator2.jsonnet:1:6)
58+
|""".stripMargin)
5559
test("merge") - check()
5660
test("null") - check()
5761
test("object") - check()

sjsonnet/test/src-native/sjsonnet/FileTests.scala

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -50,7 +50,11 @@ object FileTests extends TestSuite{
5050
test("local") - check()
5151
test("lazy") - checkGolden()
5252
test("lazy_operator1") - checkGolden()
53-
test("lazy_operator2") - checkFail("sjsonnet.Error: should happen\n at [Error].(lazy_operator2.jsonnet:1:9)\n")
53+
test("lazy_operator2") - checkFail(
54+
"""sjsonnet.Error: should happen
55+
| at [Error].(lazy_operator2.jsonnet:1:9)
56+
| at [And].(lazy_operator2.jsonnet:1:6)
57+
|""".stripMargin)
5458
test("merge") - check()
5559
test("null") - check()
5660
test("object") - check()

0 commit comments

Comments
 (0)