Skip to content
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

Allow not copying the wasm binary into the module when using WASM C API #3389

Merged
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
fix: put additional checks in wasm_runtime_is_underlying_binary_freeable
  • Loading branch information
eloparco committed May 15, 2024
commit 7e94c602ee284de0a357c53a0a2f6da32c9dc7cd
9 changes: 6 additions & 3 deletions core/iwasm/common/wasm_c_api.c
Original file line number Diff line number Diff line change
Expand Up @@ -2999,10 +2999,13 @@ wasm_module_get_name(wasm_module_t *module)
}

bool
wasm_module_is_underlying_binary_freeable(const wasm_module_t *module)
wasm_module_is_underlying_binary_freeable(
const wasm_module_t *module, const struct wasm_instance_t *instance)
{
return ((wasm_module_ex_t *)module)->is_binary_cloned
&& wasm_runtime_is_underlying_binary_freeable(*module);
if (((wasm_module_ex_t *)module)->is_binary_cloned)
return true;

return wasm_runtime_is_underlying_binary_freeable(instance->inst_comm_rt);
}

static wasm_func_t *
Expand Down
45 changes: 34 additions & 11 deletions core/iwasm/common/wasm_runtime_common.c
Original file line number Diff line number Diff line change
Expand Up @@ -1389,6 +1389,7 @@ wasm_runtime_load(uint8 *buf, uint32 size, char *error_buf,
{
LoadArgs args = { 0 };
args.name = "";
args.wasm_binary_freeable = false;
return wasm_runtime_load_ex(buf, size, &args, error_buf, error_buf_size);
}

Expand Down Expand Up @@ -7180,27 +7181,49 @@ wasm_runtime_detect_native_stack_overflow_size(WASMExecEnv *exec_env,
}

WASM_RUNTIME_API_EXTERN bool
wasm_runtime_is_underlying_binary_freeable(const wasm_module_t module)
wasm_runtime_is_underlying_binary_freeable(const wasm_module_inst_t module_inst)
{
bool is_freeable = false;
uint32 i;

#if WASM_ENABLE_INTERP != 0
eloparco marked this conversation as resolved.
Show resolved Hide resolved
if (module->module_type == Wasm_Module_Bytecode) {
if (module_inst->module_type == Wasm_Module_Bytecode) {
#if (WASM_ENABLE_JIT != 0 || WASM_ENABLE_FAST_JIT != 0) \
&& (WASM_ENABLE_LAZY_JIT != 0)
is_freeable = false;
return false;
#elif WASM_ENABLE_FAST_INTERP == 0
is_freeable = false;
return false;
#else
is_freeable = ((WASMModule *)module)->is_binary_freeable;
/* Fast interpreter mode */
WASMModule *module = ((WASMModuleInstance *)module_inst)->module;
if (!module->is_binary_freeable)
return false;
#if WASM_ENABLE_GC != 0 && WASM_ENABLE_STRINGREF != 0
if (module->string_literal_ptrs)
return false;
#endif
#if WASM_ENABLE_BULK_MEMORY != 0
for (i = 0; i < module->data_seg_count; i++)
if (!bh_bitmap_get_bit(
((WASMModuleInstance *)module_inst)->e->common.data_dropped,
i))
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

are you assuming this module instance is the only one which is based on the underlying buffer?
it seems wrong because our instantiation api allows to instantiate multiple instances from a single wasm_module_t.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, I think that was a simplification, but as you say it doesn't work if there are multiple instances created from the same module. We should be able to remove that check if we clone the data segments into the module, if I understand correctly.

return false;
#endif
wenyongh marked this conversation as resolved.
Show resolved Hide resolved
}
#endif
#if WASM_ENABLE_AOT != 0
if (module->module_type == Wasm_Module_AoT) {
is_freeable = ((AOTModule *)module)->is_binary_freeable;
}
#endif /* WASM_ENABLE_INTERP != 0 */
#if WASM_ENABLE_AOT != 0
if (module_inst->module_type == Wasm_Module_AoT) {
AOTModule *module =
(AOTModule *)((AOTModuleInstance *)module_inst)->module;
if (!module->is_binary_freeable)
return false;
#if WASM_ENABLE_GC != 0 && WASM_ENABLE_STRINGREF != 0
if (module->string_literal_ptrs)
return false;
#endif
}
#endif /* WASM_ENABLE_AOT != 0 */

return is_freeable;
(void)i;
return true;
}
wenyongh marked this conversation as resolved.
Show resolved Hide resolved
3 changes: 2 additions & 1 deletion core/iwasm/common/wasm_runtime_common.h
Original file line number Diff line number Diff line change
Expand Up @@ -1197,7 +1197,8 @@ wasm_runtime_detect_native_stack_overflow_size(WASMExecEnv *exec_env,
uint32 requested_size);

WASM_RUNTIME_API_EXTERN bool
wasm_runtime_is_underlying_binary_freeable(const wasm_module_t module);
wasm_runtime_is_underlying_binary_freeable(
const wasm_module_inst_t module_inst);

#if WASM_ENABLE_LINUX_PERF != 0
bool
Expand Down
2 changes: 1 addition & 1 deletion core/iwasm/include/wasm_c_api.h
Original file line number Diff line number Diff line change
Expand Up @@ -562,7 +562,7 @@ WASM_API_EXTERN void wasm_shared_module_delete(own wasm_shared_module_t*);
WASM_API_EXTERN bool wasm_module_set_name(wasm_module_t*, const char* name);
WASM_API_EXTERN const char *wasm_module_get_name(wasm_module_t*);

WASM_API_EXTERN bool wasm_module_is_underlying_binary_freeable(const wasm_module_t* module);
WASM_API_EXTERN bool wasm_module_is_underlying_binary_freeable(const wasm_module_t *module, const struct wasm_instance_t* instance);


// Function Instances
Expand Down
5 changes: 3 additions & 2 deletions core/iwasm/include/wasm_export.h
Original file line number Diff line number Diff line change
Expand Up @@ -1870,11 +1870,12 @@ wasm_runtime_detect_native_stack_overflow_size(wasm_exec_env_t exec_env,
/**
* Query whether the wasm binary buffer used to create the module can be freed
*
* @param module the target module
* @param module_inst the target module instance
* @return true if the wasm binary buffer can be freed
*/
WASM_RUNTIME_API_EXTERN bool
wasm_runtime_is_underlying_binary_freeable(const wasm_module_t module);
wasm_runtime_is_underlying_binary_freeable(
const wasm_module_inst_t module_inst);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This makes that the binary can be freed only after instantiation, is it better to clone data segment in wasm module and clone string_literal_ptrs in both wasm and aot modules when wasm_binary_freeable is true? So that we can free the input buffer after loading. If it is a little complex, we can help do it after this PR is merged.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, we would need to clone the data_dropped bitmap into the module to avoid using the instance. But, as you say, we can do that in a separate PR

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should be to clone the data segments in wasm file but not to clone the data_dropped bitmap. Anyway, let's do it in another PR.


#ifdef __cplusplus
}
Expand Down
4 changes: 2 additions & 2 deletions doc/memory_tune.md
Original file line number Diff line number Diff line change
Expand Up @@ -30,5 +30,5 @@ Normally there are some methods to tune the memory usage:
- set the app heap size with `wasm_runtime_instantiate`
- use `nostdlib` mode, add `-Wl,--strip-all`: refer to [How to reduce the footprint](./build_wasm_app.md#2-how-to-reduce-the-footprint) of building wasm app for more details
- use XIP mode, refer to [WAMR XIP (Execution In Place) feature introduction](./xip.md) for more details
- when using the Wasm C API in fast interpreter or AOT mode, set `clone_wasm_binary=false` in `LoadArgs` and free the wasm binary buffer (with `wasm_byte_vec_delete`) after module loading; `wasm_module_is_underlying_binary_freeable` can be queried to check if the wasm binary buffer can be safely freed (see [the example](../samples/basic/src/free_buffer_early.c))
- when using the wasm/AOT loader in fast interpreter or AOT mode, set `wasm_binary_freeable=true` in `LoadArgs` and free the wasm binary buffer (with `wasm_byte_vec_delete`) after module loading; `wasm_runtime_is_underlying_binary_freeable` can be queried to check if the wasm binary buffer can be safely freed
- when using the Wasm C API in fast interpreter or AOT mode, set `clone_wasm_binary=false` in `LoadArgs` and free the wasm binary buffer (with `wasm_byte_vec_delete`) after module loading; `wasm_module_is_underlying_binary_freeable` can be queried to check if the wasm binary buffer can be safely freed (see [the example](../samples/basic/src/free_buffer_early.c)); after the buffer is freed, `wasm_runtime_get_custom_section` cannot be called anymore
- when using the wasm/AOT loader in fast interpreter or AOT mode, set `wasm_binary_freeable=true` in `LoadArgs` and free the wasm binary buffer (with `wasm_byte_vec_delete`) after module loading; `wasm_runtime_is_underlying_binary_freeable` can be queried to check if the wasm binary buffer can be safely freed; after the buffer is freed, `wasm_runtime_get_custom_section` cannot be called anymore
14 changes: 7 additions & 7 deletions samples/basic/src/free_buffer_early.c
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ void
my_log(uint32 log_level, const char *file, int line, const char *fmt, ...)
{
char buf[200];
snprintf(buf, sizeof(buf), "[WamrLogger] %s", fmt);
snprintf(buf, sizeof(buf), "[WamrLogger] %s\n", fmt);

va_list ap;
va_start(ap, fmt);
Expand Down Expand Up @@ -89,19 +89,19 @@ main(int argc, char *argv_main[])
goto fail;
}

if (wasm_runtime_is_underlying_binary_freeable(module)) {
printf("Able to free wasm binary buffer.\n");
wasm_runtime_free(buffer);
buffer = NULL;
}

module_inst = wasm_runtime_instantiate(module, stack_size, heap_size,
error_buf, sizeof(error_buf));
if (!module_inst) {
printf("Instantiate wasm module failed. error: %s.\n", error_buf);
goto fail;
}

if (wasm_runtime_is_underlying_binary_freeable(module_inst)) {
printf("Able to free wasm binary buffer.\n");
wasm_runtime_free(buffer);
buffer = NULL;
}

char *args[1] = { "3" };
success = wasm_application_execute_func(module_inst, "mul7", 1, args);
if (!success) {
Expand Down
Loading