Engine: cleanup SEVal (#5859)

CHANGELOG_BEGIN
CHANGELOG_END

compiler/repl-service/server/src/main/scala/com/digitalasset/daml/lf/ReplServiceMain.scala

@@ -201,8 +201,7 @@ class ReplService(
 
     var scriptExpr: SExpr = SEVal(
       LfDefRef(
-        Identifier(homePackageId, QualifiedName(mod.name, DottedName.assertFromString("expr")))),
-      None)
+        Identifier(homePackageId, QualifiedName(mod.name, DottedName.assertFromString("expr")))))
     if (!results.isEmpty) {
       scriptExpr = SEApp(scriptExpr, results.map(SEValue(_)).toArray)
     }

daml-lf/interpreter/src/main/scala/com/digitalasset/daml/lf/speedy/Compiler.scala

@@ -209,7 +209,7 @@ private[lf] final case class Compiler(packages: PackageId PartialFunction Packag
   private def translate(expr0: Expr): SExpr =
     expr0 match {
       case EVar(name) => SEVar(env.lookUpExprVar(name))
-      case EVal(ref) => SEVal(LfDefRef(ref), None)
+      case EVal(ref) => SEVal(LfDefRef(ref))
       case EBuiltin(bf) =>
         bf match {
           case BFoldl => SEBuiltinRecursiveDefinition.FoldL
@@ -1236,7 +1236,7 @@ private[lf] final case class Compiler(packages: PackageId PartialFunction Packag
         case Some(actors) => SEApp(SEBuiltin(SBSome), Array(actors))
       }
       SEApp(
-        SEVal(ChoiceDefRef(tmplId, choiceId), None),
+        SEVal(ChoiceDefRef(tmplId, choiceId)),
         Array(SEValue.bool(byKey), actors, contractId, argument))
     }
 

daml-lf/interpreter/src/main/scala/com/digitalasset/daml/lf/speedy/Pretty.scala

@@ -457,7 +457,7 @@ object Pretty {
     def prettySExpr(index: Int)(e: SExpr): Doc =
       e match {
         case SEVar(i) => char('@') + str(index - i)
-        case SEVal(defId, _) =>
+        case SEVal(defId) =>
           str(defId)
         case SEValue(lit) =>
           lit match {

daml-lf/interpreter/src/main/scala/com/digitalasset/daml/lf/speedy/SExpr.scala

@@ -51,11 +51,27 @@ object SExpr {
     */
   final case class SEVal(
       ref: SDefinitionRef,
-      var cached: Option[(SValue, List[Location])],
   ) extends SExpr {
-    def execute(machine: Machine): Unit = {
+
+    // The variable `_cached` is used to cache the evaluation of the
+    // LF value defined by `ref` once it has been computed.  Hence we
+    // avoid both the lookup in the package definition HashMap and the
+    // full reevaluation of the body of the definition.
+    // Here we take advantage of the Java memory model
+    // (https://docs.oracle.com/javase/specs/jls/se8/html/jls-17.html#jls-17.7)
+    // that guarantees the write of `_cache` (in the method
+    // `setCached`) is done atomically.
+    // This is similar how hashcode evaluation is cached in String
+    // http://hg.openjdk.java.net/jdk8/jdk8/jdk/file/tip/src/share/classes/java/lang/String.java
+    private var _cached: Option[(SValue, List[Location])] = None
+
+    def cached: Option[(SValue, List[Location])] = _cached
+
+    def setCached(sValue: SValue, stack_trace: List[Location]): Unit =
+      _cached = Some((sValue, stack_trace))
+
+    def execute(machine: Machine): Unit =
       machine.lookupVal(this)
-    }
   }
 
   /** Reference to a builtin function */

daml-lf/interpreter/src/main/scala/com/digitalasset/daml/lf/speedy/Speedy.scala

@@ -685,9 +685,9 @@ object Speedy {
     * updates this solves the blow-up which would happen when a large record is
     * updated multiple times. */
   final case class KCacheVal(v: SEVal, stack_trace: List[Location]) extends Kont {
-    def execute(sv: SValue, machine: Machine) = {
+    def execute(sv: SValue, machine: Machine): Unit = {
       machine.pushStackTrace(stack_trace)
-      v.cached = Some((sv, stack_trace))
+      v.setCached(sv, stack_trace)
       machine.returnValue = sv
     }
   }

daml-script/runner/src/main/scala/com/digitalasset/daml/lf/engine/script/Runner.scala

@@ -114,7 +114,7 @@ object Script {
   def fromIdentifier(
       compiledPackages: CompiledPackages,
       scriptId: Identifier): Either[String, Script] = {
-    val scriptExpr = SEVal(LfDefRef(scriptId), None)
+    val scriptExpr = SEVal(LfDefRef(scriptId))
     val scriptTy = compiledPackages
       .getPackage(scriptId.packageId)
       .flatMap(_.lookupIdentifier(scriptId.qualifiedName).toOption) match {