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

Wrapping and unwrapping DataNewtype blows up script budget #764

Open
colll78 opened this issue Nov 26, 2024 · 1 comment
Open

Wrapping and unwrapping DataNewtype blows up script budget #764

colll78 opened this issue Nov 26, 2024 · 1 comment

Comments

@colll78
Copy link

colll78 commented Nov 26, 2024

instance PIntegral PPosixTime where
{-# INLINEABLE pdiv #-}
pdiv = phoistAcyclic $ plam $ \t1 t2 ->
pposixTime (pdiv # unPPosixTime t1 # unPPosixTime t2)
{-# INLINEABLE pmod #-}
pmod = phoistAcyclic $ plam $ \t1 t2 ->
pposixTime (pmod # unPPosixTime t1 # unPPosixTime t2)
{-# INLINEABLE pquot #-}
pquot = phoistAcyclic $ plam $ \t1 t2 ->
pposixTime (pquot # unPPosixTime t1 # unPPosixTime t2)
{-# INLINEABLE prem #-}
prem = phoistAcyclic $ plam $ \t1 t2 ->
pposixTime (prem # unPPosixTime t1 # unPPosixTime t2)
-- | @since 2.0.0
instance PNum PPosixTime where
{-# INLINEABLE (#*) #-}
t1 #* t2 = pposixTime (unPPosixTime t1 #* unPPosixTime t2)
{-# INLINEABLE (#+) #-}
t1 #+ t2 = pposixTime (unPPosixTime t1 #+ unPPosixTime t2)
{-# INLINEABLE (#-) #-}
t1 #- t2 = pposixTime (unPPosixTime t1 #- unPPosixTime t2)
{-# INLINEABLE pabs #-}
pabs = phoistAcyclic $ plam $ \t -> pposixTime (pabs # unPPosixTime t)
{-# INLINEABLE pfromInteger #-}
pfromInteger = pposixTime . pconstant
{-# INLINEABLE pnegate #-}
pnegate = phoistAcyclic $ plam $ \t -> pposixTime (pnegate # unPPosixTime t)
{-# INLINEABLE psignum #-}
psignum = phoistAcyclic $ plam $ \t -> pposixTime (psignum # unPPosixTime t)

This module is bricked. Switched from PInteger to PPosixTime and my benchmarks had a huge regression. The issue is that all the operations wrap and unwrap the data, this results in a ton of iData and unIData builtin calls, because each addition unwrapped each integer from data and then wraps the result. Instead we should have two definitions PosixTimeData and PosixTime or anything else so that this can be avoided and we can just keep it in the non-data encoded format until we want to go back to data encoding.

@SeungheonOh
Copy link
Collaborator

SeungheonOh commented Nov 26, 2024

Yes, I'm aware of this issue. We are going to remove PDataNewtype which was a big mistake. It made types more intuitive, but really obfuscated cost of construting and deconstructing data value

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

No branches or pull requests

2 participants