[instantiation linking] Refactor wasm_runtime_instantiate() #3893
Conversation
lum1n0us
force-pushed
the
fix/optimize_runtime_instantiate
branch
from
4016457
to
36646a0
Compare
lum1n0us
changed the title
lum1n0us
requested review from
loganek,
no1wudi,
wenyongh,
xujuntwt95329 and
yamt
as code owners
lum1n0us
force-pushed
the
fix/optimize_runtime_instantiate
branch
5 times, most recently
from
edddd11
to
f261840
Compare
- ~99% same with memory_instantiation() in wasm_runtime.c - new apis to create and destroy memory
…tantiation logic in wasm_runtime.c
The main goal is to let existed code run without modification
…ctor related import handling in thread management
lum1n0us
force-pushed
the
fix/optimize_runtime_instantiate
branch
from
f261840
to
b71bee8
Compare
core/iwasm/include/wasm_export.h
Outdated
|
|
||
| WASM_RUNTIME_API_EXTERN void | ||
| WASM_RUNTIME_API_EXTERN | ||
| void |
There was a problem hiding this comment.
Is there a reason to have the return type on a separate line here?
| @@ -300,15 +296,15 @@ memory_instantiate(const WASMModule *module, WASMModuleInstance *parent, | |||
| uint8 *aux_heap_base_global_data, char *error_buf, | |||
| uint32 error_buf_size) | |||
| { | |||
| bh_assert(memory != NULL); | |||
There was a problem hiding this comment.
I thought we were sticking with declarations at the top of the block for compatibility with older C standards? This puts code before the declarations.
There was a problem hiding this comment.
After careful observation, I've come to understand that it is Ok to relax the restrictions on where variable declarations are placed. All the compilers and products we're aware of won't have an issue with this change. However, if you're aware of any exceptions, please inform me.
The benefit is clear. Code readability will improve if we move variable declarations closer to where they are used.
There was a problem hiding this comment.
Sounds good, I am for this new approach.
|
|
||
| total_size = sizeof(WASMMemoryInstance *) * (uint64)memory_count; | ||
|
|
||
| if (!(memories = runtime_malloc(total_size, error_buf, error_buf_size))) { | ||
| return NULL; | ||
| } | ||
|
|
||
| memory = module_inst->global_table_data.memory_instances; | ||
| WASMMemoryInstance *memory = |
There was a problem hiding this comment.
Same as previous comment, are we ok with declarations not being at the top?
There was a problem hiding this comment.
This is addressed by the previous discussion.
| @@ -11,6 +11,7 @@ find_path(WASISDK_HOME | |||
| REQUIRED | |||
| ) | |||
|
|
|||
|
|
|||
- Remove unnecessary blank line in FindWASISDK.cmake for improved readability - Format function declaration for wasm_runtime_destroy_imports for consistency
based #3845 and #3887