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

Speed up Python docker builds using pre-compiled python #7928

Closed
wants to merge 1 commit into from

Conversation

jeffwidman
Copy link
Member

@jeffwidman jeffwidman commented Aug 30, 2023

Our Python docker builds are slow... a typical CI test run for Python
takes ~21 minutes, of which the first ~10 minutes are spent building the
image.

This results in slower local development, slow CI test suites, and slow deployments.

The main culprit is pyenv install which under the covers downloads the
python source and then compiles it locally. Profiling showed that the
download was quick, so even though pyenv supports aria2c, there's
not much to be gained there. Unfortunately, a quick look at the pyenv
issue tracker showed there's no way to to pass pre-compiled artifacts to
pyenv
.

For a long time we've bandied about the idea of switching from pyenv
to downloading pre-compiled Pythons. In fact, the GitHub Actions
publishes versions that we could use here:
https://github.com/actions/python-versions/releases

However, we use pyenv local + pyenv exec throughout our Ruby code
for switching to different Python versions. So we thought that it'd take
a week or more to fully migrate away from pyenv.

Today I had to rebuild the python image multiple times, and got so
annoyed that I decided to poke at it a bit.

It turns out that pyenv is simply a shim layer, and as long as
/usr/local/.pyenv/versions/<x.y.z>/bin exists, it will happily pass
commands to anything in that folder.

So I was able to come up with an intermediate solution that speeds the
builds up drastically without requiring a large code refactor.

Running this locally results in the Python download/install/build
step going from ~500 seconds all the way down to ~75 seconds, a savings
of 7 minutes. Given that a full CI run of the python test suite
previously took ~21 minutes, this cuts it by 1/3.

Related but pulling the pre-compiled Python from a different source:

@jeffwidman jeffwidman requested a review from a team as a code owner August 30, 2023 09:47
@jeffwidman
Copy link
Member Author

jeffwidman commented Aug 30, 2023

Copy link
Member Author

@jeffwidman jeffwidman left a comment

Choose a reason for hiding this comment

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

Annotating the changes.

@@ -16,6 +16,8 @@ USER root

# Install *only* the apt packages required for this builder image to build Python.
# C-libs needed by users to build their Python packages should be installed down below in the final docker image.
# TODO: not all these packages may be needed now that we've switched from `pyenv install` which compiled from source to
# downloading / copying pre-compiled python
Copy link
Member Author

Choose a reason for hiding this comment

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

There's likely further speed/space improvements possibly by dropping some of these packages, but I'd rather do that in a separate PR after letting this one prove itself in production.

Copy link
Member Author

Choose a reason for hiding this comment

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

Thinking on this more, pre-compiled won't need these packages, but that there's a decent chance other packages that users end up having to build in native C as part of walking their dep tree would need some of these...

But in that case, we should move them below to the "list of packages we install so users can build their native-C-based deps".

Again, probably not something to touch in this PR, but instead in a follow-on PR once I've validated the initial approach in prod.

&& rm -rf /var/lib/apt/lists/*

COPY --chown=dependabot:dependabot python/helpers /opt/python/helpers
USER root
# TODO: Now that switched from `pyenv install` which compiled from source to downloading / copying a pre-compiled python
# we could entirely drop pyenv if we change our ruby code that calls `pyenv exec` to track which version of python to
# call and uses the full python paths.
Copy link
Member Author

Choose a reason for hiding this comment

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

A quick grep of pyenv commands showed this actually should be pretty straightforward, since we already have to keep track of when to tell pyenv to switch to a different python version.

Dropping pyenv completely would simplify the Dockerfile and be one less dep we pull in. But again, rather handle that in a separate PR after letting this one marinate a bit.

&& cd /usr/local/.pyenv \
&& tar czf 3.8.tar.gz versions/$PY_3_8
&& cd /opt/hostedtoolcache/Python \
&& tar czf $PY_3_8.tar.gz $PY_3_8
Copy link
Member Author

Choose a reason for hiding this comment

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

These are tar'd for space reasons... Since this changes the compile source, I double-checked the size, and each one still averaged ~0.5GB uncompressed. Since majority of our runs use the default python (currently 3.11), it's still worth compressing the non-default versions.

&& cd /usr/local/.pyenv \
&& tar czf 3.10.tar.gz versions/$PY_3_10
&& cd /opt/hostedtoolcache/Python \
&& tar czf $PY_3_10.tar.gz $PY_3_10
Copy link
Member Author

Choose a reason for hiding this comment

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

I hate the lack of DRY across all these, but AFAIK there's no clean way to loop something like this in a multi-stage docker build.

I didn't duplicate the comments across each one... decided that doing it solely on the default 3.11 install sufficed.

# pyenv expects the python installation files in the `versions` folder, but the pre-compiled python3 / pip3
# expect to reside in the /opt/hostedtoolcache/Python/x.y.z/x64 dir, so need a symlink to make them play nice.
SharedHelpers.run_shell_command(
"ln -s /opt/hostedtoolcache/Python/#{python_version}/x64 /usr/local/.pyenv/versions/#{python_version}"
Copy link
Member Author

Choose a reason for hiding this comment

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

This is the one place where it may be more painful to swap architectures, since I'm not sure how we communicate to the Ruby code that it needs x64 vs arm64? We can worry about that later though...

# The pre-compiled Python expects to be installed to this dir
RUN mkdir -p /opt/hostedtoolcache/Python/$PY_3_11/x64/ \
&& cd /opt/hostedtoolcache/Python/$PY_3_11/x64/ \
# TODO: Add support for arm64 on Ubuntu whenever actions/python-versions adds support for it. Currently not available.
Copy link
Member Author

Choose a reason for hiding this comment

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

I expect support for this to be added at some point, eg this upstream PR:

At that point, it'll be easy here in the Dockerfile to make the x64 vs arm64 value dynamic using TARGETARCH...

Copy link
Member

Choose a reason for hiding this comment

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

I can't build this locally on the M1 chip. Did you consider copying out the built Python from the official Docker images like how we're installing Ruby? That way we get the architectures for free without extra scripting, should simplify this part.

Copy link
Member Author

Choose a reason for hiding this comment

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

I actually did consider it, but for some reason thought that we were trying to move away from copying from official docker images... but a little digging and I see it's the opposite and the Ruby flip was to move away from apt-get towards docker images.

It's a great idea, let me try it.

Copy link
Member Author

Choose a reason for hiding this comment

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

Turned out to be pretty straightforward:

And it's also a bit cleaner as it allowed copying the files directly into /usr/local/.pyenv/versions, no need for any confusing symlinks.

Our Python docker builds are slow... a typical CI test run for Python
takes ~21 minutes, of which the first ~10 minutes are spent building the
image.

This results in slower local development, slow CI test suites, and slow deployments.

The main culprit is `pyenv install` which under the covers downloads the
python source and then compiles it locally. Profiling showed that the
download was quick, so even though `pyenv` supports `aria2c`, there's
not much to be gained there. Unfortunately, a quick look at the `pyenv`
issue tracker showed [there's no way to to pass pre-compiled artifacts to
`pyenv`](https://github.com/orgs/pyenv/discussions/1872).

For a long time we've bandied about the idea of switching from `pyenv`
to downloading pre-compiled Pythons. In fact, the GitHub Actions
publishes versions that we could use here:
https://github.com/actions/python-versions/releases

However, we use `pyenv local` + `pyenv exec` throughout our Ruby code
for switching to different Python versions. So we thought that it'd take
a week or more to fully migrate away from `pyenv`.

Today I had to rebuild the python image multiple times, and got so
annoyed that I decided to poke at it a bit.

It turns out that `pyenv` is simply a shim layer, and as long as
`/usr/local/.pyenv/versions/<x.y.z>/bin` exists, it will happily pass
commands to anything in that folder.

So I was able to come up with an intermediate solution that speeds the
builds up drastically without requiring a large code refactor.

Running this locally results in the Python download/install/build
step going from ~500 seconds all the way down to ~75 seconds, a savings
of 7 minutes. Given that a full CI run of the python test suite
previously took ~21 minutes, this cuts it by 1/3.
@jeffwidman
Copy link
Member Author

Closing in favor of #7934 which is a very similar approach, but pulls the sources from the official docker image... benefits of that approach:

  • Builds are faster by about 15 seconds for some reason
  • Cleaner because the linking to the C-libs uses more relative links somehow, so I can fully copy it into /usr/local/.pyenv/versions so avoids confusing symlinks
  • Supports arm out of the box

@jeffwidman jeffwidman closed this Aug 31, 2023
@jeffwidman jeffwidman deleted the download-pre-built-python branch August 31, 2023 00:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants