Skip to content

Commit

Permalink
Make Jsonnet std lib non-global (#166)
Browse files Browse the repository at this point in the history
This serves two purposes:

1. We think we're seeing race conditions where the lazily-evaluated mutable state in the std lib `Val.Obj` is causing livelocks when evaluating Jsonnet in a multithreaded environment.

2. This will allow whoever instantiates `sjsonnet.Interpreter` to customize the `std.*`, allowing people to inject their own functions. Useful if google/jsonnet has some new std lib functions that haven't been added to sjsonnet yet, or if you have some domain-specific functions you want to expose in your config
  • Loading branch information
lihaoyi-databricks committed May 30, 2023
1 parent e405110 commit ded8e14
Show file tree
Hide file tree
Showing 5 changed files with 8 additions and 7 deletions.
4 changes: 2 additions & 2 deletions bench/src/main/scala/sjsonnet/OptimizerBenchmark.scala
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ class OptimizerBenchmark {
}
this.ev = ev
val static = inputs.map {
case (expr, fs) => ((new StaticOptimizer(ev)).optimize(expr), fs)
case (expr, fs) => ((new StaticOptimizer(ev, new Std().Std)).optimize(expr), fs)
}
val countBefore, countStatic = new Counter
inputs.foreach(t => assert(countBefore.transform(t._1) eq t._1))
Expand All @@ -45,7 +45,7 @@ class OptimizerBenchmark {
@Benchmark
def main(bh: Blackhole): Unit = {
bh.consume(inputs.foreach { case (expr, fs) =>
bh.consume((new StaticOptimizer(ev)).optimize(expr))
bh.consume((new StaticOptimizer(ev, new Std().Std)).optimize(expr))
})
}

Expand Down
2 changes: 1 addition & 1 deletion bench/src/main/scala/sjsonnet/ProfilingEvaluator.scala
Original file line number Diff line number Diff line change
Expand Up @@ -154,7 +154,7 @@ class ProfilingEvaluator(resolver: CachedResolver,

def builtins(): Seq[BuiltinBox] = {
val names = new util.IdentityHashMap[Val.Func, String]()
Std.functions.foreachEntry((n, f) => names.put(f, n))
new Std().functions.foreachEntry((n, f) => names.put(f, n))
val m = new mutable.HashMap[String, BuiltinBox]
def add(b: ExprBox, func: Val.Builtin): Unit = {
val n = names.getOrDefault(func, func.getClass.getName)
Expand Down
3 changes: 2 additions & 1 deletion sjsonnet/src/sjsonnet/Interpreter.scala
Original file line number Diff line number Diff line change
Expand Up @@ -19,11 +19,12 @@ class Interpreter(extVars: Map[String, ujson.Value],
settings: Settings = Settings.default,
storePos: Position => Unit = null,
warnLogger: (String => Unit) = null,
std: Val.Obj = new Std().Std
) { self =>

val resolver = new CachedResolver(importer, parseCache) {
override def process(expr: Expr, fs: FileScope): Either[Error, (Expr, FileScope)] =
handleException(new StaticOptimizer(evaluator).optimize(expr), fs)
handleException(new StaticOptimizer(evaluator, std).optimize(expr), fs)
}

private def warn(e: Error): Unit = warnLogger("[warning] " + formatError(e))
Expand Down
4 changes: 2 additions & 2 deletions sjsonnet/src/sjsonnet/StaticOptimizer.scala
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ import ScopedExprTransform._

/** StaticOptimizer performs necessary transformations for the evaluator (assigning ValScope
* indices) plus additional optimizations (post-order) and static checking (pre-order). */
class StaticOptimizer(ev: EvalScope) extends ScopedExprTransform {
class StaticOptimizer(ev: EvalScope, std: Val.Obj) extends ScopedExprTransform {
def optimize(e: Expr): Expr = transform(e)

def failOrWarn(msg: String, expr: Expr): Expr = {
Expand Down Expand Up @@ -38,7 +38,7 @@ class StaticOptimizer(ev: EvalScope) extends ScopedExprTransform {
scope.get(name) match {
case ScopedVal(v: Val with Expr, _, _) => v
case ScopedVal(_, _, idx) => ValidId(pos, name, idx)
case null if name == "std" => Std.Std
case null if name == "std" => std
case _ => failOrWarn("Unknown variable: "+name, e)
}

Expand Down
2 changes: 1 addition & 1 deletion sjsonnet/src/sjsonnet/Std.scala
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ import scala.util.matching.Regex
* in Scala code. Uses `builtin` and other helpers to handle the common wrapper
* logic automatically
*/
object Std {
class Std {
private val dummyPos: Position = new Position(null, 0)
private val emptyLazyArray = new Array[Lazy](0)

Expand Down

0 comments on commit ded8e14

Please sign in to comment.