-
Notifications
You must be signed in to change notification settings - Fork 1.8k
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
perf: exclude source maps from ContractData
#8022
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.
lgtm
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.
How does this help? Are we cloning this structure?
Yes, we build and keep separate |
confirming memory does not exceed 4.5G with this PR (prev was 13G) 🎉 |
Sure but it's |
yeah we don't clone it in tests, the memory usage is coming from us building it for each test contract here and then not dropping till the end of test run |
Ah it's kept in suite result |
yep, we can optimize this by not linking test contracts separately and ensuring deterministic library addresses instead, will look into this |
Clearing it in tests should be fine, it's skipped in serde and not used after f8145ed |
Motivation
ref #8014
Since #7937
ContractData
stored entireCompactBytecode
instead of just bytecode bytes.CompactBytecode
includes source maps and thus results in increased memory usage to store those inknown_contracts
when running tests.Solution
Introduce separate
BytecodeData
struct which does not keep source maps as we don't need them for bytecode identification.cc @grandizzy