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

module instance context api #2460

Closed
yamt opened this issue Aug 14, 2023 · 4 comments
Closed

module instance context api #2460

yamt opened this issue Aug 14, 2023 · 4 comments

Comments

@yamt
Copy link
Collaborator

yamt commented Aug 14, 2023

motivation

some native functions (eg. wasi) need to maintain some execution context. (eg. wasi file table)
for the in-tree code, the relevant logic can be hardcoded in the runtime. (eg. wasm_runtime_set_wasi_ctx, wasm_runtime_destroy_wasi)
however, it would be nice to have some api to maintain such a state a bit more dynamically because:

  • for better modularity
  • allow native functions maintained out-of-tree

proposed api

/*
 * module instance context APIs
 *   wasm_runtime_module_instance_context_key_create
 *   wasm_runtime_module_instance_context_key_destroy
 *   wasm_runtime_module_instance_set_context
 *   wasm_runtime_module_instance_set_context_spread
 *   wasm_runtime_module_instance_get_context
 *
 * This set of APIs is intended to be used by an embedder which provides
 * extra sets of native functions, which need per module instance state
 * and are maintained outside of the WAMR tree.
 *
 * It's modelled after the pthread specific API.
 *
 * wasm_runtime_module_instance_set_context_spread is similar to
 * wasm_runtime_module_instance_set_context, except that
 * wasm_runtime_module_instance_set_context_spread applies the change
 * to all threads in the cluster.
 * It's an undefined behavior if multiple threads in a cluster call
 * wasm_runtime_module_instance_set_context_spread on the same key
 * simultaneously. It's a caller's resposibility to perform necessary
 * serialization if necessary. For example:
 *
 * if (wasm_runtime_module_instance_get_context(inst, key) == NULL) {
 *     newctx = alloc_and_init(...);
 *     lock(some_lock);
 *     if (wasm_runtime_module_instance_get_context(inst, key) == NULL) {
 *         // this thread won the race
 *         wasm_runtime_module_instance_set_context_spread(inst, key, newctx);
 *         newctx = NULL;
 *     }
 *     unlock(some_lock);
 *     if (newctx != NULL) {
 *         // this thread lost the race, free it
 *         cleanup_and_free(newctx);
 *     }
 * }
 *
 * Note: dynamic key create/destroy while instances are live is not
 * implemented as of writing this.
 * it's caller's resposibility to ensure destorying all module instances
 * before calling wasm_runtime_module_instance_context_key_create or
 * wasm_runtime_module_instance_context_key_destroy.
 * otherwise, it's an undefined behavior.
 *
 * Note about threads:
 * - When spawning a thread, the contexts (the pointers given to
 *   wasm_runtime_module_instance_set_context) are copied from the parent
 *   instance.
 * - The destructor is called only on the main instance.
 */

WASM_RUNTIME_API_EXTERN void *
wasm_runtime_module_instance_context_key_create(
    void (*dtor)(wasm_module_inst_t inst, void *ctx));

WASM_RUNTIME_API_EXTERN void
wasm_runtime_module_instance_context_key_destroy(void *key);

WASM_RUNTIME_API_EXTERN void
wasm_runtime_module_instance_set_context(wasm_module_inst_t inst, void *key,
                                         void *ctx);
WASM_RUNTIME_API_EXTERN void
wasm_runtime_module_instance_set_context_spread(wasm_module_inst_t inst, void *key,
                                         void *ctx);
WASM_RUNTIME_API_EXTERN void *
wasm_runtime_module_instance_get_context(wasm_module_inst_t inst, void *key);

proposed implementation

#2436

Alternative 1

make the embedder to maintain the state. it probably works well if the embedder and native functions are provided by the same group.

Alternative 2

use wasm_runtime_set_custom_data.
it would work if only single state is necessary on the system.

it's easy to re-implement wasm_runtime_set_custom_data on the top of the proposed api.
the opposite is difficult.

Alternative 3

(ab?)use externref api with #2455

@lum1n0us
Copy link
Collaborator

Let me share our understanding and please correct me if I was wrong.

image

I though there are two scenarios about when to use the new capability:

  • Transfer stateful per-instance data between Host and Wasm app. Indicate by green lines.
  • Transfer stateful per-instance data between Wasm app and runtime dependent libraries. Indicate by yellow lines.

I believe "alternative 1` is a better option for the first scenario instead of the new feature. technically, "Host* and Wasm app is fully able to transfer data w/o involving runtime.

In second scenario, runtime is a producer instead of the storage. So, the new feature is necessary.

A big concern is how to let users to realize the limitation and to prevent applying the new feature to the first scenarios, passing data between host and wasm app.

@tonibofarull
Copy link
Contributor

@lum1n0us thanks for the comment!
I don't want to get ahead of @yamt about the explanation, but my point of view is the following:

Although we can implement WASI libraries and implement context handling in WAMR, I think it is better to let these extensions handle the context by themselves without the need to modify the code.

For example,

#if WASM_ENABLE_WASI_NN != 0
wasi_nn_destroy(module_inst);
#endif

This could be handled via the proposed API. This way, without needing to modify the WAMR's core code, we could enable wasi-nn through flags at build time or as a native library.

@yamt
Copy link
Collaborator Author

yamt commented Aug 29, 2023

Let me share our understanding and please correct me if I was wrong.

image

I though there are two scenarios about when to use the new capability:

* Transfer stateful per-instance data between _Host_ and _Wasm app_. Indicate by green lines.

by "host", do you mean the embedder like iwasm main.c?

* Transfer stateful per-instance data between _Wasm app_ and _runtime dependent libraries_.  Indicate by yellow lines.

I believe "alternative 1` is a better option for the first scenario instead of the new feature. technically, "Host* and Wasm app is fully able to transfer data w/o involving runtime.

In second scenario, runtime is a producer instead of the storage. So, the new feature is necessary.

i don't understand your "producer" "storage" classification.
can you explain a bit?

i'm not sure what you mean by "transfer" either.
wasm apps don't have a way to access this api directly.

A big concern is how to let users to realize the limitation and to prevent applying the new feature to the first scenarios, passing data between host and wasm app.

what limitation are you talking about?

wenyongh pushed a commit that referenced this issue Sep 7, 2023
Introduce module instance context APIs which can set one or more contexts created
by the embedder for a wasm module instance:
```C
    wasm_runtime_create_context_key
    wasm_runtime_destroy_context_key
    wasm_runtime_set_context
    wasm_runtime_set_context_spread
    wasm_runtime_get_context
```

And make libc-wasi use it and set wasi context as the first context bound to the wasm
module instance.

Also add samples.

Refer to #2460.
@yamt
Copy link
Collaborator Author

yamt commented Nov 22, 2023

closed by #2436

@yamt yamt closed this as completed Nov 22, 2023
victoryang00 pushed a commit to victoryang00/wamr-aot-gc-checkpoint-restore that referenced this issue May 27, 2024
Introduce module instance context APIs which can set one or more contexts created
by the embedder for a wasm module instance:
```C
    wasm_runtime_create_context_key
    wasm_runtime_destroy_context_key
    wasm_runtime_set_context
    wasm_runtime_set_context_spread
    wasm_runtime_get_context
```

And make libc-wasi use it and set wasi context as the first context bound to the wasm
module instance.

Also add samples.

Refer to bytecodealliance#2460.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants