Skip to content
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

Closes #27094. #27301

Closed
wants to merge 2 commits into from
Closed

Closes #27094. #27301

wants to merge 2 commits into from

Conversation

Linuxfabrik
Copy link

Signed-off-by: Markus Frei markus.frei@linuxfabrik.ch
Closes #27094.

Signed-off-by: Markus Frei <markus.frei@linuxfabrik.ch>
@MichaIng
Copy link
Member

MichaIng commented Jun 8, 2021

Ah sorry, I should have seen this earlier. Please see my PR #27403 which changes the recommendations to be based on actual OPcache usage. So only when the cache size / max keys is used over 90%, a recommendation is shown to increase the value.

Good find with the opcache.save_comments setting. My PR for this reason only checks it, when OPcache is disabled, so when recommending to enable OPcache, it should then in case recommended as well to set opcache.save_comments=1, as enabling OPcache only otherwise would break Nextcloud, as you correctly found.

Also good about the set of prime numbers for max accelerated files. I was thinking to explicitly recommend the next higher one of these, to avoid users raising the value a little without any effect. But for now, instead I think this is something for the docs.

Interesting that in your case 16 MiB interned strings buffer was required. In my case 8 = 6 MiB is sufficient, usage currently is 3.56 MiB only. This shows that it seems to depend on use case and also on PHP version, I guess, which is a good argument to base recommendations on actual usage.

@szaimen szaimen added the 3. to review Waiting for reviews label Aug 31, 2021
@szaimen szaimen added this to the Nextcloud 23 milestone Aug 31, 2021
@skjnldsv skjnldsv mentioned this pull request Oct 13, 2021
@skjnldsv
Copy link
Member

Closing in favour of #27403

@skjnldsv skjnldsv closed this Oct 22, 2021
@MichaIng MichaIng removed this from the Nextcloud 23 milestone Dec 20, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
3. to review Waiting for reviews
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Make "The PHP OPcache is not properly configured" a little bit more tolerant and precise (Suggestion)
5 participants