Performance issues with applyCostModelParams

[koslambrou]

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.

An issue was created on cardano-node repo IntersectMBO/cardano-node#4622 in order to bypass the conversion.

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.

Actual Result

Slow

Expected Result

Faster

Describe the approach you would take to fix this

No idea.

System info

OS: NixOS
Version: master
commit hash: a56c965

[thealmarty]

I'm unsure if this can be solved, but maybe Kenneth can take a look.

[kwxm]

There's an issue for this at PLT-1243, and it seems that @effectfully has some ideas. We should try to find some time to work on it.

[effectfully]

There was some more discussion here. I think the conclusion was that we shouldn't worry. I'd be fine if this issue was simply closed.

[thealmarty]

As Kenneth pointed out, ticket created already. We can close this when we finish it (PLT-1243).

[kwxm]

Checking the various discussions mentioned, it seems that the solution is that callers should call applyCostModelParams once and cache the results (presumably the ledger has to cache the result for every set of protocol parameters it encounters). Ideally we'd make applyCostModelParams a lot faster, but we've thought about that and couldn't come up with a good solution: let's keep thinking though (that's what PLT-1243 is). Perhaps we should put a prominent comment in CostModelInterface.hs so that no-one gets bitten by this in future.

So do we think it's safe to close this? Any comments from @koslambrou / @michaelpj / @zliu41 ?

[effectfully]

Ideally we'd make applyCostModelParams a lot faster, but we've thought about that and couldn't come up with a good solution

I have one idea that I'm pretty sure is going to help with at least a part of the issue (maybe not a bottleneck, though) and one hunch that I never had the time/need to investigate. The idea is to stop doing quadratic things when superlinear will suffice (documented somewhere in Jira) and a hunch is that we can structure applyCostModelParams in a way that allows GHC to pull the most expensive stuff out of it, so that it gets cached automatically at each call site. I'm not sure if the latter is worth it, given that explicit caching is simple and reliable, unlike implicit one.

[kwxm]

explicit caching is simple and reliable, unlike implicit one.

Agreed.

[koslambrou]

@kwxm I'm good with closing it. Explicit caching is the way to go :)

[kwxm]

Oh, GitHub seems to have automatically closed this when I merged the branch. I didn't know it could do that. It seems that it's because of the "Closes #4962" comment.