Skip to content
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

[PERF] - Performance issues with functions using toLedgerPParams in cardano-api #4622

Closed
koslambrou opened this issue Nov 7, 2022 · 4 comments · Fixed by #4903
Closed

[PERF] - Performance issues with functions using toLedgerPParams in cardano-api #4622

koslambrou opened this issue Nov 7, 2022 · 4 comments · Fixed by #4903
Assignees
Labels
comp: cardano-api Stale type: enhancement An improvement on the existing functionality user type: internal Created by an IOG employee

Comments

@koslambrou
Copy link
Contributor

Internal/External
Internal

Area
Other

Summary

We noticed some significant performance issues with Cardano.Api.Fees.evaluateTransactionFee and Cardano.Api.TxBody.makeTransactionBody. After profiling the code, we noticed that the reason was because the functions would convert Cardano.Api.ProtocolParameters to cardano-ledger's PParams every time we call one of the functions. However, the conversion between the two datatypes take s a significant amount of time because of a plutus-core function (applyCostModelParams ) when creating the cost model.

Steps to reproduce
Steps to reproduce the behavior:

  • Run profiling on a code which uses any function from cardano-api which uses toLedgerPParams such as makeTransactionBody and evaluateTransactionFee.

Expected behavior
I'm expecting that the conversion between cardano-api's ProtocolParameter and cardano-ledger's PParams does not require significant computation.

Even better, I think we should probably not have a different datatype for protocol parameters as we already have one in cardano-ledger. Why not create a wrapper on top of it instead of defining a different data type?

System info (please complete the following information):

  • OS Name: NixOS
  • OS Version master
  • Node version: cardano-node 1.35.3 - linux-x86_64 - ghc-8.10

Screenshots and attachments

Here are some profiling results of the problematic functions:

evaluateTransactionFee                                Cardano.Api.Fees                                         src/Cardano/Api/Fees.hs:(247,1)-(281,54)                                                          11260           4    0.0    0.0     4.3    4.1
 ...
 toLedgerPParams                                      Cardano.Api.ProtocolParameters                           src/Cardano/Api/ProtocolParameters.hs:(1355,1)-(1359,57)                                          11266           4    0.0    0.0     4.3    4.1
  toBabbagePParams                                    Cardano.Api.ProtocolParameters                           src/Cardano/Api/ProtocolParameters.hs:(1523,1)-(1600,73)                                          11267           4    0.0    0.0     4.3    4.1
   toShelleyLovelace                                  Cardano.Api.Value                                        src/Cardano/Api/Value.hs:116:1-47                                                                 11269          16    0.0    0.0     0.0    0.0
   promoteRatio                                       Cardano.Ledger.BaseTypes                                 src/Cardano/Ledger/BaseTypes.hs:182:1-68                                                          11270          12    0.0    0.0     0.0    0.0
   toAlonzoExUnits                                    Cardano.Api.Script                                       src/Cardano/Api/Script.hs:(903,1)-(907,3)                                                         11457           8    0.0    0.0     0.0    0.0
   toAlonzoCostModels                                 Cardano.Api.ProtocolParameters                           src/Cardano/Api/ProtocolParameters.hs:(800,1)-(808,59)                                            11271           4    0.0    0.0     4.3    4.1
    toAlonzoScriptLanguage                            Cardano.Api.ProtocolParameters                           src/Cardano/Api/ProtocolParameters.hs:(819,1)-(820,80)                                            11274          16    0.0    0.0     0.0    0.0
    toAlonzoCostModel                                 Cardano.Api.ProtocolParameters                           src/Cardano/Api/ProtocolParameters.hs:827:1-99                                                    11272           8    0.0    0.0     4.3    4.1
     mkCostModel                                      Cardano.Ledger.Alonzo.Scripts                            src/Cardano/Ledger/Alonzo/Scripts.hs:(245,1)-(252,20)                                             11273           8    0.0    0.0     4.3    4.1
      mkEvaluationContext                             Plutus.ApiCommon                                         src/Plutus/ApiCommon.hs:(278,1)-(281,66)                                                          11275           8    0.0    0.0     4.3    4.1
       applyCostModelParams                           PlutusCore.Evaluation.Machine.CostModelInterface         plutus-core/src/PlutusCore/Evaluation/Machine/CostModelInterface.hs:191:1-70                      11276          16    0.1    0.1     4.3    4.0
        ...

Additional context
None

@koslambrou koslambrou added cardano-api Associated with the cardano-api module type: bug Something is not working comp: cardano-api labels Nov 7, 2022
@Jimbo4350 Jimbo4350 self-assigned this Nov 7, 2022
@koslambrou koslambrou changed the title [BUG] - Performance issues with functions using toLedgerPParams in cardano-api [PERF] - Performance issues with functions using toLedgerPParams in cardano-api Nov 7, 2022
@koslambrou koslambrou added comp: performance and removed type: bug Something is not working labels Nov 7, 2022
@dorin100 dorin100 added user type: internal Created by an IOG employee type: enhancement An improvement on the existing functionality and removed cardano-api Associated with the cardano-api module comp: performance labels Nov 8, 2022
@github-actions
Copy link

github-actions bot commented Dec 9, 2022

This issue is stale because it has been open 30 days with no activity. Remove stale label or comment or this will be closed in 120 days.

@github-actions github-actions bot added the Stale label Dec 9, 2022
@steshaw
Copy link

steshaw commented Dec 28, 2022

We noticed this as well. I put together a patch against cardano-node 1.35.3 which memoises toBabbagePParams. This sped up our test suite (using cooked-validators) by 80%. This might not be the ideal fix, but it validates that there is a problem certainly for the property-testing use case.

@berewt
Copy link

berewt commented Jan 9, 2023

Wouldn't it be sufficient to change the signature from:

ShelleyBasedEra era -> TxOut CtxTx era -> ProtocolParameters -> Either MinimumUTxOError Value

to: ShelleyBasedEra era -> TxOut CtxTx era -> PParams era -> Either MinimumUTxOError Value

or if we don't want to change it, provide a new function with the second signature?

@github-actions
Copy link

github-actions bot commented Feb 9, 2023

This issue is stale because it has been open 30 days with no activity. Remove stale label or comment or this will be closed in 120 days.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
comp: cardano-api Stale type: enhancement An improvement on the existing functionality user type: internal Created by an IOG employee
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants