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

Use symlinks to add dists in the Pex CLI. #1185

Conversation

jsirois
Copy link
Member

@jsirois jsirois commented Jan 21, 2021

This avoids copying dists from ~/.pex/installed_wheels to a PEXBuilder
tmpdir before finally zipping up the tmpdir. Instead, with just a single
symlink added to the PEXBuilder tmpdir for each dist, the zip is created
directly from the sources under ~/.pex/installed_wheels providing a
modest speed up when building large PEX files.

This avoids copying dists from ~/.pex/installed_wheels to a PEXBuilder
tmpdir before finally zipping up the tmpdir. Instead, with just a single
symlink added to the PEXBuilder tmpdir for each dist, the zip is created
directly from the sources under ~/.pex/installed_wheels providing a
modest speed up when building large PEX files.
@jsirois
Copy link
Member Author

jsirois commented Jan 21, 2021

For tc.pex which has 114 distributions and weighs in at 48M - making it a moderate sized PEX - I found:

$ hyperfine --prepare 'true' --prepare 'git stash pop || true' 'python -mpex -r ~/dev/toolchainlabs/toolchain/3rdparty/python/requirements.txt --constraints ~/dev/toolchainlabs/toolchain/3rdparty/python/constraints.txt -o tc-via-pex-repository.pex --pex-repository tc.pex' 'python -mpex -v -r ~/dev/toolchainlabs/toolchain/3rdparty/python/requirements.txt --constraints ~/dev/toolchainlabs/toolchain/3rdparty/python/constraints.txt -o tc-via-pex-repository-symlinked.pex --pex-repository tc.pex'
Benchmark #1: python -mpex -r ~/dev/toolchainlabs/toolchain/3rdparty/python/requirements.txt --constraints ~/dev/toolchainlabs/toolchain/3rdparty/python/constraints.txt -o tc-via-pex-repository.pex --pex-repository tc.pex
  Time (mean ± σ):      9.622 s ±  0.093 s    [User: 8.466 s, System: 1.066 s]
  Range (min … max):    9.525 s …  9.774 s    10 runs
 
Benchmark #2: python -mpex -v -r ~/dev/toolchainlabs/toolchain/3rdparty/python/requirements.txt --constraints ~/dev/toolchainlabs/toolchain/3rdparty/python/constraints.txt -o tc-via-pex-repository-symlinked.pex --pex-repository tc.pex
  Time (mean ± σ):      8.139 s ±  0.084 s    [User: 7.635 s, System: 0.469 s]
  Range (min … max):    8.047 s …  8.290 s    10 runs
 
Summary
  'python -mpex -v -r ~/dev/toolchainlabs/toolchain/3rdparty/python/requirements.txt --constraints ~/dev/toolchainlabs/toolchain/3rdparty/python/constraints.txt -o tc-via-pex-repository-symlinked.pex --pex-repository tc.pex' ran
    1.18 ± 0.02 times faster than 'python -mpex -r ~/dev/toolchainlabs/toolchain/3rdparty/python/requirements.txt --constraints ~/dev/toolchainlabs/toolchain/3rdparty/python/constraints.txt -o tc-via-pex-repository.pex --pex-repository tc.pex'

@@ -402,9 +441,14 @@ def add_distribution(self, dist, dist_name=None):
this will be inferred from the distribution itself should it be formatted in a standard way.
:type dist: :class:`pkg_resources.Distribution`
"""
if dist.location in self._distributions:
Copy link
Member Author

Choose a reason for hiding this comment

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

The new SYMLINK copy_mode brought to light the fact we were doing over-writes here when populating the PEXBuilder with the results of multi-target resolves whenever a distribution applied to more than one target (say a universal wheel). This was a harmless time-waste previously.

@jsirois jsirois mentioned this pull request Jan 21, 2021
11 tasks
Copy link
Contributor

@Eric-Arellano Eric-Arellano left a comment

Choose a reason for hiding this comment

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

Thanks!

@jsirois jsirois merged commit 552b926 into pex-tool:master Jan 21, 2021
@jsirois jsirois deleted the pex_builder/chroot/symlink_wheel_install_chroots branch January 21, 2021 17:39
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.

2 participants