Skip to content

Use a map to implement litToArrayPreimage #740

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

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

gustavo-grieco
Copy link
Collaborator

Description

In certain runs, litToArrayPreimage was taking most of the execution time so this is an optimized version of it using maps

Checklist

  • tested locally
  • added automated tests
  • updated the docs
  • updated the changelog

@blishko
Copy link
Collaborator

blishko commented May 11, 2025

Can you provide some examples and measurements?

@msooseth
Copy link
Collaborator

Wait, doesn't Haskell do memoization of such pure functions by default? I though this will only be computed once and then memoized... maybe I am wrong? If not, maybe we could simply add Memoizable as per: https://wiki.haskell.org/Memoization

What do you think?

@msooseth
Copy link
Collaborator

Haha, I'm an idiot. There is no such Memoizable Monad and memoization is clearly not the default. OK, so this is likely needed, actually...

@msooseth
Copy link
Collaborator

@blishko keccak is actually annoyingly slow and this thing needs to be recomputed every time if it's not memoized, which is likely pretty expensive. I just somehow thought this cannot possibly be non-memoized, but it seems like I was wrong....

litToKeccak :: Expr a -> Expr a
litToKeccak e = mapExpr go e
where
go :: Expr a -> Expr a
go orig@(Lit key) = case litToArrayPreimage key of
go orig@(Lit key) = case litToArrayPreimage key of -- This now calls the optimized version
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's remove the comment here, I think it's okay without.

@msooseth msooseth added the enhancement New feature or request label May 14, 2025
@gustavo-grieco
Copy link
Collaborator Author

Here is a profiling run from around 15 minutes of the exploration of a swap function where the 100% of time spent in the interpret function. Without the patch:

COST CENTRE        MODULE                 SRC                                               %time %alloc

splitSExpr         EVM.Solvers            src/EVM/Solvers.hs:(463,1)-(472,52)                32.1   14.4
writeBytes         EVM.SMT                src/EVM/SMT.hs:(915,1)-(929,76)                    16.7    3.0
litToArrayPreimage EVM.Expr               src/EVM/Expr.hs:(760,1)-(765,19)                   12.4    0.0
getSExpr           EVM.Solvers            src/EVM/Solvers.hs:(478,1)-(492,75)                 8.6   13.4
sendLine           EVM.Solvers            src/EVM/Solvers.hs:(421,1)-(424,45)                 8.1   24.8
readWordFromBytes  EVM.Expr               src/EVM/Expr.hs:(298,1)-(307,44)                    3.7    4.3
exprToSMT          EVM.SMT                src/EVM/SMT.hs:(648,1)-(828,105)                    3.2    9.0
untilFixpoint      EVM.Types              src/EVM/Types.hs:(1565,1)-(1569,27)                 2.0    5.2
mapExprM           EVM.Traversals         src/EVM/Traversals.hs:(269,1)-(590,20)              1.7    5.6
readByte           EVM.Expr               src/EVM/Expr.hs:(225,1)-(258,31)                    1.6    0.5
maybeTryFrom       Witch.Utility          source/library/Witch/Utility.hs:(124,1)-(126,19)    1.1    1.4
sendScript         EVM.Solvers            src/EVM/Solvers.hs:(387,1)-(396,108)                0.6    1.7
sp                 EVM.SMT                src/EVM/SMT.hs:831:1-35                             0.5    1.0
splitSMGen         System.Random.SplitMix src/System/Random/SplitMix.hs:(225,1)-(229,31)      0.2    1.2
encode             Data.Serialize         src/Data/Serialize.hs:109:1-21                      0.1    2.8

And with the patch:

COST CENTRE       MODULE                 SRC                                               %time %alloc

splitSExpr        EVM.Solvers            src/EVM/Solvers.hs:(463,1)-(472,52)                37.8   16.9
sendLine          EVM.Solvers            src/EVM/Solvers.hs:(421,1)-(424,45)                14.2   27.1
getSExpr          EVM.Solvers            src/EVM/Solvers.hs:(478,1)-(492,75)                12.9   15.0
writeBytes        EVM.SMT                src/EVM/SMT.hs:(915,1)-(929,76)                    12.7    3.0
exprToSMT         EVM.SMT                src/EVM/SMT.hs:(648,1)-(828,105)                    4.1    9.5
readWordFromBytes EVM.Expr               src/EVM/Expr.hs:(298,1)-(307,44)                    3.6    3.2
mapExprM          EVM.Traversals         src/EVM/Traversals.hs:(269,1)-(590,20)              1.6    4.8
readByte          EVM.Expr               src/EVM/Expr.hs:(225,1)-(258,31)                    1.4    0.4
MAIN              MAIN                   <built-in>                                          1.3    0.1
untilFixpoint     EVM.Types              src/EVM/Types.hs:(1565,1)-(1569,27)                 1.2    2.2
sendScript        EVM.Solvers            src/EVM/Solvers.hs:(387,1)-(396,108)                1.1    1.9
maybeTryFrom      Witch.Utility          source/library/Witch/Utility.hs:(124,1)-(126,19)    0.9    1.0
sp                EVM.SMT                src/EVM/SMT.hs:831:1-35                             0.7    1.2
splitSMGen        System.Random.SplitMix src/System/Random/SplitMix.hs:(225,1)-(229,31)      0.2    1.2
encode            Data.Serialize         src/Data/Serialize.hs:109:1-21                      0.1    2.2

@blishko
Copy link
Collaborator

blishko commented May 22, 2025

How can I reproduce this?

Copy link
Collaborator

@blishko blishko left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this will be good, but I think we can clean it up a bit.

-- | images of keccak(bytes32(x)) where 0 <= x < 256
preImages :: [(W256, Word8)]
preImages = [(keccak' (word256Bytes . into $ i), i) | i <- [0..255]]

-- | images of keccak(bytes32(x)) where 0 <= x < 256
preImageLookupMap :: Map.Map W256 (Word8, W256)
preImageLookupMap = Map.fromList $ map (\(imageHash, originalId) -> (imageHash, (originalId, imageHash + fromInteger 255))) preImages
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think we need to store imageHash + 255 in the value.

Co-authored-by: Martin Blicha <martin.blicha@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants