Skip to content

Commit 4d1ce40

Browse files
Restore old Evaluator switch, and put the new Evaluator behind a flag (#347)
Before: ``` Result "com.databricks.jsonnet.bundle.test.InternalJsonnetBuilderBenchmark.main": 239.623 ±(99.9%) 2.487 ms/op [Average] (min, avg, max) = (238.165, 239.623, 242.976), stdev = 1.645 CI (99.9%): [237.136, 242.110] (assumes normal distribution) ``` After: ``` Result "com.databricks.jsonnet.bundle.test.InternalJsonnetBuilderBenchmark.main": 224.443 ±(99.9%) 1.335 ms/op [Average] (min, avg, max) = (223.504, 224.443, 226.269), stdev = 0.883 CI (99.9%): [223.108, 225.779] (assumes normal distribution) ``` Looks like there's still a small regression somewhere - will continue investigating Initial 3.3.5 commit ``` Result "com.databricks.jsonnet.bundle.test.InternalJsonnetBuilderBenchmark.main": 216.718 ±(99.9%) 2.125 ms/op [Average] (min, avg, max) = (214.841, 216.718, 218.835), stdev = 1.406 CI (99.9%): [214.593, 218.843] (assumes normal distribution) ```
1 parent aeb6b9a commit 4d1ce40

5 files changed

Lines changed: 323 additions & 254 deletions

File tree

sjsonnet/src/sjsonnet/Evaluator.scala

Lines changed: 130 additions & 67 deletions
Original file line numberDiff line numberDiff line change
@@ -30,64 +30,58 @@ class Evaluator(resolver: CachedResolver,
3030
var tailstrict: Boolean = false
3131

3232
override def visitExpr(e: Expr)(implicit scope: ValScope): Val = try {
33-
(e._tag: @switch) match {
34-
case ExprTags.ValidId => visitValidId(e.asInstanceOf[ValidId])
35-
case ExprTags.BinaryOp => visitBinaryOp(e.asInstanceOf[BinaryOp])
36-
case ExprTags.Select => visitSelect(e.asInstanceOf[Select])
37-
case ExprTags.`Val.Func` => e.asInstanceOf[Val.Func]
38-
case ExprTags.`Val.Literal` => e.asInstanceOf[Val.Literal]
39-
case ExprTags.ApplyBuiltin0 => visitApplyBuiltin0(e.asInstanceOf[ApplyBuiltin0])
40-
case ExprTags.ApplyBuiltin1 => visitApplyBuiltin1(e.asInstanceOf[ApplyBuiltin1])
41-
case ExprTags.ApplyBuiltin2 => visitApplyBuiltin2(e.asInstanceOf[ApplyBuiltin2])
42-
case ExprTags.ApplyBuiltin3 => visitApplyBuiltin3(e.asInstanceOf[ApplyBuiltin3])
43-
case ExprTags.ApplyBuiltin4 => visitApplyBuiltin4(e.asInstanceOf[ApplyBuiltin4])
44-
case ExprTags.And => visitAnd(e.asInstanceOf[And])
45-
case ExprTags.Or => visitOr(e.asInstanceOf[Or])
46-
case ExprTags.UnaryOp => visitUnaryOp(e.asInstanceOf[UnaryOp])
47-
case ExprTags.Apply1 => visitApply1(e.asInstanceOf[Apply1])
48-
case ExprTags.Lookup => visitLookup(e.asInstanceOf[Lookup])
49-
case ExprTags.Function =>
50-
val f = e.asInstanceOf[Function]
51-
visitMethod(f.body, f.params, f.pos)
52-
case ExprTags.LocalExpr => visitLocalExpr(e.asInstanceOf[LocalExpr])
53-
case ExprTags.Apply => visitApply(e.asInstanceOf[Apply])
54-
case ExprTags.IfElse => visitIfElse(e.asInstanceOf[IfElse])
55-
case ExprTags.Apply3 => visitApply3(e.asInstanceOf[Apply3])
56-
case ExprTags.`ObjBody.MemberList` =>
57-
val oml = e.asInstanceOf[ObjBody.MemberList]
58-
visitMemberList(oml.pos, oml, null)
59-
case ExprTags.Apply2 => visitApply2(e.asInstanceOf[Apply2])
60-
case ExprTags.AssertExpr => visitAssert(e.asInstanceOf[AssertExpr])
61-
case ExprTags.ApplyBuiltin => visitApplyBuiltin(e.asInstanceOf[ApplyBuiltin])
62-
case ExprTags.Comp => visitComp(e.asInstanceOf[Comp])
63-
case ExprTags.Arr => visitArr(e.asInstanceOf[Arr])
64-
case ExprTags.SelectSuper => visitSelectSuper(e.asInstanceOf[SelectSuper])
65-
case ExprTags.LookupSuper => visitLookupSuper(e.asInstanceOf[LookupSuper])
66-
case ExprTags.InSuper => visitInSuper(e.asInstanceOf[InSuper])
67-
case ExprTags.ObjExtend => visitObjExtend(e.asInstanceOf[ObjExtend])
68-
case ExprTags.`ObjBody.ObjComp` => visitObjComp(e.asInstanceOf[ObjBody.ObjComp], null)
69-
case ExprTags.Slice => visitSlice(e.asInstanceOf[Slice])
70-
case ExprTags.Import => visitImport(e.asInstanceOf[Import])
71-
case ExprTags.Apply0 => visitApply0(e.asInstanceOf[Apply0])
72-
case ExprTags.ImportStr => visitImportStr(e.asInstanceOf[ImportStr])
73-
case ExprTags.ImportBin => visitImportBin(e.asInstanceOf[ImportBin])
74-
case ExprTags.Error => visitError(e.asInstanceOf[Expr.Error])
75-
case _ => visitInvalid(e)
33+
e match {
34+
case e: ValidId => visitValidId(e)
35+
case e: BinaryOp => visitBinaryOp(e)
36+
case e: Select => visitSelect(e)
37+
case e: Val => e
38+
case e: ApplyBuiltin0 => visitApplyBuiltin0(e)
39+
case e: ApplyBuiltin1 => visitApplyBuiltin1(e)
40+
case e: ApplyBuiltin2 => visitApplyBuiltin2(e)
41+
case e: ApplyBuiltin3 => visitApplyBuiltin3(e)
42+
case e: ApplyBuiltin4 => visitApplyBuiltin4(e)
43+
case e: And => visitAnd(e)
44+
case e: Or => visitOr(e)
45+
case e: UnaryOp => visitUnaryOp(e)
46+
case e: Apply1 => visitApply1(e)
47+
case e: Lookup => visitLookup(e)
48+
case e: Function => visitMethod(e.body, e.params, e.pos)
49+
case e: LocalExpr => visitLocalExpr(e)
50+
case e: Apply => visitApply(e)
51+
case e: IfElse => visitIfElse(e)
52+
case e: Apply3 => visitApply3(e)
53+
case e: ObjBody.MemberList => visitMemberList(e.pos, e, null)
54+
case e: Apply2 => visitApply2(e)
55+
case e: AssertExpr => visitAssert(e)
56+
case e: ApplyBuiltin => visitApplyBuiltin(e)
57+
case e: Comp => visitComp(e)
58+
case e: Arr => visitArr(e)
59+
case e: SelectSuper => visitSelectSuper(e)
60+
case e: LookupSuper => visitLookupSuper(e)
61+
case e: InSuper => visitInSuper(e)
62+
case e: ObjExtend => visitObjExtend(e)
63+
case e: ObjBody.ObjComp => visitObjComp(e, null)
64+
case e: Slice => visitSlice(e)
65+
case e: Import => visitImport(e)
66+
case e: Apply0 => visitApply0(e)
67+
case e: ImportStr => visitImportStr(e)
68+
case e: ImportBin => visitImportBin(e)
69+
case e: Expr.Error => visitError(e)
70+
case e => visitInvalid(e)
7671
}
7772
} catch {
7873
Error.withStackFrame(e)
7974
}
8075
// This is only needed for --no-static-errors, otherwise these expression types do not make it past the optimizer
81-
private def visitInvalid(e: Expr): Nothing = (e._tag : @switch) match {
82-
case ExprTags.Id =>
83-
val id = e.asInstanceOf[Id]
84-
Error.fail("Unknown variable: " + id.name, id.pos)
85-
case ExprTags.Self =>
86-
Error.fail("Can't use self outside of an object", e.pos)
87-
case ExprTags.`$` =>
88-
Error.fail("Can't use $ outside of an object", e.pos)
89-
case ExprTags.Super =>
90-
Error.fail("Can't use super outside of an object", e.pos)
76+
def visitInvalid(e: Expr): Nothing = e match {
77+
case Id(pos, name) =>
78+
Error.fail("Unknown variable: " + name, pos)
79+
case Self(pos) =>
80+
Error.fail("Can't use self outside of an object", pos)
81+
case $(pos) =>
82+
Error.fail("Can't use $ outside of an object", pos)
83+
case Super(pos) =>
84+
Error.fail("Can't use super outside of an object", pos)
9185
}
9286

9387
def visitAsLazy(e: Expr)(implicit scope: ValScope): Lazy = e match {
@@ -162,7 +156,7 @@ class Evaluator(resolver: CachedResolver,
162156
Error.fail(materializeError(visitExpr(e.value)), e.pos)
163157
}
164158

165-
private def materializeError(value: Val) = value match {
159+
protected def materializeError(value: Val) = value match {
166160
case Val.Str(_, s) => s
167161
case r => Materializer.stringify(r)
168162
}
@@ -193,7 +187,7 @@ class Evaluator(resolver: CachedResolver,
193187
}
194188
}
195189

196-
private def visitApply(e: Apply)(implicit scope: ValScope) = {
190+
protected def visitApply(e: Apply)(implicit scope: ValScope) = {
197191
val lhs = visitExpr(e.value)
198192

199193
if (tailstrict) {
@@ -215,7 +209,7 @@ class Evaluator(resolver: CachedResolver,
215209
}
216210
}
217211

218-
private def visitApply0(e: Apply0)(implicit scope: ValScope): Val = {
212+
protected def visitApply0(e: Apply0)(implicit scope: ValScope): Val = {
219213
val lhs = visitExpr(e.value)
220214
if (e.tailstrict) {
221215
tailstrict = true
@@ -227,7 +221,7 @@ class Evaluator(resolver: CachedResolver,
227221
}
228222
}
229223

230-
private def visitApply1(e: Apply1)(implicit scope: ValScope): Val = {
224+
protected def visitApply1(e: Apply1)(implicit scope: ValScope): Val = {
231225
val lhs = visitExpr(e.value)
232226
if (tailstrict) {
233227
lhs.cast[Val.Func].apply1(visitExpr(e.a1), e.pos)
@@ -242,7 +236,7 @@ class Evaluator(resolver: CachedResolver,
242236
}
243237
}
244238

245-
private def visitApply2(e: Apply2)(implicit scope: ValScope): Val = {
239+
protected def visitApply2(e: Apply2)(implicit scope: ValScope): Val = {
246240
val lhs = visitExpr(e.value)
247241
if (tailstrict) {
248242
lhs.cast[Val.Func].apply2(visitExpr(e.a1), visitExpr(e.a2), e.pos)
@@ -258,7 +252,7 @@ class Evaluator(resolver: CachedResolver,
258252
}
259253
}
260254

261-
private def visitApply3(e: Apply3)(implicit scope: ValScope): Val = {
255+
protected def visitApply3(e: Apply3)(implicit scope: ValScope): Val = {
262256
val lhs = visitExpr(e.value)
263257
if (tailstrict) {
264258
lhs.cast[Val.Func].apply3(visitExpr(e.a1), visitExpr(e.a2), visitExpr(e.a3), e.pos)
@@ -275,7 +269,7 @@ class Evaluator(resolver: CachedResolver,
275269
}
276270
}
277271

278-
private def visitApplyBuiltin0(e: ApplyBuiltin0)(implicit scope: ValScope) = {
272+
protected def visitApplyBuiltin0(e: ApplyBuiltin0)(implicit scope: ValScope) = {
279273
if (tailstrict) {
280274
e.func.evalRhs(this, e.pos)
281275
} else if (e.tailstrict) {
@@ -288,7 +282,7 @@ class Evaluator(resolver: CachedResolver,
288282
}
289283
}
290284

291-
private def visitApplyBuiltin1(e: ApplyBuiltin1)(implicit scope: ValScope) = {
285+
protected def visitApplyBuiltin1(e: ApplyBuiltin1)(implicit scope: ValScope) = {
292286
if (tailstrict) {
293287
e.func.evalRhs(visitExpr(e.a1), this, e.pos)
294288
} else if (e.tailstrict) {
@@ -301,7 +295,7 @@ class Evaluator(resolver: CachedResolver,
301295
}
302296
}
303297

304-
private def visitApplyBuiltin2(e: ApplyBuiltin2)(implicit scope: ValScope) = {
298+
protected def visitApplyBuiltin2(e: ApplyBuiltin2)(implicit scope: ValScope) = {
305299
if (tailstrict) {
306300
e.func.evalRhs(visitExpr(e.a1), visitExpr(e.a2), this, e.pos)
307301
} else if (e.tailstrict) {
@@ -314,7 +308,7 @@ class Evaluator(resolver: CachedResolver,
314308
}
315309
}
316310

317-
private def visitApplyBuiltin3(e: ApplyBuiltin3)(implicit scope: ValScope) = {
311+
protected def visitApplyBuiltin3(e: ApplyBuiltin3)(implicit scope: ValScope) = {
318312
if (tailstrict) {
319313
e.func.evalRhs(visitExpr(e.a1), visitExpr(e.a2), visitExpr(e.a3), this, e.pos)
320314
} else if (e.tailstrict) {
@@ -327,7 +321,7 @@ class Evaluator(resolver: CachedResolver,
327321
}
328322
}
329323

330-
private def visitApplyBuiltin4(e: ApplyBuiltin4)(implicit scope: ValScope) = {
324+
protected def visitApplyBuiltin4(e: ApplyBuiltin4)(implicit scope: ValScope) = {
331325
if (tailstrict) {
332326
e.func.evalRhs(visitExpr(e.a1), visitExpr(e.a2), visitExpr(e.a3), visitExpr(e.a4), this, e.pos)
333327
} else if (e.tailstrict) {
@@ -340,7 +334,7 @@ class Evaluator(resolver: CachedResolver,
340334
}
341335
}
342336

343-
private def visitApplyBuiltin(e: ApplyBuiltin)(implicit scope: ValScope) = {
337+
protected def visitApplyBuiltin(e: ApplyBuiltin)(implicit scope: ValScope) = {
344338
val arr = new Array[Lazy](e.argExprs.length)
345339
var idx = 0
346340

@@ -381,7 +375,7 @@ class Evaluator(resolver: CachedResolver,
381375
visitExpr(e.returned)
382376
}
383377

384-
private def visitSlice(e: Slice)(implicit scope: ValScope): Val = {
378+
protected def visitSlice(e: Slice)(implicit scope: ValScope): Val = {
385379
def extractParam(e: Option[Expr], default:Int): Int = e match {
386380
case Some(expr) => visitExpr(expr) match {
387381
case _:Val.Null => default
@@ -803,7 +797,7 @@ class Evaluator(resolver: CachedResolver,
803797
}
804798

805799
@tailrec
806-
private final def visitComp(f: List[CompSpec], scopes: Array[ValScope]): Array[ValScope] = f match{
800+
final def visitComp(f: List[CompSpec], scopes: Array[ValScope]): Array[ValScope] = f match{
807801
case (spec @ ForSpec(_, name, expr)) :: rest =>
808802
visitComp(
809803
rest,
@@ -895,6 +889,75 @@ class Evaluator(resolver: CachedResolver,
895889
})
896890
}
897891

892+
class NewEvaluator(
893+
resolver: CachedResolver,
894+
extVars: String => Option[Expr],
895+
wd: Path,
896+
settings: Settings,
897+
warnLogger: Error => Unit = null) extends Evaluator(resolver, extVars, wd, settings, warnLogger) {
898+
899+
override def visitExpr(e: Expr)(implicit scope: ValScope): Val = try {
900+
(e._tag: @switch) match {
901+
case ExprTags.ValidId => visitValidId(e.asInstanceOf[ValidId])
902+
case ExprTags.BinaryOp => visitBinaryOp(e.asInstanceOf[BinaryOp])
903+
case ExprTags.Select => visitSelect(e.asInstanceOf[Select])
904+
case ExprTags.`Val.Func` => e.asInstanceOf[Val.Func]
905+
case ExprTags.`Val.Literal` => e.asInstanceOf[Val.Literal]
906+
case ExprTags.ApplyBuiltin0 => visitApplyBuiltin0(e.asInstanceOf[ApplyBuiltin0])
907+
case ExprTags.ApplyBuiltin1 => visitApplyBuiltin1(e.asInstanceOf[ApplyBuiltin1])
908+
case ExprTags.ApplyBuiltin2 => visitApplyBuiltin2(e.asInstanceOf[ApplyBuiltin2])
909+
case ExprTags.ApplyBuiltin3 => visitApplyBuiltin3(e.asInstanceOf[ApplyBuiltin3])
910+
case ExprTags.ApplyBuiltin4 => visitApplyBuiltin4(e.asInstanceOf[ApplyBuiltin4])
911+
case ExprTags.And => visitAnd(e.asInstanceOf[And])
912+
case ExprTags.Or => visitOr(e.asInstanceOf[Or])
913+
case ExprTags.UnaryOp => visitUnaryOp(e.asInstanceOf[UnaryOp])
914+
case ExprTags.Apply1 => visitApply1(e.asInstanceOf[Apply1])
915+
case ExprTags.Lookup => visitLookup(e.asInstanceOf[Lookup])
916+
case ExprTags.Function =>
917+
val f = e.asInstanceOf[Function]
918+
visitMethod(f.body, f.params, f.pos)
919+
case ExprTags.LocalExpr => visitLocalExpr(e.asInstanceOf[LocalExpr])
920+
case ExprTags.Apply => visitApply(e.asInstanceOf[Apply])
921+
case ExprTags.IfElse => visitIfElse(e.asInstanceOf[IfElse])
922+
case ExprTags.Apply3 => visitApply3(e.asInstanceOf[Apply3])
923+
case ExprTags.`ObjBody.MemberList` =>
924+
val oml = e.asInstanceOf[ObjBody.MemberList]
925+
visitMemberList(oml.pos, oml, null)
926+
case ExprTags.Apply2 => visitApply2(e.asInstanceOf[Apply2])
927+
case ExprTags.AssertExpr => visitAssert(e.asInstanceOf[AssertExpr])
928+
case ExprTags.ApplyBuiltin => visitApplyBuiltin(e.asInstanceOf[ApplyBuiltin])
929+
case ExprTags.Comp => visitComp(e.asInstanceOf[Comp])
930+
case ExprTags.Arr => visitArr(e.asInstanceOf[Arr])
931+
case ExprTags.SelectSuper => visitSelectSuper(e.asInstanceOf[SelectSuper])
932+
case ExprTags.LookupSuper => visitLookupSuper(e.asInstanceOf[LookupSuper])
933+
case ExprTags.InSuper => visitInSuper(e.asInstanceOf[InSuper])
934+
case ExprTags.ObjExtend => visitObjExtend(e.asInstanceOf[ObjExtend])
935+
case ExprTags.`ObjBody.ObjComp` => visitObjComp(e.asInstanceOf[ObjBody.ObjComp], null)
936+
case ExprTags.Slice => visitSlice(e.asInstanceOf[Slice])
937+
case ExprTags.Import => visitImport(e.asInstanceOf[Import])
938+
case ExprTags.Apply0 => visitApply0(e.asInstanceOf[Apply0])
939+
case ExprTags.ImportStr => visitImportStr(e.asInstanceOf[ImportStr])
940+
case ExprTags.ImportBin => visitImportBin(e.asInstanceOf[ImportBin])
941+
case ExprTags.Error => visitError(e.asInstanceOf[Expr.Error])
942+
case _ => visitInvalid(e)
943+
}
944+
} catch {
945+
Error.withStackFrame(e)
946+
}
947+
// This is only needed for --no-static-errors, otherwise these expression types do not make it past the optimizer
948+
override def visitInvalid(e: Expr): Nothing = (e._tag : @switch) match {
949+
case ExprTags.Id =>
950+
val id = e.asInstanceOf[Id]
951+
Error.fail("Unknown variable: " + id.name, id.pos)
952+
case ExprTags.Self =>
953+
Error.fail("Can't use self outside of an object", e.pos)
954+
case ExprTags.`$` =>
955+
Error.fail("Can't use $ outside of an object", e.pos)
956+
case ExprTags.Super =>
957+
Error.fail("Can't use super outside of an object", e.pos)
958+
}
959+
}
960+
898961
object Evaluator {
899962
val emptyStringArray = new Array[String](0)
900963
val emptyLazyArray = new Array[Lazy](0)

sjsonnet/src/sjsonnet/Interpreter.scala

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -69,7 +69,9 @@ class Interpreter(queryExtVar: String => Option[ExternalVariable[_]],
6969
private def warn(e: Error): Unit = warnLogger("[warning] " + formatError(e))
7070

7171
def createEvaluator(resolver: CachedResolver, extVars: String => Option[Expr], wd: Path,
72-
settings: Settings, warn: Error => Unit): Evaluator =
72+
settings: Settings, warn: Error => Unit): Evaluator = if (settings.useNewEvaluator)
73+
new NewEvaluator(resolver, extVars, wd, settings, warn)
74+
else
7375
new Evaluator(resolver, extVars, wd, settings, warn)
7476

7577
/**

sjsonnet/src/sjsonnet/Settings.scala

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ class Settings(
1111
val strictInheritedAssertions: Boolean = false,
1212
val strictSetOperations: Boolean = false,
1313
val throwErrorForInvalidSets: Boolean = false,
14+
val useNewEvaluator: Boolean = false,
1415
)
1516

1617
object Settings {

0 commit comments

Comments
 (0)