Skip to content

Add configuration opcache.jit_max_trace_length #11173

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

Merged
merged 1 commit into from
May 2, 2023

Conversation

wxue1
Copy link
Contributor

@wxue1 wxue1 commented May 2, 2023

Max length of a single trace. A long trace generates long JITTed code, which influences the performance slightly.
opcache.jit_max_trace_length range is [4,1024], the default value is 1024 now.

Signed-off-by: Wang, Xue [email protected]
Reviewed-by: Su, Tao [email protected]

Copy link
Member

@dstogov dstogov left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is almost ready.
The minimal changes are required.

@@ -336,6 +349,7 @@ ZEND_INI_BEGIN()
STD_PHP_INI_ENTRY("opcache.jit_max_recursive_calls" , "2", PHP_INI_ALL, OnUpdateUnrollC, max_recursive_calls, zend_jit_globals, jit_globals)
STD_PHP_INI_ENTRY("opcache.jit_max_recursive_returns" , "2", PHP_INI_ALL, OnUpdateUnrollR, max_recursive_returns, zend_jit_globals, jit_globals)
STD_PHP_INI_ENTRY("opcache.jit_max_polymorphic_calls" , "2", PHP_INI_ALL, OnUpdateLong, max_polymorphic_calls, zend_jit_globals, jit_globals)
STD_PHP_INI_ENTRY("opcache.jit_max_trace_length" , "512", PHP_INI_ALL, OnUpdateUnrollT, max_trace_length, zend_jit_globals, jit_globals)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please keep the old default value "1024" to keep the original behaviuor.

@@ -258,6 +258,19 @@ static ZEND_INI_MH(OnUpdateUnrollL)
ZEND_JIT_TRACE_MAX_LOOPS_UNROLL);
return FAILURE;
}

static ZEND_INI_MH(OnUpdateUnrollT)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please, use a better name - OnUpdateMaxTraceLength

@@ -7591,7 +7591,7 @@ int ZEND_FASTCALL zend_jit_trace_hot_root(zend_execute_data *execute_data, const
zend_jit_op_array_trace_extension *jit_extension;
size_t offset;
uint32_t trace_num;
zend_jit_trace_rec trace_buffer[ZEND_JIT_TRACE_MAX_LENGTH];
zend_jit_trace_rec *trace_buffer = (zend_jit_trace_rec *)malloc(JIT_G(max_trace_length) * sizeof(zend_jit_trace_rec));
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Keep the preallocated trace_buffer. Your patch limits the upper bound anyway.
Note: In case you swittch to malloc() you need to free() the buffer later.
Anyway, it's easier and more efficient to keep the preallocated buffer.

zend_jit_trace_rec trace_buffer[ZEND_JIT_TRACE_MAX_LENGTH];
zend_jit_trace_rec *trace_buffer = (zend_jit_trace_rec *)malloc(JIT_G(max_trace_length) * sizeof(zend_jit_trace_rec));
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The same.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The same.

Thanks, all are done

@wxue1 wxue1 force-pushed the jit_max_trace_length branch from 3a3ee01 to 86788cb Compare May 2, 2023 11:54
Max length of a single trace. A long trace generates long JITTed
code, which influences the performance slightly.
opcache.jit_max_trace_length range is [4,1024], the default value
is 1024.

Signed-off-by: Wang, Xue <[email protected]>
Reviewed-by:   Su, Tao   <[email protected]>
@wxue1 wxue1 force-pushed the jit_max_trace_length branch from 86788cb to 1d66114 Compare May 2, 2023 12:00
@dstogov dstogov merged commit 0e5ac62 into php:master May 2, 2023
llaville added a commit to llaville/php-compatinfo-db that referenced this pull request Oct 10, 2023
llaville added a commit to llaville/php-compatinfo-db that referenced this pull request Nov 18, 2023
llaville added a commit to llaville/php-compatinfo-db that referenced this pull request Nov 23, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants