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

POC: Enable extensions + staged build #33

Merged
merged 6 commits into from
Oct 19, 2022
Merged

Conversation

withinboredom
Copy link
Collaborator

@withinboredom withinboredom commented Oct 17, 2022

The first stage builds PHP, taking into account some "best practices" from the official image and is built on the official image. It is a stand-in for the official image that is yet to exist (PHP 8.3).

The second stage uses the first stage and copies in the golang build environment from the official golang image. It then builds frankenphp.

The final stage combines the output from the first and second stages from the official PHP image. Once PHP 8.3 is released, this step will just be copying the output from the second stage instead of combining first & second stage.

Copy link
Owner

@dunglas dunglas left a comment

Choose a reason for hiding this comment

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

Thank you very much for working on that.

Dockerfile Outdated Show resolved Hide resolved
Dockerfile Outdated Show resolved Hide resolved
Dockerfile Outdated Show resolved Hide resolved
Dockerfile Outdated Show resolved Hide resolved
Dockerfile Show resolved Hide resolved
Dockerfile Outdated Show resolved Hide resolved
Dockerfile Outdated Show resolved Hide resolved
Dockerfile Outdated Show resolved Hide resolved
Dockerfile Outdated Show resolved Hide resolved
Dockerfile Show resolved Hide resolved
Dockerfile Outdated Show resolved Hide resolved
Dockerfile Outdated Show resolved Hide resolved
@alexander-schranz
Copy link
Contributor

alexander-schranz commented Oct 17, 2022

This is the docker image which is currently working for me using:

FROM php:8.2.0RC4-zts-bullseye AS php-base

ENV PHP_URL="https://github.com/dunglas/php-src/archive/refs/heads/frankenphp-8.2.tar.gz"
ENV PHP_ASC_URL=""
ENV PHP_SHA256=""

# ...

https://github.com/alexander-schranz/frankenphp/blob/feature/php-extensions/Dockerfile

@dunglas
Copy link
Owner

dunglas commented Oct 18, 2022

Super smart trick!

@stefanpoensgen
Copy link

This is the docker image which is currently working for me using:

FROM php:8.2.0RC4-zts-bullseye AS php-base

ENV PHP_URL="https://github.com/dunglas/php-src/archive/refs/heads/frankenphp-8.2.tar.gz"
ENV PHP_ASC_URL=""
ENV PHP_SHA256=""

# ...

https://github.com/alexander-schranz/frankenphp/blob/feature/php-extensions/Dockerfile

@alexander-schranz
I tested your Dockerfile and at first everything looks good to me, but is it possible that opcache has no impact? Although it is active.

@alexander-schranz
Copy link
Contributor

@stefanpoensgen in the /_profiler/phpinfo it was active for me. Maybe I forget something in the php.ini maybe you can check that.

@stefanpoensgen
Copy link

It is active indeed. But if I look at "Symfony initialization" and "Peak memory usage" in the profiler, it is 4 times higher than with my usual caddy+phpfpm setup. These are pretty much the values I get when I disable opcache :D
So my first thought was that opcache maybe has no impact at all.

@alexander-schranz
Copy link
Contributor

alexander-schranz commented Oct 18, 2022

Maybe @dunglas has a hint here. I could not do a performance test yet. As I only uses none Docker PHP runner in other cases, because Docker is really slow on my machine (mac optimization can not be enabled docker freezes then).
So todo a performance test I would need to first compile FrankenPHP locally and not in Docker to test it against e.g. PHP-FPM or Roadrunner. Are you using caddy+phpfpm also in Docker?

@withinboredom
Copy link
Collaborator Author

Updated the Dockerfile, now builds extensions just fine. I've included the install-php-extensions script for testing in the final stage. So please test:

docker build -t test .
docker run -p 80:80 -p 443:443 -it --rm --entrypoint bash test
# install-php-extensions redis
# frankenphp run --config /etc/Caddyfile

Then verify the extension is present when going to https://localhost

@withinboredom
Copy link
Collaborator Author

withinboredom commented Oct 18, 2022

So my first thought was that opcache maybe has no impact at all.

Opcache needs to be installed/enabled separately with this change. We can enable it again, though sometimes opcache is not desired (for example, when building an image with xdebug enabled).

Dockerfile Outdated Show resolved Hide resolved
@withinboredom
Copy link
Collaborator Author

I've added a sed to the final image to support running one-off commands better.

@alexander-schranz
Copy link
Contributor

@stefanpoensgen you are correct the opcache is not working as it seems sadly not use the correct tar.gz. I could not yet found out why there are ENV vars if it is no not possible to modify them via the ENV statement.

@withinboredom
Copy link
Collaborator Author

withinboredom commented Oct 18, 2022

you are correct the opcache is not working as it seems sadly not use the correct tar.gz. I could not yet found out why there are ENV vars if it is no not possible to modify them via the ENV statement.

@alexander-schranz, this comment doesn't seem to be related to this PR, but if you're talking about this comment then it is because that env var is only used during the initial build and is never used again. Further, this wouldn't work because none of the scripts expect a .tar.gz, they expect a tar.xz which is a totally different (and incompatible) compression algorithm (-P vs -z with tar).

@dunglas dunglas merged commit 2890aae into dunglas:main Oct 19, 2022
@dunglas
Copy link
Owner

dunglas commented Oct 19, 2022

Thank you @withinboredom! Great work

@alexander-schranz
Copy link
Contributor

🎉 @withinboredom great work 👍

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