-
Notifications
You must be signed in to change notification settings - Fork 318
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
CIP-0132? | New Plutus Builtin DropList #767
base: master
Are you sure you want to change the base?
Conversation
The fundamental issue here is that we can't just make every function on builtin types a builtin (or even every recursive function). Today it's If we can't write acceptably fast recursive functions in Plutus Core, that's a problem with Plutus Core. It's also unclear to me whether the cost comes from doing the recursion, or from having to call |
We have done quite a bit of profiling. There is a drastic performance difference in the first Alternatively, O(1) index data-structures would address the vast majority of the use-cases for this builtin. |
Right, but you can use the budget profiling to work out exactly where the budget is going in the naive version. If you're not using |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@colll78 @michaelpj @michele-nuzzi adding to next CIP meeting agenda (https://hackmd.io/@cip-editors/83) for introduction at least.
Please @colll78 remove the template comments from the markup, since they're no longer useful after the content is written & can get in the way of editing.
cip-builtin-drop/README.md
Outdated
|
||
### Implementation Plan | ||
<!-- A plan to meet those criteria. Or `N/A` if not applicable. --> | ||
- [] Passes all requirements of both Plutus and Ledger teams as agreed to improve Plutus script efficiency and usability. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@michaelpj please let us know if this single checkbox item properly signifies all the requirements for Plutus Core changes as per CIP-0035, or if you think these need to be itemised further here & in similar proposals.
cip-builtin-drop/README.md
Outdated
- Philip DiSarro <[email protected]> | ||
Implementors: [] | ||
Discussions: | ||
- https://github.com/cardano-foundation/CIPs/pull/418/ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- https://github.com/cardano-foundation/CIPs/pull/418/ | |
- https://github.com/cardano-foundation/CIPs/pull/418 | |
- https://github.com/cardano-foundation/CIPs/pull/767 |
I presume that the first of these links represents one of the "design patterns" for which this would be useful (please correct if the older link is unrelated).
elemAt 26: naive
elemAtFast 26: heuristic optimized
tail26: tail called 26 times no recursion
elemAt: canonical ordering naive
elemAt: canonical ordering fast
tail called 26 times without recursion is 6632 mem, which blows everything else out of the water. It accounts for a very small portion of the budget. The difference in efficiency is quite dramatic. |
I very much appreciate @michaelpj's stance here. On the other hand, imho lists are fundamental enough to warrant dedicated builtin functions. |
I'd like to replicate the analysis to better understand what's going on here, but I haven't had time. |
@colll78 I'm (belatedly?) marking this as In any case please update this thread if anything has changed on your end or if you have any ideas how to push this forward in the Plutus world. Editors have not heard from Plutus official representatives in a while and we should consider it a top priority to confirm we have the right contacts & process for that (cc @Ryun1 @Crypto2099). |
I'm not sure what this is waiting on. I believe the benchmarking above should be sufficient to demonstrate how critical this feature is. If anyone would like to reproduce the benchmarks locally you can |
OK @colll78 I do agree with what you are saying, and think this should move forward even without a response from the current Plutus representatives about MPJ's requested details about benchmarking. As you say it should be independently verifiable and therefore we can work on merging this as So @Ryun1 @Crypto2099 if you agree let's assign a CIP number after tomorrow's meeting (I've put it on Review agenda: https://hackmd.io/@cip-editors/96) and leave aside the benchmarking question on the way to merge: unless the Plutus team wants to indicate any particular reservations. |
Co-authored-by: Ryan <[email protected]>
Comments from CIP Editor meeting # 96 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
On agenda for Review
again next meeting (https://hackmd.io/@cip-editors/97) - in the meantime @colll78 please try to post here whatever you can find confirming an implementation plan & update whether a hard fork is expected to be required. I don't think any of these are required before merging but it would be nice to do that with the document as complete as possible.
Please don't forget to change the containing directory to CIP-0132
🎉
cip-builtin-drop/README.md
Outdated
@@ -0,0 +1,117 @@ | |||
--- | |||
CIP: ? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
CIP: ? | |
CIP: 132 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I like this CIP and I propose that we merge it after @colll78 includes his benchmarking results in the CIP itself (at present it's quite vague on the benefits of adding the builtin). It would be even better if we could see the effect of adding the builtin on some actually used in practice smart contract rather than a microbenchmark.
There are however many things to discuss and investigate here. Why is the naive implementation so inefficient? Is it the cost of doing recursion in Plutus, is it the cost of calling 3 builtins per element, can there be something wrong with the UPLC of the benchmarks? And the most important question is whether this is an inherent limitation of Plutus or something that we can fix without introducing a tailored builtin (I highly doubt it's fixable without adding a builtin for any reasonable definition of "fixable").
And do we need to make the builtin tailored? Maybe we should add splitAt
instead? Or sliceList
similar to sliceByteString
that we already have? Or some general paramorphism?
My conjecture is that paramorphism would be an overkill and splitAt
a nice general enough solution that is still efficient.
So basically, show me that this meaningfully speeds up (by at least several percent) a popular smart contract and I'll be happy to champion this CIP. In the meantime, I've added the dropList
builtin for experimentation purposes here.
## Motivation: why is this CIP necessary? | ||
The deterministic script evaluation property of the ledger (also stated as "script interpreter arguments are fixed") is a unique characteristic of the Cardano ledger that allows us to perform powerful optimizations that are not possible in systems with indeterminstic script evaluation. For instance, searching for elements in a data structure can | ||
be done entirely off-chain, and then we simply provide the onchain code with the index (via redeemer) to where the element we want to find is supposed to be, and then check (onchain) that it is indeed the element we were expecting. This design pattern of passing the indices of elements required for validation logic in the redeemer is commonly referred to as redeemer-indexing. | ||
Even though it is still a very powerful optimization in its current state, it is currently bottlenecked by the lack of a builtin that applies tail a given number of times to a list. Currently, any implementation of `elemAt :: Integer -> List a -> a` or `drop` requires the use of the fixed point combinator (Y combinator) which has a significant cost in onchain code. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just to nitpick, it's the Z combinator, not Y.
Consider the naive approach: | ||
```haskell | ||
{- | Fixpoint recursion. Used to encode recursive functions. | ||
Hopefully this illustrates the overhead that this incurs. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The fact that it looks big does not mean that it's inefficient. We've benchmarked various ways to get recursion in Haskell (sic), the Z combinator is about on par with direct recursion. This doesn't translate to UPLC of course (since UPLC doesn't even have direct recursion), but my point is that it's very non-obvious how your Haskell code snippet illustrates any overhead.
The inefficiency comes from evaluating more things on the Plutus side than on the Haskell side. All those #
are going to be evaluated in Plutus and if those were $
in Haskell instead, they'd be much more efficient -- and that is where the overhead arises from. Plus the fact that you currently need 3 builtin calls per element to implement dropList
in Plutus and those are expensive.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I included that to illustrate how inefficient it is specifically in ex-unit terms, and as you said most of that cost comes from all those function applications that are invoked. Yes though, I agree that it only accounts for a small portion of the budget and most of the cost comes from the actual content of each recursive call.
CIP-0132/README.md
Outdated
pif b case_true case_false = pforce $ (pforce $ punsafeBuiltin PLC.IfThenElse) # b # pdelay (f PTrue) # pdelay (f PFalse) | ||
where | ||
f = \case | ||
PTrue -> case_true | ||
PFalse -> case_false |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why not just inline case_true
and case_false
instead of using f PTrue
and f PFalse
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In Plutarch you can think of Haskell level work as a sort of pre-compilation step. The above would actually be inlined when compiled, because f
or \case
(a Haskell level function) is used on PTrue
(a Haskell level argument), this would become:
pif b case_true case_false = pforce $ (pforce $ punsafeBuiltin PLC.IfThenElse) # b # pdelay case_true # pdelay case_false
after the pre-compilation step. I have edited it for clarity though.
pelemAt' = phoistAcyclic $ | ||
pfix #$ plam $ \self n xs -> | ||
pif | ||
(n #== 0) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This loops on negative integers (I understand that this snippet is for illustration purposes only, but I'll still leave this comment).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Intended behavior is to error on negative integers anyway, in this case looping is a more efficient way to achieve that because looping results in a script failure from ex-unit budget exceeding and we don't need to waste a check for n > 0
in the happy path. The only case this isn't more efficient is if the script will fail anyway, and in that case we don't care.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's a really good point, thanks!
We propose to introduce a new Plutus builtin
dropList
that drops a given number of elements from aBuiltInList
. Currently,elemAt
anddrop
must be implemented using fixed-point recursion and invoking many other Plutus builtins in each recursive call. These functions are critical components of many common smart contract design patterns (ie redeemer-indexing) so their poor performance has a large impact on the throughput of many protocols.(latest version in branch)