From 989b32c0e62198e5a2752a43fb91be4c5acb5169 Mon Sep 17 00:00:00 2001 From: effectfully Date: Mon, 2 Sep 2024 19:39:52 +0200 Subject: [PATCH 1/3] [Builtins] Make unlifting lazy again --- plutus-core/plutus-core/src/PlutusCore/Builtin/Meaning.hs | 8 +------- 1 file changed, 1 insertion(+), 7 deletions(-) diff --git a/plutus-core/plutus-core/src/PlutusCore/Builtin/Meaning.hs b/plutus-core/plutus-core/src/PlutusCore/Builtin/Meaning.hs index f7260d203d7..ac7d1f07577 100644 --- a/plutus-core/plutus-core/src/PlutusCore/Builtin/Meaning.hs +++ b/plutus-core/plutus-core/src/PlutusCore/Builtin/Meaning.hs @@ -282,13 +282,7 @@ instance -- See Note [One-shotting runtime denotations]. -- Grow the builtin application within the received action and recurse on the result. toMonoF getBoth = BuiltinExpectArgument . oneShot $ \arg -> - -- Ironically computing the unlifted value strictly is the best way of doing deferred - -- unlifting. This means that while the resulting 'ReadKnownM' is only handled upon full - -- saturation and any evaluation failure is only registered when the whole builtin - -- application is evaluated, a Haskell exception will occur immediately. - -- It shouldn't matter though, because a builtin is not supposed to throw an - -- exception at any stage, that would be a bug regardless. - toMonoF @val @args @res $! do + oneShot (toMonoF @val @args @res) $ do (f, exF) <- getBoth x <- readKnown arg -- See Note [Strict application in runtime denotations]. From 25183286fb46804efd7a41c88a2f69f714ee774b Mon Sep 17 00:00:00 2001 From: effectfully Date: Fri, 29 Nov 2024 11:19:07 +0100 Subject: [PATCH 2/3] Docs --- .../src/PlutusCore/Builtin/Meaning.hs | 22 +++++++++++++++++++ 1 file changed, 22 insertions(+) diff --git a/plutus-core/plutus-core/src/PlutusCore/Builtin/Meaning.hs b/plutus-core/plutus-core/src/PlutusCore/Builtin/Meaning.hs index 1842630c1f8..db672a0652f 100644 --- a/plutus-core/plutus-core/src/PlutusCore/Builtin/Meaning.hs +++ b/plutus-core/plutus-core/src/PlutusCore/Builtin/Meaning.hs @@ -294,6 +294,28 @@ instance -- See Note [One-shotting runtime denotations]. -- Grow the builtin application within the received action and recurse on the result. toMonoF getBoth = BuiltinExpectArgument . oneShot $ \arg -> + -- The lazy application of 'toMonoF' ensures that unlifting of the argument will happen + -- upon full saturation and not before that. This is known as "operationally deferred + -- unlifting" (as opposed to "operationally immediate unlifting") or "call-by-name + -- unlifting" (as opposed to "call-by-value unlifting"). We do it this way to guarantee that + -- the cost of unlifting will be accounted for before unlifting is performed. If we did + -- unlifting eagerly here, this would make the node do work that is not accounted for until + -- full saturation is reached, which may never happen if the partial application is thrown + -- away. + -- + -- The disadvantage of this approach is that @addInteger 42@ will always unlift @42@ upon + -- full saturation even if this partial application is saved to a variable. But the way + -- costing calibration benchmarks are setup, we always evaluate a single application, so + -- the cost of unlifting is included in the cost of the builtin regardless of whether + -- there's caching of unlifting or not. Hence the user pays for unlifting anyway and we can + -- prioritize safety over performance here. + -- + -- 'oneShot' ensures that GHC doesn't attempt to pull stuff out of the builtin + -- implementation to create thunks. This would give us a "call-by-need" behavior, which may + -- sound enticing as it would give us both caching and operationally deferred unlifting, but + -- this comes at a cost of creating unnecessary thunks in the most common case where there's + -- no benefit from having caching as the builtin application is going to be computed only + -- once. So we choose the "call-by-name" behavior and 'oneShot' is what enables that. oneShot (toMonoF @val @args @res) $ do (f, exF) <- getBoth -- Force the argument that gets passed to the denotation. This seems to help performance From 98b994b94078dbe2b289348993554067f0f3d63b Mon Sep 17 00:00:00 2001 From: effectfully Date: Fri, 29 Nov 2024 13:31:46 +0100 Subject: [PATCH 3/3] Update plutus-core/plutus-core/src/PlutusCore/Builtin/Meaning.hs Co-authored-by: Kenneth MacKenzie --- plutus-core/plutus-core/src/PlutusCore/Builtin/Meaning.hs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/plutus-core/plutus-core/src/PlutusCore/Builtin/Meaning.hs b/plutus-core/plutus-core/src/PlutusCore/Builtin/Meaning.hs index db672a0652f..35e56b5e071 100644 --- a/plutus-core/plutus-core/src/PlutusCore/Builtin/Meaning.hs +++ b/plutus-core/plutus-core/src/PlutusCore/Builtin/Meaning.hs @@ -305,7 +305,7 @@ instance -- -- The disadvantage of this approach is that @addInteger 42@ will always unlift @42@ upon -- full saturation even if this partial application is saved to a variable. But the way - -- costing calibration benchmarks are setup, we always evaluate a single application, so + -- costing calibration benchmarks are set up, we always evaluate a single application, so -- the cost of unlifting is included in the cost of the builtin regardless of whether -- there's caching of unlifting or not. Hence the user pays for unlifting anyway and we can -- prioritize safety over performance here.