search

salesagility / SuiteCRM

SuiteCRM - Open source CRM for the world
https://www.suitecrm.com
GNU Affero General Public License v3.0
4.26k stars 2.03k forks source link

Fix #10427 - APC does not work for SuiteCRM 7.14, PHP8 #10428

Open gunnicom opened 1 month ago

gunnicom commented 1 month ago

See https://github.com/salesagility/SuiteCRM/issues/10427

SuiteBot commented 1 month ago

This pull request has been mentioned on SuiteCRM. There might be relevant details there:

https://community.suitecrm.com/t/apc-cache-not-working/69261/2

simonr44 commented 1 month ago

Hi Gunnicom,

Thanks for contributing to SuiteCRM.

I have tested your changes and they work well.

Having looked at the code, there are some related changes that may be worthwhile.

The check && ini_get('apc.stat') == 0 may no longer be required on the above also?

In addition it may be worth lowering the priority in include/SugarCache/SugarCacheAPC.php line 49 Changing protected $_priority = 940; to protected $_priority = 890; will ensure APCu is prioritised over other caches that may be available but not fully configured such as memcached.

gunnicom commented 1 month ago

@simonr44 At first I thought about the file include/SugarCache/SugarCache.php, too, but on second look I assumed, that that is only for OPcache part of APC, that vanished long ago. So I decided to not include that here. The real solution would be to remove that part completely.

simonr44 commented 1 month ago

Agreed yes APCu no longer includes opcode caches, however clearing the APCu cache when this function is called may still have benefits. Perhaps the function name is no longer too relevant but it may still have value in one form or another.

simonr44 commented 1 month ago

On further inspection apc_delete_file looks obsolete now, so 157 - 159 could go?

apcu_clear_cache() may still be worth calling in cleanOpcodes()

gunnicom commented 1 month ago

@simonr44 That could all be discussed in a different issue/PR. Keep this PR to fixing APCu functionality, and leave cleanup of old obsolete APC OPcache to a different PR.

simonr44 commented 1 month ago

There is a good chance we would look to bring these changes in together and test together so we'll make a new PR to cover those.

Thanks again for your contribution.