wamr: bump the default stack guard size #2241
Conversation
|
@no1wudi how do you think? |
LGTM, and maybe we should notify user that using thread, since default thread on nuttx is 2048 for many platforms. |
| config INTERPRETERS_WAMR_STACK_GUARD_SIZE | ||
| int "Custom stack guard size" | ||
| default 0 | ||
| default 1024 |
There was a problem hiding this comment.
Is it good to resolve 1KB for stack guard by default?
There was a problem hiding this comment.
what do you mean by "resolve" in this context?
There was a problem hiding this comment.
typo I mean reserve
There was a problem hiding this comment.
1KB might be a bit too large for typical nuttx targets.
but it's definitely safer than 0.
also, 1KB is what wamr uses by default for other platforms.
There was a problem hiding this comment.
I assume that the stack size consumed by wamr function is fixed, why wasm engine can't catch the stack overflow before jump to the new function?
There was a problem hiding this comment.
because your assumption is wrong?
at least in the current wamr aot implementation, a caller doesn't have precise knowledge about how much stack the callee consumes. the callees need to check the overflow by itself and the check itself might consume stack. (thus the small amount of stack is reserved here.)
There was a problem hiding this comment.
But 1KB isn't a small amount of stack.
using 0 by default seems a bit too aggressive to me. 1024 here is what would have been used before this kconfig knob was introduced by apache#1259. It also matches some expectations in the recent wamrc. https://github.com/bytecodealliance/wasm-micro-runtime/blob/40b430fd240ad45f5c7a7dd0239fa2425e7c294e/core/iwasm/compilation/aot_emit_aot_file.c#L2672-L2678
yamt
force-pushed
the
wamr-stack-guard
branch
from
415d714 to
4d2eea3
Compare
i added some description in the help text. |
| the majority of other platforms, including Linux. It also matches | ||
| the expections in the WAMR AoT compiler. | ||
| On the other hand, it might be a bit larger than what's strictly | ||
| necessary, especially for typical NuttX targets. |
There was a problem hiding this comment.
so, why not give a small value match the nuttx normal case?
There was a problem hiding this comment.
just because it's difficult to fine tune this kind of things.
| config INTERPRETERS_WAMR_STACK_GUARD_SIZE | ||
| int "Custom stack guard size" | ||
| default 0 | ||
| default 1024 |
There was a problem hiding this comment.
But 1KB isn't a small amount of stack.
4 participants
Summary
using 0 by default seems a bit too aggressive to me.
1024 here is what would have been used before this kconfig knob was introduced by #1259. It also matches some expectations in the recent wamrc. https://github.com/bytecodealliance/wasm-micro-runtime/blob/40b430fd240ad45f5c7a7dd0239fa2425e7c294e/core/iwasm/compilation/aot_emit_aot_file.c#L2672-L2678
Impact
Testing