-
Notifications
You must be signed in to change notification settings - Fork 646
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
add hashmap to get_const_offset #4010
Conversation
This change has worse performance than HEAD for small modules, but it is a substantial improvement for modules with large numbers of constants (I saw a 18% speedup in loading a module with 10,000 consts). Ideas:
|
@sjamesr thanks for your effort to improve the performance, I haven't read the detailed code to understand the optimization, but it seems that a 18% speedup is not as expected. I tried to improve the perpformance with another PR (see #4012), and tested the standalone case in cd tests/standalone/test-printf
time iwasm --heap-size=102400 --stack-size=8096000 test_printf_builtin.wasm
# or modify the source code to print the execution time of wasm_loader_load manually I also tested you patch, but it didn't improve the loading time for the test-print case, instead, it is even worse. So I think maybe my PR is better? And could you test my PR for your case? Thanks in advance. |
This does indeed show a substantial improvement in our benchmark, a 15% speedup in loading a module with ~1000 consts. After this, the largest single cost in our benchmark is Thank you for doing this! My change adds a bh_hash_map of |
OK, welcome, I refactored some code and applied similar changes to wasm mini loader. If you are to drop this PR, then I will let others help review my PR. For reading leb in wasm loader of fast interpreter, one optimization I can think is that we don't need to check the leb format again in the second scan since the checks have been done in the first scan. I also submitted another PR (see #4017) for it, but it doesn't improve performance a lot for the case test-printf. Maybe you can have a try for your case. Another optimization is to refine the |
No description provided.