[Builtins] No-legacy unlifting #4510
Conversation
This reverts commit 19656b6.
…to effectfully/builtins/defer-readKnown-until-full-saturation
|
/benchmark plutus-benchmark:validation |
|
Comparing benchmark results of 'plutus-benchmark:validation' on 'e59ade143' (base) and '5d3fb5abe' (PR)
|
|
-7.18% on average. @michaelpj ^ this is deferred unlifting done right and without support for immediate unlifting. The latter is going to slow down the former (don't know by how much at the moment). We may remedy that if instead of choosing the unlifting mode when constructing the
|
We'll definitely need to do this, since we need it to be configurable at runtime. |
| spendBudgetCek (BBuiltinApp fun) cost | ||
| let !(errOrRes, logs) = makeKnownRun (Just term) x | ||
| let !(errOrRes, logs) = runEmitter $ runExceptT getX | ||
| ?cekEmitter logs | ||
| case errOrRes of | ||
| Left err -> throwMakeKnownErrorWithCause err | ||
| Right res -> pure res | ||
| Left err -> throwReadKnownErrorWithCause $ term <$ err | ||
| Right x -> pure x |
There was a problem hiding this comment.
Note to self: this should be perfectly fine, but there's a subtlety here: the costing function kinda assumes that the arguments are of particular types, however an argument can be of the wrong type at runtime and the costing function will be applied to it and something will be computed. In the other PR an argument of the unexpected type is an evaluation error right away. It's probably fine to be a bit sloppy with how costing is managed in case of evaluation failure, but this needs to be documented and discussed.
There was a problem hiding this comment.
Oh no, it's all wrong. Ok, that's for tomorrow.
There was a problem hiding this comment.
Right, that is subtle: it fits into the world in which "real" builtins take typed constants and need to be costed appropriately (i.e. taking into account the cases where they're given ill-typed arguments). I think this all works out okay because the runtime-typechecking part is quick and happens first, but it is indeed another dark corner.
|
Closing in favor of #4516. |
Don't look here yet.