IntersectMBO / cardano-node

The core component that is used to participate in a Cardano decentralised blockchain.
https://cardano.org
Apache License 2.0
3.06k stars 720 forks source link

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

Closed koslambrou closed 1 year ago

koslambrou commented 2 years ago

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:

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):

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

github-actions[bot] commented 1 year ago

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.

steshaw commented 1 year ago

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 commented 1 year ago

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[bot] commented 1 year ago

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.