-
Notifications
You must be signed in to change notification settings - Fork 647
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
API and data structure refactor #3218
API and data structure refactor #3218
Conversation
Hi, @yamt, I did some further refactoring according to the first bulletin mentioned in this issue #2530. Could you please take a look at this PR at your convenience? By the way, I tested the performance on the 32-bit platform after refactoring. On benchmarks, including CoreMark, Dhrystone, and JetStream, there was basically no performance impact for JetStream, and the performance was roughly down by 1% for CoreMark and 2% for Dhrystone. I think generally speaking, we don't have to worry about the performance impact on the 32-bit platform |
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
…ve common data member from WASMModuleInstanceExtraCommon to WASMModuleInstance
e7a955b
to
b6662d1
Compare
@yamt, I merged this PR (and also PR #3209) since we are to implement the memory64 feature and hope to refactor the related APIs and data structures before that, and also to make the related update for wamr-rust-sdk, Python language binding and Go language binding. @TianlongLiang have done a lot of test on the two PRs and also the code has been carefully reviewed. Please let us know if there are issues found. Many thanks. |
this patch looks reasonable at a glance. but i have no time to make careful review this month. i guess ideally we can stop aot relying on known sizeof(WASMModuleInstance) so that all these |
Yes, it may be another way that AOT doesn't rely on BTW, we found the global shared memory lock issue you mentioned in #3209, and plan to add a field in WASMMemoryInstance, |
…ASMModuleInstance (bytecodealliance#3218) Remove the unused parameter `signature` from `wasm_runtime_lookup_function`. Refactor the layout of WASMModuleInstance structure: - move common data members `c_api_func_imports` and `cur_exec_env` from `WASMModuleInstanceExtraCommon` to `WASMModuleInstance` - In `WASMModuleInstance`, enlarge `reserved[3]` to `reserved[5]` in case that we need to add more fields in the future ps. bytecodealliance#2530 bytecodealliance#3202
wasm_runtime_lookup_function
c_api_func_imports
andcur_exec_env
fromWASMModuleInstanceExtraCommon
toWASMModuleInstance
WASMModuleInstance
enlarge reserved[3] to reserved[5] in case add more data member in the future