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

about memory grow problem #119

Closed
GuoLei1990 opened this issue Oct 17, 2019 · 14 comments
Closed

about memory grow problem #119

GuoLei1990 opened this issue Oct 17, 2019 · 14 comments

Comments

@GuoLei1990
Copy link

GuoLei1990 commented Oct 17, 2019

I have a memory problem!but I don't know create issues in which project. maybe is llvm or wasi. I use llvm and wasi-sdk compile wasm.
the problem is I init wasm lib version with code
var mem = new WebAssembly.Memory({ 'initial': initialMemory}); fetch("laya.physics3D.wasm.wasm").then((response) => { response.arrayBuffer().then((buffer) => { WebAssembly.instantiate(buffer, { wasi_unstable: { }, env: { memory: mem, } }).then((physics3D) => { }); }); });
I compile the wasm with "--import-memory" by llvm ,but the initial memory not really work, wasm still grow the memory base the initial memory end.
for example I run a sample need 7M(or pages) wasm memory, if initial memory is 2M the memory in chrome is 2M+7M=9M,if the initialMemory is 100M the memory in chrome is 100M+7M=107M. in fact wasm ignore the initial memory and waste. so I must initial mininum size 2 pages with new WebAssembly.Memory({ 'initial': 2}); to avoid waste,but this is not I want. because this will grow memory many times,this is very slow,and especially when convert it to js version,it's very very slow,but i think the problem is in llvm or wasi. If I can sucess give a big memory to wasm, and wasm can use this memory first form start( not always grow from this memory end),will
save many performance,do not need to grow memoy many times,for my project(it's a 3D engine) the initial memory size must be a config write by developer according to their game, this is good for performance( avoid grow memory many times). before i use emsdk to compile wasm, don't have this problem. I hope this is my mistake cause!

@mikevoronov
Copy link

mikevoronov commented Oct 17, 2019

Hi!

It is highly likely that the underlying problem is described here. I will make PR with fix today or tomorrow.

P.S. Some more details in addition: WASI dlmalloc start by invoking sys_alloc. In the first call sys_alloc determines the memory base by invoking sbrk(0). This call returns end of the initial memory. So, memory between __heap_end and initial couldn't be used by dlmalloc now.

@sbc100
Copy link
Member

sbc100 commented Oct 17, 2019

Is there a reason you need to --import-memory? Currently WASI dictates that memory be exported. At least for now. I'm curious why you would need to import it?

@GuoLei1990
Copy link
Author

GuoLei1990 commented Oct 18, 2019

@michaelvoronov you are very good! I understand now,so WASI is designed like what I describe now,and you will imporve and fix this problem tomorrow. Thank you very much,your fix is very important for me.

@GuoLei1990
Copy link
Author

GuoLei1990 commented Oct 18, 2019

@sbc100 Thank you for reply,the key reason is performance.

Environment:My Project is a 3D game engine ,the source is javascript,but the physics module is wasm,the developer use engine will adjust the physics initial memory by a config param to optimization performance(for example,A game need 16M at least,B game need 32M,they will config the initial memory to avoid memory growth times),my physics module has compile into wasm or js lib already,so I must use new WebAssembly.Memory({ 'initial': initialMemory} to import memory with initialMemory config.

Performance:
1:So if initial memory can't work, wasm will grow the memory many times,I think this is slow("but i think wasi have optimization to this.Maybe i am wrong,I just think this can improve in theory").
2:But the js version(convert by wasm2js tool) must be can improve performance ,in chrome performance tool I can see __wasm_memory_grow function is very very slow and call many times,I think the times of this function call is same with wasm version,so if wasm solve,the js version can solve.and now I must need js version,because some platform like apple's JSCore can't run wasm version.
3:Before I use emsdk to compile wasm,emsdk support this and can avoid memory grow. now i use llvm+wasi,the file size and performance is good(except memory grow problem).
4:I think it's good for both wasm version and js version if support initial memory import.

@sbc100
Copy link
Member

sbc100 commented Oct 18, 2019

OK that sounds like reasonable use case. Perhaps we should open a separate issue to support imported memory, since WASI doesn't support that today.

@GuoLei1990
Copy link
Author

@sbc100 Thank you very much! I hope you can support import memory👍,I will continuous attention this problem.

@tlively
Copy link
Member

tlively commented Oct 19, 2019

Would explicitly growing the memory to its configured size on startup be a simpler way to solve this problem? It sounds like the performance problem would be growing the memory many times, but surely growing it just once at the beginning would be ok.

@GuoLei1990
Copy link
Author

GuoLei1990 commented Oct 19, 2019

@tlively Yes! this is a flexible way to solve problems, I try memory.grow API before,but still can't solve this problem. Do you know another API can achieve this goal what you say?

@tlively
Copy link
Member

tlively commented Oct 19, 2019

The only other way to grow the memory I know of is by the clang builtin function __builtin_wasm_grow_memory, but that should be identical to using the JS API. I suspect that the allocator doesn’t know how to use extra space that it does not request itself, but I don’t know for sure. Perhaps @sunfishcode knows whether this is expected to work.

@mikevoronov
Copy link

mikevoronov commented Oct 20, 2019

@GuoLei1990 You could try WebAssembly/wasi-libc#114. I've already tested it on SQLite.

About performance of importing memory - I am dealing mostly with standalone Wasm execution environments like wasmtime and wasmer and for them you could set initial memory pages in wasm binary for exported memory. And then during instantiation they set the initial position to value from the wasm file. I believe that browsers should work exactly the same (e.g. v8 seems so).

So, you could not use --import-memory and just set limits in wasm file - it seems that with this dlmalloc fix you will get the behaviour you want.

P.S. To set limits you could convert wasm file to wat, write a correct x to the (memory (;0;) x) S-expression and then convert it back to wasm.

@GuoLei1990
Copy link
Author

@michaelvoronov its really work,Great!now the behaviour is what I want,the performance has improve now. Thank you and everyone relpy me,you are good people.

btw,what your means of "set limits in wasm file",you means lld port --initial-memory?

@mikevoronov
Copy link

mikevoronov commented Oct 21, 2019

I meant that instead of using importing memory you could rather set memory limits in your compiled wasm file. According to the spec, each wasm file could define initial and maximum memory pages. So, you could set it by hands in your compiled file. E.g. by following way (with wabt installed) for test.wasm file:

  1. Translate wasm to wat by wasm2wat:
wasm2wat test.wasm -o test.wat
  1. Open test.wat with some text editor and find "(memory (;0;) x)" (without quotes), where x - is a concrete number that depends on your program (note, that wat is just a special text Webassembly format). And x is also represent initial memory (in Wasm pages). You can replace it with your number (also you could set the maximum page count here).

  2. Then translate wat to wasm back:

wat2wasm test.wat -o test_modified.wasm

@GuoLei1990
Copy link
Author

@michaelvoronov thank you very much! I understand.

@sunfishcode
Copy link
Member

The original bug here seems to be fixed, in WASI libc, as noted above. If there's anything further to discuss here, feel free to re-open this or file a new issue!

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

5 participants