docker icon indicating copy to clipboard operation
docker copied to clipboard

fix: opcache configuration

Open adripo opened this issue 1 year ago • 9 comments

fix #2184

replica of https://github.com/nextcloud/all-in-one/pull/2762

adripo avatar Mar 21 '24 04:03 adripo

As stated in the AIO issue, I would not raise OPcache size and interned strings buffer, unless there is really a case where this is required. Reducing JIT to 8 MiB alone should solve the issue.

MichaIng avatar Mar 29 '24 15:03 MichaIng

@MichaIng @J0WI I tried to start a discussion on AIO repo before your review for the correct JIT value, but no further analysis has been done on this topic. If you want to participate here is the link. It would be nice to have the same configurations in Docker and AIO solution.

https://github.com/nextcloud/all-in-one/discussions/4416

adripo avatar Apr 03 '24 17:04 adripo

I think the solution done here, as well as in AIO, is fine. Since the JIT cache size seems to pre-allocate memory from overall OPcache (same as interned strings buffer does), it should fix the warning.

My idea to generate an image with JIT disabled completely was more for testing purpose, to see whether it has the same larger than expected impact on OPcache buffer usage that I observed on my test system, strengthening my guess that JIT uses in fact more OPcache buffer than what one configures with opcache.jit_buffer_size (and what usage stats report back). I find such hidden memory usage weird and it makes it more difficult to find right values other than trial&error. So in this case, I would try to get some clarification on this in PHP bug tracker.

AIO uses doubled OPcache size now. On x86_64, this is not such a problem (compared to embedded devices, ARM SBCs and such), and the image comes with quite a number of additional apps, also larger ones like Nextcloud Office, so it makes sense cover possible edge cases with high OPcache usage, for Docker containers where changing such configs is not so easy/common.

For this lighter Nextcloud Docker image, it should be fine to leave the overall OPcache size at 128M (default). It can be raised later in case there appear really cases, where it is still insufficient.

MichaIng avatar Apr 03 '24 19:04 MichaIng

I agree. Let's fix the alert first and do other fixes if necessary in the future.

adripo avatar Apr 07 '24 00:04 adripo

I'm not fully up to speed on this topic, but to help nudge this along:

In general we try to keep this image set aligned with what's recommended in the official Nextcloud Admin Manual for a baseline deployment.

My concern here is that we're going off book.

Maybe we work on clarifying / updating the upstream docs first: https://docs.nextcloud.com/server/latest/admin_manual/installation/server_tuning.html#enable-php-opcache

After all part of the problem seems to be that the docs lack clarity in this area.

AIO is sort of a different creature. There things can be tuned based on fewer unknowns (it has a specific way of doing things, is designed for a specific type of deployment, and the pieces it is commonly deployed with are easier to predict).

This image, however, isn't like that.

If nothing else, let's push an update of the upstream docs in parallel. After all, you all have already done the mental work of figuring out some values that apparently make more sense as baseline defaults. The benefits should not be limited to only this Docker image. Let's make things smoother for others as well. :-)

joshtrichards avatar May 17 '24 18:05 joshtrichards

Maybe we work on clarifying / updating the upstream docs first: https://docs.nextcloud.com/server/latest/admin_manual/installation/server_tuning.html#enable-php-opcache

Absolutely. Looks like someone threw inside a number without checking back anything (no offence 😄). I think I mentioned it elsewhere already, that additionally it makes sense to mention that JIT is x86_64-only, not supported on ARM, RISC-V or anything else.

MichaIng avatar May 17 '24 18:05 MichaIng

@MichaIng https://github.com/nextcloud/documentation/pull/11872/files#r1638279688 didn't update the recommend jit_buffer_size. Are you still planning to update that in the docs?

J0WI avatar Oct 08 '24 13:10 J0WI

I'll do when I find time. Kinda busy recently, though it is a small change only.

MichaIng avatar Oct 08 '24 13:10 MichaIng

@J0WI PR is up: https://github.com/nextcloud/documentation/pull/12284

MichaIng avatar Oct 14 '24 14:10 MichaIng