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

Pex PEX perf regression #1054

Closed
jsirois opened this issue Oct 3, 2020 · 1 comment · Fixed by #1056
Closed

Pex PEX perf regression #1054

jsirois opened this issue Oct 3, 2020 · 1 comment · Fixed by #1056

Comments

@jsirois
Copy link
Member

jsirois commented Oct 3, 2020

Pex ships a PEX of itself for those who wish to download a Pex binary instead of pip installing it. Pants is an important user of the Pex PEX and f07ae47 (via git bisect) introduced a performance regression that induces ~300ms of startup overhead over a v2.1.16 baseline.

Tested with the following which emulates the simplest Pex PEX use case in Pants for interpreter selection for Pants tools:

$ python -mpex . -cpex --unzip -opex.pex
$ ./pex.pex --interpreter-constraint=">=3.6" -- -c 'import sys; print(sys.executable)'
$ multitime -n10 ./pex.pex --interpreter-constraint=">=3.6" -- -c 'import sys; print(sys.executable)'

v2.1.16

===> multitime results
1: ./pex.pex --interpreter-constraint=>=3.6 -- -c "import sys; print(sys.executable)"
            Mean        Std.Dev.    Min         Median      Max
real        0.861       0.007       0.851       0.859       0.874       
user        0.780       0.008       0.770       0.779       0.798       
sys         0.080       0.009       0.067       0.080       0.093 

f07ae47

===> multitime results
1: ./pex.pex --interpreter-constraint=>=3.6 -- -c "import sys; print(sys.executable)"
            Mean        Std.Dev.    Min         Median      Max
real        1.184       0.013       1.170       1.182       1.217       
user        1.076       0.012       1.056       1.080       1.091       
sys         0.101       0.012       0.080       0.099       0.123
@jsirois
Copy link
Member Author

jsirois commented Oct 3, 2020

Noting that the specific mechanism at play here in the regression is --unzip. We ship the Pex PEX in --unzip mode since, without it:

$ python -mpex . -cpex -opex.pex
$ ./pex.pex --interpreter-constraint=">=3.6" -- -c 'import sys; print(sys.executable)'
$ multitime -n10 ./pex.pex --interpreter-constraint=">=3.6" -- -c 'import sys; print(sys.executable)'
===> multitime results
1: ./pex.pex --interpreter-constraint=>=3.6 -- -c "import sys; print(sys.executable)"
            Mean        Std.Dev.    Min         Median      Max
real        1.068       0.009       1.059       1.065       1.090       
user        0.982       0.011       0.971       0.976       1.006       
sys         0.082       0.010       0.057       0.085       0.094 

It makes sense that --unzip mode is sensitive to import scope since we always re-exec from the PEX file in that mode and the more imported code that is needed to re-exec the PEX, the slower it will be:
https://github.com/pantsbuild/pex/blob/cbb44f55f9e8f6293bd6bcd4f337e2aebdc1f1f5/pex/pex_builder.py#L32-L60

@jsirois jsirois self-assigned this Oct 3, 2020
jsirois added a commit to jsirois/pex that referenced this issue Oct 3, 2020
This is a bandaid fix only. Appropriate comments have been added to
help prevent re-regression with a longer term fix tracked by pex-tool#1055.

Fixes pex-tool#1054
jsirois added a commit that referenced this issue Oct 3, 2020
This is a bandaid fix only. Appropriate comments have been added to
help prevent re-regression with a longer term fix tracked by #1055.

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

Successfully merging a pull request may close this issue.

1 participant