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

implementation of wasm shared heap #3543

Closed
wants to merge 7 commits into from

Conversation

WenLY1
Copy link
Contributor

@WenLY1 WenLY1 commented Jun 18, 2024

  • add WAMR_BUILD_SHARED_HEAP and WASM_ENABLE_SHARED_HEAP

  • add wamrc --enable-shared-heap

  • consider the behavior of import memory in multi-module feature

  • adapt address conversion/validation
    aot code boundary check
    wasm_runtime_invoke_native
    wasm_runtime_invoke_native_raw
    wasm_runtime_validate_app_addr
    wasm_runtime_addr_app_to_native
    wasm_runtime_addr_native_to_app
    wasm_runtime_validate_native_addr

  • interpreter/fast-jit do boundary check
    classic interp mode
    fast interp mode
    jit

  • Support setting shared heap’s size and shared heap’s alloc options in RuntimeInitArgs for wasm_runtime_full_init

  • Add check in load module, default memory’s max_memory_size should be no larger than 4G-shared_heap_size, if not, reduce it to 4G-shared_heap_size

  • Add API wasm_runtime_shared_malloc/wasm_runtime_shared_free

  • App allocates memory from shared heap with API shared_malloc/shared_free
    example

            #include <stdio.h>  
            #include <stdlib.h>  
            extern void *shared_malloc(int size);  
            extern void shared_free(void *ptr);  
            int main()  
            {  
	            int *pa = (int *)malloc(4);  
	            *pa = 4;  
	            printf("pa value is %d\n", *pa);  
	            free(pa);  
	            int *ps = (int *)shared_malloc(4);  
	            *ps = 5;  
	            printf("ps value is %d\n", *ps);  
	            shared_free(ps);  
            } 

#if WASM_ENABLE_SHARED_HEAP != 0
REG_NATIVE_FUNC(shared_malloc, "(i)i"),
REG_NATIVE_FUNC(shared_free, "(*)"),
#endif
Copy link
Collaborator

Choose a reason for hiding this comment

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

i guess this should be a separate lib, not a part of libc_builtin.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Got


module_inst->shared_heap->size = shared_heap_size;

if (!mem_allocator_create_with_struct_and_pool(
Copy link
Collaborator

Choose a reason for hiding this comment

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

doesn't this allocator put metadata on the heap itself?
if so, i guess it isn't appropriate for shared heap.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The heap is inited by module_inst->shared_heap->data, and in future implementation, module_inst->shared_heap->data is to be shared between multi modules

Copy link
Collaborator

Choose a reason for hiding this comment

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

i meant that, if metadata is stored on the shared heap itself, any modules accessible to the heap can easily corrupt the metadata.
is it ok for your use cases?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Because mem_allocator_create_with_struct_and_pool rather than mem_allocator_create is used to create a allocator, the metadata of allocator is separated from the heap

@WenLY1 WenLY1 marked this pull request as draft June 20, 2024 12:27
@wenyongh
Copy link
Contributor

@WenLY1 thanks for uploading the PR, I have created branch dev/shared_heap as discussed, please submit the PR to that branch instead.

@wenyongh
Copy link
Contributor

wenyongh commented Oct 9, 2024

Close this PR as another PR is under review instead: #3823.

@wenyongh wenyongh closed this Oct 9, 2024
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

Successfully merging this pull request may close these issues.

3 participants