Allow using mmap for shared linear memory if OS_ENABLE_HW_BOUND_CHECK is disabled

[loganek]

For shared memory, the max memory size must be defined in advanced. Re-allocation for growing memory can't be used as it might change the base address, therefore for OS_ENABLE_HW_BOUND_CHECK enabled the memory is mmaped, and if the flag is disabled, the memory is allocated. This change introduces a flag that allows users to use mmap for reserving memory address space even if the OS_ENABLE_HW_BOUND_CHECK is disabled.

[wenyongh]

@loganek we are considering using os_mmap to allocate memory no matter whether OS_ENABLE_HW_BOUND_CHECK is enabled or not, since currently the code of memory instantiation/de-instantiation is very complex and not easy to maintain, and we may implement wasm64 feature later, which will make them more complex. os_mmap should have been implemented in all the platforms, and using it may greatly simplify the code, and for this PR, we don't need to introduce WAMR_ENABLE_SHARED_MEMORY_MMAP. How do you think?

[yamt]

@loganek we are considering using os_mmap to allocate memory no matter whether OS_ENABLE_HW_BOUND_CHECK is enabled or not, since currently the code of memory instantiation/de-instantiation is very complex and not easy to maintain, and we may implement wasm64 feature later, which will make them more complex. os_mmap should have been implemented in all the platforms, and using it may greatly simplify the code, and for this PR, we don't need to introduce WAMR_ENABLE_SHARED_MEMORY_MMAP. How do you think?

how do you plan to implement the realloc case?

[yamt]

This change introduces a flag that allows users to use mmap for reserving memory address space even if the OS_ENABLE_HW_BOUND_CHECK is disabled.

why do you want to use mmap?

[wenyongh]

@loganek we are considering using os_mmap to allocate memory no matter whether OS_ENABLE_HW_BOUND_CHECK is enabled or not, since currently the code of memory instantiation/de-instantiation is very complex and not easy to maintain, and we may implement wasm64 feature later, which will make them more complex. os_mmap should have been implemented in all the platforms, and using it may greatly simplify the code, and for this PR, we don't need to introduce WAMR_ENABLE_SHARED_MEMORY_MMAP. How do you think?

how do you plan to implement the realloc case?

Do you mean realloc memory when memory.grow? Yes, it is a good question, per my understanding, for shared memory, no need to mmap a new memory, just increase the current page count. For non-shared memory, runtime should mmap a new memory, duplicate memory data to it and update the related info, like:

wasm-micro-runtime/core/iwasm/common/wasm_memory.c

Lines 705 to 716 in bb053e3

	if (!(memory_data_new =
	wasm_runtime_realloc(memory_data_old, (uint32)total_size_new))) {
	if (!(memory_data_new = wasm_runtime_malloc((uint32)total_size_new))) {
	ret = false;
	goto return_func;
	}
	if (memory_data_old) {
	bh_memcpy_s(memory_data_new, (uint32)total_size_new,
	memory_data_old, total_size_old);
	wasm_runtime_free(memory_data_old);
	}
	}

[yamt]

@loganek we are considering using os_mmap to allocate memory no matter whether OS_ENABLE_HW_BOUND_CHECK is enabled or not, since currently the code of memory instantiation/de-instantiation is very complex and not easy to maintain, and we may implement wasm64 feature later, which will make them more complex. os_mmap should have been implemented in all the platforms, and using it may greatly simplify the code, and for this PR, we don't need to introduce WAMR_ENABLE_SHARED_MEMORY_MMAP. How do you think?

how do you plan to implement the realloc case?

Do you mean realloc memory when memory.grow? Yes, it is a good question, per my understanding, for shared memory, no need to mmap a new memory, just increase the current page count. For non-shared memory, runtime should mmap a new memory, duplicate memory data to it and update the related info, like:

wasm-micro-runtime/core/iwasm/common/wasm_memory.c

Lines 705 to 716 in bb053e3

	if (!(memory_data_new =
	wasm_runtime_realloc(memory_data_old, (uint32)total_size_new))) {
	if (!(memory_data_new = wasm_runtime_malloc((uint32)total_size_new))) {
	ret = false;
	goto return_func;
	}
	if (memory_data_old) {
	bh_memcpy_s(memory_data_new, (uint32)total_size_new,
	memory_data_old, total_size_old);
	wasm_runtime_free(memory_data_old);
	}
	}

realloc can be far cheaper than malloc+memcpy+free.
i'd suggest to introduce os_mremap, which a platform can use realloc to implement. (and maybe mremap on linux)

[wenyongh]

@loganek we are considering using os_mmap to allocate memory no matter whether OS_ENABLE_HW_BOUND_CHECK is enabled or not, since currently the code of memory instantiation/de-instantiation is very complex and not easy to maintain, and we may implement wasm64 feature later, which will make them more complex. os_mmap should have been implemented in all the platforms, and using it may greatly simplify the code, and for this PR, we don't need to introduce WAMR_ENABLE_SHARED_MEMORY_MMAP. How do you think?

how do you plan to implement the realloc case?

Do you mean realloc memory when memory.grow? Yes, it is a good question, per my understanding, for shared memory, no need to mmap a new memory, just increase the current page count. For non-shared memory, runtime should mmap a new memory, duplicate memory data to it and update the related info, like:

wasm-micro-runtime/core/iwasm/common/wasm_memory.c

Lines 705 to 716 in bb053e3

	if (!(memory_data_new =
	wasm_runtime_realloc(memory_data_old, (uint32)total_size_new))) {
	if (!(memory_data_new = wasm_runtime_malloc((uint32)total_size_new))) {
	ret = false;
	goto return_func;
	}
	if (memory_data_old) {
	bh_memcpy_s(memory_data_new, (uint32)total_size_new,
	memory_data_old, total_size_old);
	wasm_runtime_free(memory_data_old);
	}
	}

realloc can be far cheaper than malloc+memcpy+free. i'd suggest to introduce os_mremap, which a platform can use realloc to implement. (and maybe mremap on linux)

Thanks a lot, it is good.

[loganek]

@wenyongh, I think using mmap in all the cases sounds reasonable, and given that grow isn't that frequent operation, and it always allocates big chunk of memory, that shouldn't impact the performance either. Having said that, I'd suggest we merge this PR and continue the work on simplifications on top of that so our usecase is unblocked. What do you think?

[wenyongh]

@wenyongh, I think using mmap in all the cases sounds reasonable, and given that grow isn't that frequent operation, and it always allocates big chunk of memory, that shouldn't impact the performance either. Having said that, I'd suggest we merge this PR and continue the work on simplifications on top of that so our usecase is unblocked. What do you think?

@loganek ok, it is good to me, let's review the PR and merge it after it is carefully checked.

[loganek]

Thanks, please review it then and let me know if you have any feedback. As a followup, I can work on migrating to mmap completely later this week or next week.

[loganek]

@wenyongh looks like the build failed for unrelated reason - would you be able to restart the build?

[wenyongh]

LGTM with minor comment.

[wenyongh]

Is it better to move wasm_munmap_linear_memory/wasm_mmap_linear_memory from wasm_runtime_common.h/c to wasm_memory.h/c? We normally implement memory related operations (except shared memory) in the latter.

[loganek]

Sure, done.

[loganek]

Actually, I've reverted this change since I realized that set_error_buf is not defined and we'd have to define it for wasm_memory too. Because I'm planning to refactor the code and use mmap everywhere, I'll keep it where it is now and move things around during the refactoring. I hope that's ok.