Skip to content

Update docs for PHP Opcache use#1717

Merged
yosifkit merged 7 commits intodocker-library:masterfrom
jandom:improve/php-opcache-docs
May 22, 2020
Merged

Update docs for PHP Opcache use#1717
yosifkit merged 7 commits intodocker-library:masterfrom
jandom:improve/php-opcache-docs

Conversation

@jandom
Copy link
Contributor

@jandom jandom commented May 14, 2020

  • otherwise massive perf penalty
  • tested against bare metal EC2

jandom added 4 commits May 14, 2020 16:59
- otherwise massive perf penalty
- tested against bare metal EC2
@tianon
Copy link
Member

tianon commented May 14, 2020

Thanks for the contribution!

In reading this, it feels like we're spending a lot of time duplicating common PHP documentation instead of something that's actually Docker-specific. Perhaps we could instead leave this as a very short hint somewhere and point to the official upstream documentation instead?

@jandom
Copy link
Contributor Author

jandom commented May 14, 2020

Oh hi, wow, thanks for having a look so quickly!

Absolutely, happy to follow your lead on this – the example is very verbose – just let me know how to best present it as a hit? Should I scrap the example and replace it with some documentation links?

Just to set the context for how this came about. I moved a large-ish production workload from "bare-metal" PHP ElasticBeanstalk to a multicontainer Docker ElasticBeanstalk and saw a pretty big performance drop (requests would take 0.6s rather than expected 0.3s).

Then went that rabbit hole and solved my problem, so wanted to spare people some time ("no, it's not the instance type being too small, not it's not an esoteric symfony configuration")

@jandom
Copy link
Contributor Author

jandom commented May 18, 2020

hi there @tianon – i've truncated the contribution a bit, pointing to some useful blogs instead of including a verbose opcache example, let me know if that's closer to what you wanted?

@tianon
Copy link
Member

tianon commented May 21, 2020

Frankly, I was expecting something even more terse, because IMO opcache isn't special/unique enough to warrant a separate section from the already-included "PHP Core Extensions" section which this is duplicating. I'd probably add something like this in the "Configuration" section that already talks about production usage:

In many production environments, it is also recommended to (build and) enable the PHP core OPcache extension for performance. See the upstream OPcache documentation for more details.

In many production environments, it is also recommended to (build and) enable the PHP core OPcache extension for performance.  See [the upstream OPcache documentation](https://www.php.net/manual/en/book.opcache.php) for more details.

@jandom
Copy link
Contributor Author

jandom commented May 21, 2020

Perfect, updated the wording to match what you said – it's now just a paragraph below the configuration section. Should be ready to go 🤞

Copy link
Member

@tianon tianon left a comment

Choose a reason for hiding this comment

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

Thanks for the contribution and for hanging with me while we worked this out 👍

@jandom
Copy link
Contributor Author

jandom commented May 22, 2020

@tianon no, thank you for the review – this doc will hopefully save people a ton of time (it would have saved me) because managed platforms (usually) ship with the extension auto-enabled

@yosifkit yosifkit merged commit 5b34dbb into docker-library:master May 22, 2020
RUN mv "$PHP_INI_DIR/php.ini-production" "$PHP_INI_DIR/php.ini"

# Override with custom opcache settings
COPY config/opcache.ini $PHP_INI_DIR/conf.d/
Copy link
Contributor

Choose a reason for hiding this comment

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

@yosifkit Why was this removed?

The conf.d-directory is a custom scan-dir, so it is not duplicating common PHP documentation.
https://github.com/docker-library/php/blob/master/7.4/alpine3.12/fpm/Dockerfile#L125

Without this info it is kind of hard to know were to put custom config files, that's why the info was there.

Copy link
Member

Choose a reason for hiding this comment

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

See a few lines up:

The default config can be customized by copying configuration files into the $PHP_INI_DIR/conf.d/ directory.

Copy link
Contributor

Choose a reason for hiding this comment

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

Thanks for the hint! (I must be blind 🤦‍♂️)

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.

4 participants