Align fiber C stack size #7084
Align fiber C stack size #7084
Conversation
Also align the macro names and generally macro definitions are always at the top of the file.
| size = ZEND_FIBER_DEFAULT_C_STACK_SIZE; | ||
| } else if (size < ZEND_FIBER_MIN_C_STACK_SIZE) { | ||
| size = ZEND_FIBER_MIN_C_STACK_SIZE; | ||
| } else if (size > ZEND_FIBER_MAX_C_STACK_SIZE) { |
There was a problem hiding this comment.
Do we actually need a maximum here? Does it pose any issues if exceeded, or is it arbitrary?
There was a problem hiding this comment.
I do not think it's a good idea to set a huge C stack size for fibers, it is usually a mistake to set it a huge value.
Professionals can modify ZEND_FIBER_MAX_C_STACK_SIZE to remove the limitation, it is also a kind of configuration.
| @@ -25,6 +25,15 @@ | |||
|
|
|||
| BEGIN_EXTERN_C() | |||
|
|
|||
| #define ZEND_FIBER_C_STACK_ALIGNMENT (4 * 1024) | |||
| #define ZEND_FIBER_DEFAULT_C_STACK_SIZE (ZEND_FIBER_C_STACK_ALIGNMENT * (sizeof(void *) * 64)) | |||
| #define ZEND_FIBER_MIN_C_STACK_SIZE (128 * 1024) | |||
There was a problem hiding this comment.
Why this as a minimum stack size. ZEND_FIBER_C_STACK_ALIGNMENT * (ZEND_FIBER_GUARD_PAGES + 1) should work, so why require more if the user wishes to set it to the minimum?
There was a problem hiding this comment.
After we rely on the virtual memory mechanism, too small a C stack size does not seem to make any sense, it will only increase the risk of the crash.
Professionals can still remove the limitation by changing the macro definition.
Also, align the macro names.
Generally, macro definitions are always at the top of the file, it's easier to edit.