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

Speed defaultCekParameters* back up? #6180

Open
effectfully opened this issue Jun 5, 2024 · 1 comment
Open

Speed defaultCekParameters* back up? #6180

effectfully opened this issue Jun 5, 2024 · 1 comment

Comments

@effectfully
Copy link
Contributor

This:

defaultCekParametersA :: Typeable ann => MachineParameters CekMachineCosts DefaultFun (CekValue DefaultUni DefaultFun ann)
defaultCekParametersA =
    noinline mkMachineParameters DefaultFunSemanticsVariantA cekCostModelVariantA

and other such defaultCekParameters* definitions are artificially slowed down with that noinline call, so that if we use them anywhere where performance matters, we can spot it right away. But we use this definition a lot in tests and so many our tests are artificially slowed down. This is perhaps a waste of resources. Should we remove the noinline call? But then there's no guarantee that we won't get the same slowdown accidentally, since performance of those definitions isn't checked anywhere. And it probably won't even work properly now, because inlining machine parameters for each semantics variant will produce dozens of thousands of lines of code and GHC may simply give up trying to optimize all of that.

Or should we "simply" use the production mkMachineParametersFor everywhere including tests? That would reliably give us proper performance. The issue is with the "simple" part as that would me a major overhaul of tests. At least we can do it incrementally I suppose.

@kwxm
Copy link
Contributor

kwxm commented Jun 6, 2024

I was nervous about that code so I left it untouched. Can we try taking out noinline and see what effect it has on the total test execution time? That might help us to decide how seriously we should look at this.

See also #6088, which might need some reorganisation of the tests anyway.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants