-
Notifications
You must be signed in to change notification settings - Fork 101
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
Heap size limits for module loading + Memory improvements. #964
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Some add on concerns:
- need to retain info for the REPL (in
initReplState'
probably) - need to run all the pact repl tests for BOTH stripped and not
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks great @jmcardon
src/Pact/Gas.hs
Outdated
throwErr GasError info $ "Gas limit (" <> pretty _geGasLimit <> ") exceeded: " <> pretty gUsed | ||
else return gUsed | ||
|
||
logGas :: Text -> GasArgs -> Gas -> Eval e () |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If these refactors aren't needed then remove them -- looks like you switched to computeGas
so we shouldn't be exporting internal ops anymore or making them toplevel
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
looks good!
I added a couple of nits that aren't deal breakers.
I do have some questions regarding the tests. I saw that in memory cost was disabled in the gas tests and the golden tests. Is this because the expected gas differed?
No description provided.