-
Notifications
You must be signed in to change notification settings - Fork 778
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
Fix EVM test scripts #2451
Fix EVM test scripts #2451
Conversation
@jochem-brouwer In looking at the changes from #2405 again, why do we even bother creating a memory store of anything other than 8192 bytes? I know this is ancient history but it seems like it might make it might be more efficient to just have the private |
Codecov Report
Additional details and impacted files
Flags with carried forward coverage won't be shown. Click here to find out more. |
We do, because every call frame needs a new buffer (so up to 1024 buffers). And we need to clear out the created buffers. If we keep "caching" this memory it means that we keep the memory allocated for the tx which used the most memory per frame (so we do not "garbage collect"). So I am not sure if this is the way the go? |
As best I can tell, the code you introduced in #2404 just automatically expands the memory buffer to 8192 bytes every you read from it, if it's not already 8192 bytes. So maybe I'm not sure the difference? |
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.
LGTM!
The
evm
tests have been failing since #2405 but thenpm run coverage
script wasn't picking this up due to the way it was constructed and missing the error code returned by the initial call to run the tests.TODO
evm/test/memory.spec.ts