plutus icon indicating copy to clipboard operation
plutus copied to clipboard

[Epic] AST and codegen optimizations

Open effectfully opened this issue 1 year ago • 3 comments

This issue is for dumping all the plans regarding AST optimizations (such as case-of-case, case-of-known-constructors etc) in a largely unstructured way. We'll have separate messages for Plutus Core and Plutus IR, please just edit those instead of creating more messages.

effectfully avatar May 09 '24 00:05 effectfully

Untyped Plutus Core

  • [ ] regarding order of effects, we should figure out a strategy for when it's fine to change it and when it's not. It's worth handling logging and bottom separately, since messing with the former is OK (given it's locked behind a flag not enabled by default) and messing with the latter is a no-no
  • [ ] I saw ((\cse -> case cse [cse]) error), we should rewrite this to just error
  • [ ] we need a test that the inliner inline blah in (\b -> force ifThenElse b branch1 branch2) blah. If it doesn't, we should implement that functionality
  • [ ] handle chooseList in case-of-case similarly to ifThenElse?
  • [ ] Aiken folks gave us an idea: try replacing Scott-encoded tuples with SOP-encoded tuples in the recursive functions combinators, SOP-encoded tuples should be faster thus making mutual recursion faster. If there’s anything else Scott-encoded there, we should change it too. Perhaps we could update the entire PlutusCore.StdLib.Meta.Data.Tuple. We have to keep the old code too for older versions of Plutus
  • [ ] both UPLC and PIR have termEvaluationOrder defined for them which goes into "known function body" in case of Apply (see the logic there) and goes into "known delayed term" for Force, but does not go into "known branch" for Constr
  • [ ] both UPLC and PIR have termEvaluationOrder defined for them which does into "known delayed term" for Force and considers that to be MaybeWork. Is it work though? It literally has to be optimized away, do we want to prevent some optimization from firing just because there's an immediately Forced Delay? This should at least be discussed in a Note
  • [ ] we sometimes generate things like force (force (force ifThenElse b (delay (delay x)) (delay (delay y)))). Surely we can turn double-force-delays into single force-delays?
  • [ ] “Outer let-floating” for UPLC is considered to be moving (\x1...xn -> body) t1 ... tn outside of a context. We have investigated the case where the context is force, this comment should argue why I believe that for proper force-delay cancellation we need to implement something more specialised, as is done in the PR. This is for investigating other contexts: <context> ((\x1 ... xn -> body) t1 ... tn).
  • [ ] (((\a b c -> a + b + c) 10) 20) 30 is more expensive than (case (constr 0 [10, 20, 30]) f) given we apply more than 3 arguments, so we can have transform that converts chained applications into this form. This is one way of implementing it, but this could be simpler since this deals with some other stuff as well (I read somewhere ApplyN is planned, so once we have that this will be outdated. Though, I think this transformer is simple and good to have enough till than)
  • [ ] force-delay knows how to look under applications, but force-case-delay doesn't. We should just shove those into a singe optimization to avoid reimplementing the same tricks

effectfully avatar May 09 '24 00:05 effectfully

Plutus IR

  • [ ] now that we have caseList and caseData, the case-of-case optimization should perhaps do something about those builtins as well
  • [ ] the dead code eliminator has a bug causing it to remove data types that are not dead, see #6517

effectfully avatar May 09 '24 00:05 effectfully

Plinth

  • [ ] Add Lazy and use it throughout Plinth code instead of the slower () (extricated from #6060). PR: #5910
  • [ ] builtinListIndexing.pir.golden has chooseList {data} {Unit -> Unit -> data}, why do we generate those double Units? We should fix that
  • [ ] unionValue is inefficient, because unionWith is, because Map.union is, because of all the These extra work that it does. If unionValue is important enough to be considered as a builtin, it is important enough to be optimized in Plinth. PR: #6590
  • [ ] @colll78 has an interesting point about optimizing functions by letting them loop instead of throwing an error: "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". Now we do want to throw errors normally for debugging purposes, but maybe we could have an optimization removing such clauses before submitting the script to the chain

effectfully avatar Oct 03 '24 21:10 effectfully