Skip to content

Comments

Prevent unnecessarily frequent default expunge operations#588

Open
madmajestro wants to merge 1 commit intokrakjoe:masterfrom
madmajestro:prevent-repeated-default-expunge-operations
Open

Prevent unnecessarily frequent default expunge operations#588
madmajestro wants to merge 1 commit intokrakjoe:masterfrom
madmajestro:prevent-repeated-default-expunge-operations

Conversation

@madmajestro
Copy link
Contributor

@madmajestro madmajestro commented Aug 15, 2025

A full cache wipe is now performed if too little SHM has been freed by removing expired entries during the default expunge operation. This is intended to prevent unnecessarily frequent calls of the default expunge operation, which should reduce cpu load in some cases. The new configuration setting apc.expunge_threshold defaults to 5 (0.5%) and specifies the thousandth of SHM that must be available after expired entries have been removed during the default expunge operation. Since apc.expunge_threshold replaces apc.smart, apc.smart has been removed.

@madmajestro
Copy link
Contributor Author

This is my suggestion for reducing high CPU usage caused by frequently executed default expunge operations.

In my tests, this seemed effective. The number of cleanups and defragmentations decreased, while the number of expunges remained constant.

php_apc.c Outdated
STD_PHP_INI_ENTRY("apc.gc_ttl", "3600", PHP_INI_SYSTEM, OnUpdateLong, gc_ttl, zend_apcu_globals, apcu_globals)
STD_PHP_INI_ENTRY("apc.ttl", "0", PHP_INI_SYSTEM, OnUpdateLong, ttl, zend_apcu_globals, apcu_globals)
STD_PHP_INI_ENTRY("apc.smart", "0", PHP_INI_SYSTEM, OnUpdateLong, smart, zend_apcu_globals, apcu_globals)
STD_PHP_INI_ENTRY("apc.smart", "1", PHP_INI_SYSTEM, OnUpdateLong, smart, zend_apcu_globals, apcu_globals)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Perhaps it would be better to remove apc.smart and use a different name like apc.expunge_percentage, instead of changing the behavior of apc.smart again. Which do you think is better?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Probably it's best to not use apc.smart. Nobody really understands what this undocumented option does...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, I think so too. Do you have any concerns about removing apc.smart completely and replacing it with a setting called apc.expunge_threshold?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed.

  • I decided to use apc.expunge_threshold as a new configuration setting and removed apc.smart.
  • apc.expunge_threshold is specified in thousandths of SHM that must be free.
  • The default value is 5 (0.5%), as this prevented all worst-case scenarios during my test runs.

@madmajestro
Copy link
Contributor Author

Even if it works well with 1 %, it would also be reasonable to use thousendth instead of percent. If the general concept is ok, i could do some additional testing + tuning with values lower than 1 %.

@madmajestro
Copy link
Contributor Author

@nikic
Friendly ping.

We should take care of this PR first, as it affects the test results of #589. If this is merged, I'd like to do some more testing / fine-tuning with #589 before it is merged.

@madmajestro madmajestro force-pushed the prevent-repeated-default-expunge-operations branch from ec5ebc9 to 6dce289 Compare August 21, 2025 18:04
@madmajestro madmajestro force-pushed the prevent-repeated-default-expunge-operations branch 3 times, most recently from 413d821 to fc927ca Compare September 4, 2025 13:40
Copy link
Collaborator

@nikic nikic left a comment

Choose a reason for hiding this comment

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

Having the option specify the value in thousands is a bit weird. Might it make more sense to make this a floating point value?

@madmajestro madmajestro force-pushed the prevent-repeated-default-expunge-operations branch from fc927ca to d9744bd Compare September 24, 2025 20:53
@madmajestro
Copy link
Contributor Author

Fixed. Yes, that's a good idea. It's easier to understand and more flexible.

php_apc.c Outdated
return FAILURE;
}

double *p = (double *) ZEND_INI_GET_ADDR();
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why does this use a different approach (ZEND_INI_GET_ADDR instead of using APCG) than the other ini settings?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Changed.

By using ZEND_INI_GET_ADDR, the OnUpdatePercentage function was also be usable for other INI settings that must be specified as a percentage. However, since there is currently no other INI setting that requires a percentage, I've removed the use of ZEND_INI_GET_ADDR. As soon as there are multiple percentage values, we can consider using ZEND_INI_GET_ADDR again.

@madmajestro madmajestro force-pushed the prevent-repeated-default-expunge-operations branch 2 times, most recently from e4b458a to 727b662 Compare October 18, 2025 19:44
A full cache wipe is now performed if too little SHM has been freed by
removing expired entries during the default expunge operation. This is
intended to prevent unnecessarily frequent calls of the default expunge
operation, which should reduce cpu load in some cases. The new
configuration setting apc.expunge_threshold defaults to 0.5 (0.5%) and
specifies the percentage of SHM that must be available after expired
entries have been removed during the default expunge operation. Since
apc.expunge_threshold replaces apc.smart, apc.smart has been removed.
@madmajestro madmajestro force-pushed the prevent-repeated-default-expunge-operations branch from 727b662 to 84af7d5 Compare October 18, 2025 19:46
Copy link
Collaborator

@nikic nikic left a comment

Choose a reason for hiding this comment

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

LGTM

@nikic
Copy link
Collaborator

nikic commented Dec 6, 2025

I wonder whether it would make sense to create a release from the current master branch before merging this.

@madmajestro
Copy link
Contributor Author

I wonder whether it would make sense to create a release from the current master branch before merging this.

I agree. It probably wouldn't hurt, but this is the safer approach.

But please take a look at #606 before creating a release.

@madmajestro
Copy link
Contributor Author

@nikic
I just saw that you released version 5.1.28 on pecl, but the release isn't available on GitHub yet…

@nikic
Copy link
Collaborator

nikic commented Dec 8, 2025

@nikic I just saw that you released version 5.1.28 on pecl, but the release isn't available on GitHub yet…

Oops, fixed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants