- Notifications
You must be signed in to change notification settings - Fork 2k
feat: additional opcache setting in check php.ini #9032
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
Conversation
| I have concerns.
|
| Probably a simple mention of |
| This command is mainly used to check the PHP settings of production environments.
|
| @ddevsr If you want to show opcache detailed settings, how about adding an argument? |
| 👋 Hi, @ddevsr! |
0079494 to 5418869 Compare 21e7965 to 36698fc Compare $ php spark phpini:check CodeIgniter v4.5.4 Command Line Tool - Server Time: 2024-08-22 09:50:55 UTC+00:00 +-------------------------+---------------------------------------+---------------------------------------+-------------+---------------------------------------+ | Directive | Global | Current | Recommended | Remark | +-------------------------+---------------------------------------+---------------------------------------+-------------+---------------------------------------+ | error_reporting | 32767 | 32767 | 5111 | | | display_errors | 1 | 1 | 0 | | | display_startup_errors | 1 | 1 | 0 | | | log_errors | 1 | 1 | | | | error_log | D:/Project/laragon/tmp/php_errors.log | D:/Project/laragon/tmp/php_errors.log | | | | default_charset | UTF-8 | UTF-8 | UTF-8 | | | max_execution_time | 0 | 0 | | The default is 30. | | memory_limit | 1024M | 1024M | | > post_max_size | | post_max_size | 2G | 2G | | > upload_max_filesize | | upload_max_filesize | 2G | 2G | | < post_max_size | | max_input_vars | 1000 | 1000 | | The default is 1000. | | request_order | GP | GP | GP | | | variables_order | GPCS | GPCS | GPCS | | | date.timezone | Asia/Jakarta | Asia/Jakarta | UTC | | | mbstring.language | neutral | neutral | neutral | | | opcache.enable | 0 | 0 | 1 | | | opcache.enable_cli | 1 | 1 | 1 | | | opcache.jit | tracing | tracing | tracing | | | opcache.jit_buffer_size | 128M | 128M | 256 | Adjust with your free space of memory | | zend.assertions | 1 | 1 | -1 | | +-------------------------+---------------------------------------+---------------------------------------+-------------+---------------------------------------+$ php spark phpini:check opcache CodeIgniter v4.5.4 Command Line Tool - Server Time: 2024-08-22 09:52:13 UTC+00:00 +---------------------------------+---------+---------+-------------+----------------------------------------------------------------------------------+ | Directive | Global | Current | Recommended | Remark | +---------------------------------+---------+---------+-------------+----------------------------------------------------------------------------------+ | opcache.enable | 0 | 0 | 1 | | | opcache.enable_cli | 1 | 1 | 1 | | | opcache.jit | tracing | tracing | tracing | | | opcache.jit_buffer_size | 128M | 128M | 256 | Adjust with your free space of memory | | opcache.memory_consumption | 728 | 728 | 512 | Adjust with your free space of memory | | opcache.interned_strings_buffer | 64 | 64 | 64 | | | opcache.max_accelerated_files | 58000 | 58000 | 40000 | Find many files in your project (example: find your_project/ -iname *.php|wc -l) | | opcache.max_wasted_percentage | 15 | 15 | 15 | | | opcache.validate_timestamps | 0 | 0 | 0 | | | opcache.revalidate_freq | 0 | 0 | 0 | | | opcache.save_comments | 0 | 0 | 0 | | +---------------------------------+---------+---------+-------------+----------------------------------------------------------------------------------+ |
| This looks good, however, I wonder if we should keep all 4 opcache keys in the default table. IMO only the first 2 will be relevant now. |
kenjis left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for updating!
But I don't agree with the recommendation values.
I will see the values.
system/Security/CheckPhpIni.php Outdated
| if ($argument === 'opcache') { | ||
| $items = [ | ||
| 'opcache.enable' => ['recommended' => '1'], | ||
| 'opcache.enable_cli' => ['recommended' => '1'], |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why opcache.enable_cli is 1?
In my understanding, it does not improve performance unless you set opcache.file_cache correctly.
https://stackoverflow.com/questions/77778860/proper-opcache-configuration-for-cli
| Recommendations by ChatGPT CodeIgniter 4 Assistant When deploying CodeIgniter 4 (CI4) in a production environment, configuring PHP’s OPcache properly is essential to ensure optimal performance. Below are the recommended values for the specified OPcache settings that should be good for most CI4 users. However, certain values might need adjustment based on the specific needs of your application and server resources. Recommended PHP ini settings for OPcache in a Production Environment
Additional Considerations:
|
| Recommendations by Claude 3.5 Sonnet As a PHP expert, I can provide recommended values for the PHP ini settings you've listed, specifically tailored for a CodeIgniter 4 production environment. These recommendations should be suitable for most CI4 users, but keep in mind that optimal values can vary depending on your specific application and server resources. Let me explain each setting:
These values provide a good starting point for a CodeIgniter 4 production environment. However, you may need to adjust them based on your specific application requirements, server resources, and performance needs. Regular monitoring and tuning can help you find the optimal settings for your particular use case. |
system/Security/CheckPhpIni.php Outdated
| 'opcache.max_wasted_percentage' => ['recommended' => '15'], | ||
| 'opcache.validate_timestamps' => ['recommended' => '0'], | ||
| 'opcache.revalidate_freq' => ['recommended' => '0'], | ||
| 'opcache.save_comments' => ['recommended' => '0'], |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Setting opcache.save_comments to 0 is dangerous. Some packages may use doc comments.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Already test smooth for my project. For recomendation production perspective, i think can disabled in production with remark notes
"require": { "php": "^8.1", "chillerlan/php-qrcode": "^5.0", "codeigniter4/framework": "^4.5.3", "firebase/php-jwt": "^6.10", "mpdf/mpdf": "^8.2" }, system/Security/CheckPhpIni.php Outdated
| $items = [ | ||
| 'opcache.enable' => ['recommended' => '1'], | ||
| 'opcache.enable_cli' => ['recommended' => '1'], | ||
| 'opcache.jit' => ['recommended' => 'tracing'], |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is it safe to recommend to enable jit?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not safe if using third-party ext like xdebug.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Based on other and my benchmark, JIT have benefit significant. Moreover, in PHP8.4 a new, better JIT has been implemented
Ref :
Co-authored-by: kenjis <kenji.uui@gmail.com>
Co-authored-by: kenjis <kenji.uui@gmail.com>
| I guess there is no consensus on recommended values? |
samsonasik left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we can accept it, and improve recommended values as we go later ;)
michalsn left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Seems reasonable, although some sort of changelog entry would be nice.
paulbalandan left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Approved, pending changelog entry.
michalsn left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I will add a changelog entry after merging this in a separate PR.
Description
I think this additional needed to recommendation.
Checklist: