Skip to content

Commit

Permalink
Simplify and clarify the public interface to Speedy. (digital-asset#5881
Browse files Browse the repository at this point in the history 
)

* Simplify and clarify the public interface to Speedy.

- Remove `isFinal`. A client just uses `run()`.
- Remove `toSValue`. The value in available in `SResultFinalValue(v: SValue)`.
- A client never directly access the `.ctrl` (or `.returnValue`) components.
- A client may use `setExpressionToEvaluate(expr)` to evaluate a new expression on an existing machine.

changelog_begin
changelog_end

* remove while loop which executes just once

* avoid unnecessary mutation when running speedy
  • Loading branch information
@nickchapman-da
nickchapman-da authored May 7, 2020
1 parent 6642b1f commit 53b2479
Show file tree
Hide file tree
Showing 14 changed files with 84 additions and 106 deletions.
4 changes: 2 additions & 2 deletions ...er/scenario-service/server/src/main/scala/com/digitalasset/daml/lf/scenario/Context.scala
View file
Original file line number Diff line number Diff line change
Expand Up @@ -167,8 +167,8 @@ class Context(val contextId: Context.ContextId) {
Identifier(assert(PackageId.fromString(pkgId)), assert(QualifiedName.fromString(name))),
).map { machine =>
ScenarioRunner(machine).run() match {
case Right((diff @ _, steps @ _, ledger)) =>
(ledger, machine, Right(machine.toSValue))
case Right((diff @ _, steps @ _, ledger, value)) =>
(ledger, machine, Right(value))
case Left((err, ledger)) =>
(ledger, machine, Left(err))
}
Expand Down
5 changes: 3 additions & 2 deletions daml-lf/engine/src/main/scala/com/digitalasset/daml/lf/engine/Engine.scala
View file
Original file line number Diff line number Diff line change
Expand Up @@ -285,9 +285,10 @@ final class Engine {
machine: Machine,
time: Time.Timestamp
): Result[(Tx.Transaction, Tx.Metadata)] = {
while (!machine.isFinal) {
var finished: Boolean = false
while (!finished) {
machine.run() match {
case SResultFinalValue(_) => ()
case SResultFinalValue(_) => finished = true

case SResultError(err) =>
return ResultError(
Expand Down
16 changes: 5 additions & 11 deletions daml-lf/interpreter/src/main/scala/com/digitalasset/daml/lf/speedy/Speedy.scala
View file
Original file line number Diff line number Diff line change
Expand Up @@ -133,6 +133,11 @@ object Speedy {
case _ =>
}

def setExpressionToEvaluate(expr: SExpr): Unit = {
ctrl = expr
returnValue = null
}

/** Run a machine until we get a result: either a final-value or a request for data, with a callback */
def run(): SResult = {
// Note: We have an outer and inner while loop.
Expand Down Expand Up @@ -258,19 +263,8 @@ object Speedy {
throw DamlEArithmeticError(e.getMessage)
}
}

}

def toValue: V[V.ContractId] =
toSValue.toValue

def toSValue: SValue =
if (!isFinal) {
crash("toSValue: machine not final")
} else {
returnValue
}

def print(count: Int) = {
println(s"Step: $count")
if (returnValue != null) {
Expand Down
19 changes: 7 additions & 12 deletions daml-lf/interpreter/src/test/scala/com/digitalasset/daml/lf/speedy/InterpreterTest.scala
View file
Original file line number Diff line number Diff line change
Expand Up @@ -30,13 +30,10 @@ class InterpreterTest extends WordSpec with Matchers with TableDrivenPropertyChe
submissionTime = Time.Timestamp.now(),
transactionSeed = None,
)
while (!machine.isFinal) {
machine.run match {
case SResultFinalValue(_) => ()
case res => throw new RuntimeException(s"Got unexpected interpretation result $res")
}
machine.run() match {
case SResultFinalValue(v) => v
case res => throw new RuntimeException(s"Got unexpected interpretation result $res")
}
machine.toSValue
}

"evaluator behaves responsibly" should {
Expand Down Expand Up @@ -148,13 +145,11 @@ class InterpreterTest extends WordSpec with Matchers with TableDrivenPropertyChe
)
}
"interpret" in {
while (!machine.isFinal) {
machine.run match {
case SResultFinalValue(_) => ()
case res => throw new RuntimeException(s"Got unexpected interpretation result $res")
}
val value = machine.run() match {
case SResultFinalValue(v) => v
case res => throw new RuntimeException(s"Got unexpected interpretation result $res")
}
machine.toSValue match {
value match {
case SValue.SList(lst) =>
lst.length shouldBe 100000
val arr = lst.toImmArray
Expand Down
6 changes: 3 additions & 3 deletions daml-lf/interpreter/src/test/scala/com/digitalasset/daml/lf/speedy/SBuiltinTest.scala
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1459,13 +1459,13 @@ object SBuiltinTest {
)
final case class Goodbye(e: SError) extends RuntimeException("", null, false, false)
try {
while (!machine.isFinal) machine.run() match {
case SResultFinalValue(_) => ()
val value = machine.run() match {
case SResultFinalValue(v) => v
case SResultError(err) => throw Goodbye(err)
case res => throw new RuntimeException(s"Got unexpected interpretation result $res")
}

Right(machine.toSValue)
Right(value)
} catch { case Goodbye(err) => Left(err) }
}

Expand Down
7 changes: 3 additions & 4 deletions daml-lf/interpreter/src/test/scala/com/digitalasset/daml/lf/speedy/SpeedyTest.scala
View file
Original file line number Diff line number Diff line change
Expand Up @@ -260,13 +260,12 @@ object SpeedyTest {
)
final case class Goodbye(e: SError) extends RuntimeException("", null, false, false)
try {
while (!machine.isFinal) machine.run() match {
case SResultFinalValue(_) => ()
val value = machine.run() match {
case SResultFinalValue(v) => v
case SResultError(err) => throw Goodbye(err)
case res => throw new RuntimeException(s"Got unexpected interpretation result $res")
}

Right(machine.toSValue)
Right(value)
} catch {
case Goodbye(err) => Left(err)
}
Expand Down
13 changes: 6 additions & 7 deletions daml-lf/interpreter/src/test/scala/com/digitalasset/daml/lf/speedy/svalue/OrderingSpec.scala
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@ import com.daml.lf.speedy.SResult._
import com.daml.lf.speedy.SValue._
import com.daml.lf.speedy.SExpr.SEImportValue
import com.daml.lf.speedy.{SBuiltin, SExpr, SValue}
import com.daml.lf.testing.parser.Implicits._
import com.daml.lf.value.Value
import com.daml.lf.value.TypedValueGenerators.genAddend
import com.daml.lf.value.ValueGenerators.{absCoidV0Gen, comparableAbsCoidsGen}
Expand Down Expand Up @@ -475,17 +474,17 @@ class OrderingSpec
new util.ArrayList[X](as.asJava)

private val txSeed = crypto.Hash.hashPrivateKey("SBuiltinTest")
private def dummyMachine = Speedy.Machine fromExpr (
expr = e"NA:na ()",
private def initMachine(expr: SExpr) = Speedy.Machine fromSExpr (
sexpr = expr,
compiledPackages = PureCompiledPackages(Map.empty, Map.empty),
scenario = false,
Time.Timestamp.now(),
Some(txSeed),
InitialSeeding(Some(txSeed)),
Set.empty,
)

private def translatePrimValue(v: Value[Value.AbsoluteContractId]) = {
val machine = dummyMachine
machine.ctrl = SEImportValue(v)
val expr = SEImportValue(v)
val machine = initMachine(expr)
machine.run() match {
case SResultFinalValue(value) => value
case _ => throw new Error(s"error while translating value $v")
Expand Down
45 changes: 19 additions & 26 deletions daml-lf/repl/src/main/scala/com/digitalasset/daml/lf/repl/testing/Main.scala
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ import com.daml.lf.speedy.Pretty._
import com.daml.lf.speedy.SError._
import com.daml.lf.speedy.SResult._
import com.daml.lf.types.Ledger
import com.daml.lf.speedy.SExpr.{LfDefRef, SEValue}
import com.daml.lf.speedy.SExpr.LfDefRef
import com.daml.lf.validation.Validation
import com.daml.lf.testing.parser
import com.daml.lf.language.LanguageVersion
Expand Down Expand Up @@ -169,7 +169,7 @@ object Repl {
.newBuilder(PureCompiledPackages(packages).right.get, Time.Timestamp.MinValue, nextSeed())
.fold(err => sys.error(err.toString), identity)
def run(expr: Expr)
: (Speedy.Machine, Either[(SError, Ledger.Ledger), (Double, Int, Ledger.Ledger)]) =
: (Speedy.Machine, Either[(SError, Ledger.Ledger), (Double, Int, Ledger.Ledger, SValue)]) =
(build(expr), ScenarioRunner(build(expr)).run())
}

Expand Down Expand Up @@ -410,33 +410,26 @@ object Repl {
submissionTime = Time.Timestamp.now(),
transactionSeed = None
)
var count = 0
val startTime = System.nanoTime()
var errored = false
while (!machine.isFinal && !errored) {
machine.run match {
case SResultError(err) =>
println(prettyError(err, machine.ptx).render(128))
errored = true
case SResultFinalValue(_) => ()
case other =>
sys.error("unimplemented callback: " + other.toString)
}
count += 1
val valueOpt = machine.run match {
case SResultError(err) =>
println(prettyError(err, machine.ptx).render(128))
None
case SResultFinalValue(v) =>
Some(v)
case other =>
sys.error("unimplemented callback: " + other.toString)
}
val endTime = System.nanoTime()
val diff = (endTime - startTime) / 1000 / 1000
machine.print(count)
println(s"steps: $count")
machine.print(1)
println(s"time: ${diff}ms")
if (!errored) {
val result = machine.ctrl match {
case SEValue(sv) =>
prettyValue(true)(sv.toValue).render(128)
case x => x.toString
}
println("result:")
println(result)
valueOpt match {
case None => ()
case Some(value) =>
val result = prettyValue(true)(value.toValue).render(128)
println("result:")
println(result)
}
case Some(_) =>
println("Error: " + id + " not a value.")
Expand Down Expand Up @@ -472,7 +465,7 @@ object Repl {
case Left((err, ledger @ _)) =>
println(prettyError(err, machine.ptx).render(128))
(false, state)
case Right((diff @ _, steps @ _, ledger)) =>
case Right((diff @ _, steps @ _, ledger, value @ _)) =>
// NOTE(JM): cannot print this, output used in tests.
//println(s"done in ${diff.formatted("%.2f")}ms, ${steps} steps")
println(prettyLedger(ledger).render(128))
Expand Down Expand Up @@ -508,7 +501,7 @@ object Repl {
prettyLoc(machine.lastLocation).render(128) +
": " + prettyError(err, machine.ptx).render(128))
failures += 1
case Right((diff, steps, ledger @ _)) =>
case Right((diff, steps, ledger @ _, value @ _)) =>
successes += 1
totalTime += diff
totalSteps += steps
Expand Down
13 changes: 7 additions & 6 deletions .../scenario-interpreter/src/main/scala/com/digitalasset/daml/lf/speedy/ScenarioRunner.scala
View file
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ final case class ScenarioRunner(

import scala.util.{Try, Success, Failure}

def run(): Either[(SError, Ledger), (Double, Int, Ledger)] =
def run(): Either[(SError, Ledger), (Double, Int, Ledger, SValue)] =
Try(runUnsafe) match {
case Failure(SRunnerException(err)) =>
Left((err, ledger))
Expand All @@ -41,18 +41,19 @@ final case class ScenarioRunner(
Right(res)
}

private def runUnsafe(): (Double, Int, Ledger) = {
private def runUnsafe(): (Double, Int, Ledger, SValue) = {
// NOTE(JM): Written with an imperative loop and exceptions for speed
// and so that we don't need to worry about stack usage.
val startTime = System.nanoTime()
var steps = 0
while (!machine.isFinal) {
var finalValue: SValue = null
while (finalValue == null) {
//machine.print(steps)
steps += 1 // this counts the number of external `Need` interactions
val res: SResult = machine.run()
res match {
case SResultFinalValue(_) =>
()
case SResultFinalValue(v) =>
finalValue = v
case SResultError(err) =>
throw SRunnerException(err)

Expand Down Expand Up @@ -91,7 +92,7 @@ final case class ScenarioRunner(
}
val endTime = System.nanoTime()
val diff = (endTime - startTime) / 1000.0 / 1000.0
(diff, steps, ledger)
(diff, steps, ledger, finalValue)
}

private def crash(reason: String) =
Expand Down
2 changes: 1 addition & 1 deletion ...preter/src/perf/benches/scala/com/digitalasset/daml/lf/speedy/perf/CollectAuthority.scala
View file
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ class CollectAuthorityState {
val machine = buildMachine(expr)
ScenarioRunner(machine).run() match {
case Left((err, _)) => sys.error(prettyError(err, machine.ptx).render(80))
case Right((_, steps, _)) => steps
case Right((_, steps, _, _)) => steps
}
}
}
Expand Down
9 changes: 6 additions & 3 deletions ...nario-interpreter/src/test/scala/com/digitalasset/daml/lf/speedy/ScenarioRunnerTest.scala
View file
Original file line number Diff line number Diff line change
Expand Up @@ -23,9 +23,12 @@ class ScenarioRunnerTest extends AsyncWordSpec with Matchers with ScalaFutures {
transactionSeed = None
)
val sr = ScenarioRunner(m, _ + "-XXX")
sr.run()
m.ctrl shouldBe null
m.returnValue shouldBe SValue.SParty(Ref.Party.assertFromString("foo-bar-XXX"))
sr.run() match {
case Right((_, _, _, value)) =>
value shouldBe SValue.SParty(Ref.Party.assertFromString("foo-bar-XXX"))
case res =>
sys.error(s"unexpected result $res")
}
}
}

Expand Down
3 changes: 1 addition & 2 deletions daml-script/runner/src/main/scala/com/digitalasset/daml/lf/engine/script/Runner.scala
View file
Original file line number Diff line number Diff line change
Expand Up @@ -309,8 +309,7 @@ class Runner(
}

def run(expr: SExpr): Future[SValue] = {
machine.ctrl = expr
machine.returnValue = null
machine.setExpressionToEvaluate(expr)
stepToValue()
.fold(Future.failed, Future.successful)
.flatMap {
Expand Down
2 changes: 1 addition & 1 deletion ...ndbox/src/main/scala/com/digitalasset/platform/sandbox/stores/ledger/ScenarioLoader.scala
View file
Original file line number Diff line number Diff line change
Expand Up @@ -127,7 +127,7 @@ object ScenarioLoader {
ScenarioRunner(speedyMachine).run match {
case Left(e) =>
throw new RuntimeException(s"error running scenario $scenarioRef in scenario $e")
case Right((_, _, l)) => l
case Right((_, _, l, _)) => l
}
}

Expand Down
Loading

0 comments on commit 53b2479

Please sign in to comment.