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

Remove transitivity of component PEXes #12688

Closed
stuhood opened this issue Aug 27, 2021 · 0 comments · Fixed by #12808
Closed

Remove transitivity of component PEXes #12688

stuhood opened this issue Aug 27, 2021 · 0 comments · Fixed by #12808
Assignees
Labels

Comments

@stuhood
Copy link
Member

stuhood commented Aug 27, 2021

To fix #12548, we moved to constructing thirdparty PEXes from a lockfile/constraints file as multiple components composed using the PEX path. But as described in #12548 (comment), we currently build each of the component PEXes as transitive due to pex-tool/pex#1423.

This is safe, but redundant. If the individual PEXes could be built with --no-transitive, they would be approximately 40% of their current size.

@stuhood stuhood added the bug label Aug 27, 2021
stuhood added a commit that referenced this issue Aug 28, 2021
As described in #12548 (and further clarified in #12548 (comment)), when we extract subsets of a `repository.pex` (which is itself built from a lockfile or constraints file), each of the subsets is likely to be highly redundant. And because the subsets cannot be structure/content shared by the CAS, the result is an increase of storage usage proportional to the number of distinct subsets. Concretely: in some repositories, we observed a ~45X blowup of storage due to ~90 different subsets being constructed.

To fix this, this change moves to constructing each `requirements.pex` subset of a `repository.pex` from a PEX_PATH containing one PEX per root requirement. As implemented here, the storage blowup is reduced to ~4.4X, but as mentioned in #12688, these "single-requirement" PEXes are currently transitive, and it's possible that this could be fixed in the future. 

An alternative to this change would have been to remove the "subsetting" step entirely, and to directly use the entire `repository.pex` in consumers (even if they only used a small fraction of it). But that would have the disadvantage that any change to any requirement would invalidate all consumers in a repository, regardless of whether the requirement was relevant to them, and would additionally violate hermeticity by allowing, for example, `test`s to observe resources that they don't have dependencies on. 

Fixes #12548.

[ci skip-rust]
@jsirois jsirois self-assigned this Sep 9, 2021
@benjyw benjyw removed the in-progress label Sep 9, 2021
jsirois added a commit that referenced this issue Sep 9, 2021
Pex 2.1.48 brings `--layout {packed,loose}` alternate layouts for PEXes.
These are both more friendly to remote caching, leading to smaller
artifacts to cache and greater cache hit ratios in the face of
requirement changes. Since the loose layout still does not perform well
with the local CAS scheme, we use packed for now.

Fixes #12548
Fixes #12688
Fixes #12803
stuhood pushed a commit to stuhood/pants that referenced this issue Sep 9, 2021
Pex 2.1.48 brings `--layout {packed,loose}` alternate layouts for PEXes.
These are both more friendly to remote caching, leading to smaller
artifacts to cache and greater cache hit ratios in the face of
requirement changes. Since the loose layout still does not perform well
with the local CAS scheme, we use packed for now.

Fixes pantsbuild#12548
Fixes pantsbuild#12688
Fixes pantsbuild#12803

[ci skip-rust]

[ci skip-build-wheels]
jsirois added a commit to jsirois/pants that referenced this issue Oct 1, 2021
Pex 2.1.48 brings `--layout {packed,loose}` alternate layouts for PEXes.
These are both more friendly to remote caching, leading to smaller
artifacts to cache and greater cache hit ratios in the face of
requirement changes. Since the loose layout still does not perform well
with the local CAS scheme, we use packed for now.

Fixes pantsbuild#12548
Fixes pantsbuild#12688
Fixes pantsbuild#12803

(cherry picked from commit 433a4dd)

# Rust tests and lints will be skipped. Delete if not intended.
[ci skip-rust]

# Building wheels and fs_util will be skipped. Delete if not intended.
[ci skip-build-wheels]
jsirois added a commit that referenced this issue Oct 1, 2021
…3a4dd 0d36002 7ba06a5)  (#13078)

This is a three-part-cherry-pick:

1. Upgrade to Pex 2.1.48 and leverage packed layout. (#12808)

Pex 2.1.48 brings `--layout {packed,loose}` alternate layouts for PEXes.
These are both more friendly to remote caching, leading to smaller
artifacts to cache and greater cache hit ratios in the face of
requirement changes. Since the loose layout still does not perform well
with the local CAS scheme, we use packed for now.

Fixes #12548
Fixes #12688
Fixes #12803

(cherry picked from commit 433a4dd)

2. Upgrade to Pex 2.1.49. (#12853) 

This fixes a bug activating previously `--not-zip-safe` dependency-only
PEXes, which Pants used extensively before the Pex 2.1.48 upgrade.

(cherry picked from commit 0d36002)

3. Upgrade to Pex 2.1.50. (#12888)

This fixes a bug executing PEX zipapps that do not have the execute bit
set, which is the case for the Pex PEX we download and run in the
release script.

(cherry picked from commit 7ba06a5)

With a fourth commit that applies `./build-support/bin/generate_all_lockfiles.sh`.

Fixes #13075
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants