Skip to content

opcache.interned_strings_buffer either has no effect or opcache_get_status() / phpinfo() is wrong #11715

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

Closed
gndk opened this issue Jul 16, 2023 · 3 comments

Comments

@gndk
Copy link

gndk commented Jul 16, 2023

Description

In my php.ini I have set opcache.interned_strings_buffer to 16.

phpinfo() confirms this:

image

But opcache_get_status()...

image

and phpinfo()...

image

... show interned_strings_usage['buffer_size'] as full at ~6mb.

I don't know if it's relevant, but opcache has enough free memory:

image

But the used_memory and free_memory also add up to a different value than the 1024 I set for opcache.memory_consumption, although close.

PHP Version

8.2.8

Operating System

Ubuntu 20.04

@nielsdos
Copy link
Member

nielsdos commented Jul 16, 2023

I just tried this and got a different, but equally weird result.
I got 193752 used, 12388696 free. Sum that up and you get 12582448, which is just under 12MiB, less than the 16 I set it at 🤔

EDIT: I don't understand this code:

ZCSG(interned_strings).end =
(zend_string*)((char*)accel_shared_globals +
ZCG(accel_directives).interned_strings_buffer * 1024 * 1024);

I would've expected this instead:

		ZCSG(interned_strings).end =
			(zend_string*)((char*)ZCSG(interned_strings).start +
				ZCG(accel_directives).interned_strings_buffer * 1024 * 1024);

i.e. to compute the end, instead of starting from the start of the struct, we start at the interned string table start. This makes more sense to me. And this yields the right result on my computer. But I'm not sure, I didn't look too deep into it yet as I'm out of time for today.

EDIT 2: Wait but then this line is also wrong:

accel_shared_globals_size = ZCG(accel_directives).interned_strings_buffer * 1024 * 1024;

I'm confused

@gndk
Copy link
Author

gndk commented Jul 17, 2023

Glad you could confirm!

I'm confused

Well, sorry to add to the confusion, but I actually had it set to 256 before (which I then realised was much too high), but even with that it still was stuck on the 6mb from my screenshot..

@nielsdos
Copy link
Member

I think I figured it out and will try to create a PR soon.
There's two issues I think: the end calculation is wrong, and the memory usage/free reporting is using the wrong values.
I think if I'm right, it makes sense you're seeing that 6MB behaviour.

nielsdos added a commit to nielsdos/php-src that referenced this issue Jul 17, 2023
… or opcache_get_status() / phpinfo() is wrong

There are a couple of oddities.

1) The interned strings buffer comprises the whole hashtable
   datastructure.
   Therefore, it seems that the interned strings buffer size is the size of
   only said table. However, in the current code it also includes the size
   of the zend_accel_shared_globals.

2) ZCSG(interned_strings).end is computed starting from the accelerator
   globals struct itself. I would expect it to start from the part where
   the interned strings table starts.

3) When computing the used size, it is done using
   ZCSG(interned_strings).end - ZCSG(interned_strings).start. However,
   this does not include the uin32_t slots array because
   ZCSG(interned_strings).start pointers after that array.

This patch corrrects these 3 points.
nielsdos added a commit to nielsdos/php-src that referenced this issue Jul 17, 2023
… or opcache_get_status() / phpinfo() is wrong

There are a couple of oddities.

1) The interned strings buffer comprises the whole hashtable
   datastructure.
   Therefore, it seems that the interned strings buffer size is the size of
   only said table. However, in the current code it also includes the size
   of the zend_accel_shared_globals.

2) ZCSG(interned_strings).end is computed starting from the accelerator
   globals struct itself. I would expect it to start from the part where
   the interned strings table starts.

3) When computing the used size, it is done using
   ZCSG(interned_strings).end - ZCSG(interned_strings).start. However,
   this does not include the uin32_t slots array because
   ZCSG(interned_strings).start pointers after that array.

This patch corrrects these 3 points.
nielsdos added a commit that referenced this issue Jul 21, 2023
* PHP-8.1:
  Fix GH-11715: opcache.interned_strings_buffer either has no effect or opcache_get_status() / phpinfo() is wrong
nielsdos added a commit that referenced this issue Jul 21, 2023
* PHP-8.2:
  Fix GH-11715: opcache.interned_strings_buffer either has no effect or opcache_get_status() / phpinfo() is wrong
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants