-
Notifications
You must be signed in to change notification settings - Fork 325
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-0091? | Don't force Built-In functions #459
Conversation
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 CIP is a bit of a sketch, but this is an idea we've considered before and I like it. Well, I like the more extreme version where we just don't let builtins be forced at all.
However, I do think that this is likely to have a relatively minor impact, and so to some degree whether or not we do this depends on how easy it is to implement, particularly in respect to maintaining backwards compatibility (builtins in old language versions must continue to work the same forever!).
CIP-????/README.md
Outdated
|
||
There are two options for the implementation of this proposal. | ||
Either the new functions are added to the set of available builtin functions or they replace the current functions. | ||
This is up to discussion and shifts additional work to either the implementation of UPLC or the implementation of PLC. |
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.
It's not really more work if we replace them. If it was the case that builtins didn't need to be forced, we could just add an optimization pass that removed all the forces.
CIP-????/README.md
Outdated
This is up to discussion and shifts additional work to either the implementation of UPLC or the implementation of PLC. | ||
|
||
Addition: | ||
- UPLC needs to support a more diverse set of operations, implying more resources needed for maintainance and secondary implementations |
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.
Sadly, this is true to some degree anyway, since we have to support the old versions forever in the implementation.
CIP-????/README.md
Outdated
## Specification | ||
<!-- The technical specification should describe the proposed improvement in sufficient technical detail. In particular, it should provide enough information that an implementation can be performed solely on the basis of the design in the CIP. This is necessary to facilitate multiple, interoperable implementations. --> | ||
For all existing UPLC Builtin Functions _x_ that require _n > 0_ forces for evaluation, this proposal suggests to implement the builtin function _x_ | ||
without any required forces. |
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 would propose a much stronger version of this. Instead of just implementing the current builtins so that they don't require forces, we instead make it so that no builtins ever need forces. That makes the backwards compatibility story trickier for the evaluator, but I think we could manage it.
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.
Ideally, I would like to do this in a sneaky way so we can use the same implementation for previous ledger languages as well, but that might be tricky.
CIP-????/README.md
Outdated
|
||
It must also explain how the proposal affects the backward compatibility of existing solutions when applicable. If the proposal responds to a CPS, the 'Rationale' section should explain how it addresses the CPS, and answer any questions that the CPS poses for potential solutions. | ||
--> | ||
This proposal reduces the resources needed to evaluate builtin functions by removing the need to apply no-op force operations to them. |
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 would really like an impact assessment of this. My suspicion is that the actual performance impact will be negligible, and the main impact will be on making it easier for compiler writers and simplifying the language. As a compiler writer and a maintainer of the language, I appreciate those things, but they're much weaker reasons than widespread performance improvements IMO.
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 would really like an impact assessment of this.
There was one data point here.
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.
Also there is a proposed solution there: add the necessary delays before the builtin and let simplification erase the force/delay pairs. One limitation mentioned there was that type arguments are not required to come first, but this can be made a requirement (as we've been discussing elsewhere).
CIP-????/README.md
Outdated
This proposal reduces the resources needed to evaluate builtin functions by removing the need to apply no-op force operations to them. | ||
|
||
If the decision is to replace the builtin functions: | ||
The resulting implementation will break backwardscompatability of implementing Plutus Smart Contracts. |
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.
It's a non-backwards-compatible change, so it will require a new Plutus ledger language, see CIP-35.
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.
Would it be non-backwards-compatible if unrestricted force is implemented?
CIP-????/README.md
Outdated
|
||
## Path to Active | ||
|
||
I need some advice on the following to sections. As I understand the specification and implementation of UPLC and PLC is currently under supervision of |
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 statement looks like it was finished "... IOG and will thus need their approval." as currently written oin the initial comment for this PR. You are on the right track with @michaelpj's review especially the reference to CIP-0035 which explains the things that would need to go into the Plan to Active.
Since @michaelpj just submitted a CIP also dealing with types in Plutus you can probably just adapt the one here, modifying the section about "benchmarks" to reflect his comments above about performance impact (?): https://github.com/michaelpj/CIPs/blob/mpj/sums-of-products/CIP-%3F%3F%3F%3F/README.md#path-to-active
Don't allow forcing what is necessarily in WHNF. |
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.
What if we simply implement an optimization that binds force (force (builtin fstPair))
to a variable and replaces all occurrences of this expression with that name? And similarly for all other builtins. Then we'll get a constant and pretty negligible amount of overhead per builtin.
While I certainly like the idea of untangling UPLC from TPLC/PIR/Plutus Tx, I'm not sure I like the idea of untangling UPLC from any kind of types. Built-in functions have signatures, those signatures have quantifiers, we should account for quantifiers somehow and force
is a sensible way of doing that, semantics-wise. If we can get away with having minimal runtime/size overhead by pulling those force
s out of the program, then I'll vote for not weirdifying the language. Particularly because such a change would be even trickier in the blockchain context where we have to maintain backwards compatibility or perform weird and potentially risky tricks.
Inviting @mjaskelioff and @jmchapman to participate in this discussion.
CIP-????/README.md
Outdated
|
||
It must also explain how the proposal affects the backward compatibility of existing solutions when applicable. If the proposal responds to a CPS, the 'Rationale' section should explain how it addresses the CPS, and answer any questions that the CPS poses for potential solutions. | ||
--> | ||
This proposal reduces the resources needed to evaluate builtin functions by removing the need to apply no-op force operations to them. |
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 would really like an impact assessment of this.
There was one data point here.
@effectfully What you describe is more or less exactly what i.e. plutonomy does. I am describing here the ideal situation from a user perspective, not from a blockchain developer perspective. Whether or not it is feasible/worthwile is up to discussion. |
Another data point is here: IntersectMBO/plutus#5112 That suggested that getting rid of a significant fraction of the redundant forces in a program had minimal effect on runtime. |
Plutarch already does this. It's not always worth it though, because let-bindings are expensive too.
I disagree with the reasoning in your argument. Semantics-wise, The real issue in my opinion is Plutus's reliance on impure behaviour. Even though Plutus is supposedly a functional language, you're supposed to cause a transaction failure by the script failing, an impure effect! Now, all of a sudden order of evaluation matters, when it should not have mattered in the first place. Of course, order of evaluation always matters in some sense, since one manner of evaluation might push the transaction past its allocated script budget, yet I believe that in practice, this is rare, as the programs we put in scripts aren't too complicated. How would Plutus be made pure? Rather than failing invalid terms, make them stuck. Then, In practice this would mean the pathological example of I believe this would allow entirely dropping all vestiges of call-by-name from the translation of Haskell into UPLC. (Of course, you'd need a new language version, and existing Plutus scripts can't be trivially ported over, unless they're already in the idiomatic style which no one uses because of performance reasons.) I considered this approach while thinking about replacing UPLC with an SKI calculus after revisiting https://okmij.org/ftp/tagless-final/ski.pdf for the 100th time. As for the budget issue, this is still relevant, but even without this change it is quite honestly a huge pain the ass as you've likely already noticed.
I still believe we should get rid of |
This is all a bit of a digression, but...
This isn't quite right. Generally, polymorphism can be weird in strict languages if you erase the types entirely, because you can end up in a situation like this:
and then we evaluate
Surprise! Order of evaluation does not matter today (apart from logging, which isn't observable on-chain). Since the only effect is error and the language is strict, changing the order of evaluation cannot affect whether or not you fail, it can only affect which failure you hit, and they're not distinguishable, so it doesn't matter.
I think this depends a lot on how we stuckness was handled. I see two approaches:
So I think this proposal is either equivalent to the status quo or a weird strict/non-strict hybrid. |
I think 2. is the best solution. It is "weirdly" non-strict, but it solves the problem cleanly I think. You're right wrt. the rest FWIW. Rather than order of evaluation, perhaps saying evaluation matters even if you ignore its result? Which is essentially the same as saying it's impure. The precise semantics would likely be something like this:
We get:
(excuse me for not using the correct UPLC syntax, I can't remember it). Perhaps the name "stuck" isn't quite right, because it's not possible to become unstuck (since there is nothing to wait for). The original term is entirely gone and replaced by something which is really just an error. The difference is entirely in how we handle the error. In fact, you can describe the above as simply: Make I will probably make a CIP as I believe this is IMO the cleanest solution. I don't believe we have to remove |
I don't think is a strictness issue, it's an issue of breaking canonicity, where in our case "canonicity" means that evaluation of any closed term either produces a value in canonical form or diverges. With @L-as' proposal we'd have non-divergent non-canonical values. Which wouldn't be particularly unusual (those are normal when evaluating open terms or in a language with axioms/postulates), but it doesn't make for an appealing metatheory.
Strict/non-strict hybrids are not necessarily weird and I remember proposing Plutus Core to be call-by-push-value (which supports both strict and non-strict) back in 2018. @L-as how would (I'm horribly abusing the notation)
evaluate with your proposal? |
@effectfully It would diverge as expected. |
@L-as I don't understand the point then. You said
but if a basic if-then-else expression diverges unless we keep the existing machinery that prevents the two branches from being evaluated at the same time, then what problem are we trying to solve here with the introduction of stuck values? |
I don't believe we have any machinery to prevent this besides the compiler adding in lambdas (or delays) to the arguments, then forcing the result. This would not change. |
Sorry, I really don't get it. So given
How is it unexpected that
blows up but it's expected that
loops? |
I am not sure how to proceed. I think the discussion kind of derailed on a related topic that is however not connected to this CIP. There also does not seem a strong position to keep the CIP (right now), so I am pondering if I should just leave it open to be considered at another point again or just close it? I think finalizing it does not make sense if there is no strong support to accept it. |
@nielstron Personally, I think it's useful to have Proposed CIPs that won't necessarily be accepted (@rphair I can't remember what we decided was the right way to do this?). Then in future if the discussion recurs we can refer to the previous proposal and see what we thought. So I would be keen for you to incorporate some of the discussion into the main text and then we can merge it. |
Co-authored-by: Matthias Benkort <5680256+KtorZ@users.noreply.github.com>
I have adjusted the proposal as per the comments. |
To repeat my recommendation, I think it would be fine to merge this as Proposed. |
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 I agree and would be inclined to merge this for reasons you mention in #459 (comment) since it seems the comments are now incorporated as of 0839537.
@nielstron the text you removed as "artifacts" in 27e4916 are in fact the canonical section titles as per CIP-0001: so if you would please put them back in as such (and fix the CIP header title, or if not then this PR title), I am ready to approve this.
Co-authored-by: Robert Phair <rphair@cosd.com>
Co-authored-by: Robert Phair <rphair@cosd.com>
Co-authored-by: Robert Phair <rphair@cosd.com>
@rphair Thanks! Changes applied. |
@Ryun1 @Crypto2099 this has been waiting for a 2nd editor review before merge since March 2023. Since all the discussion was settled as much as it ever could be (I think), and the last ruling from Plutus expertise was for this to stand as Proposed even though nobody will do it (#459 (comment)), I'm marking it |
Can you rename the folder for this proposal from |
Co-authored-by: Ryan <44342099+Ryun1@users.noreply.github.com>
@nielstron the only thing we're waiting for before approving & merging is you renaming the containing directory. |
Oh sorry, I missed that. I also adjusted the implementation plan to correctly note there is no plan to actually implement this CIP |
A rough draft of a CIP that suggests to remove the type instantiations (which are essentially no-ops) from builting UPLC functions.
This should be benefitial for the development of (among others) @aiken-lang, @Hyperion-BT 's helios, and @OpShin 's eopsin.
Rendered draft