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

using embedded pip is slow #6060

Closed
dimbleby opened this issue Jul 24, 2022 · 4 comments · Fixed by #6062
Closed

using embedded pip is slow #6060

dimbleby opened this issue Jul 24, 2022 · 4 comments · Fixed by #6062
Labels
kind/bug Something isn't working as expected

Comments

@dimbleby
Copy link
Contributor

dimbleby commented Jul 24, 2022

Related to #6047

Installing a large number of packages with the embedded pip takes noticeably longer than using a more regular pip - I see the install stage go from ~90s to ~60s in a particular project, if I tweak poetry so that it doesn't use embedded pip. I suppose that reaching inside a wheel and executing pip from there is more expensive.

Probably there are reasons for choosing embedded pip - presumably it leaves poetry less exposed to whatever foolishness the user has managed to achieve with their own pip. And perhaps not every environment has a non-embedded pip in it.

On the other hand this is a noticeable performance penalty. As someone who doesn't habitually mess up their environment, I'd be happy for poetry to use the pip that is already in it.

Perhaps it would be reasonable to use the embedded pip only as a last resort, if no other was available?

This changed at #4011, where the implementation of run_pip() gained an embedded=True parameter.

@dimbleby dimbleby added kind/bug Something isn't working as expected status/triage This issue needs to be triaged labels Jul 24, 2022
@neersighted
Copy link
Member

neersighted commented Jul 24, 2022

Honestly, the long term path forward is intended to be moving the install code inside this repo instead of depending on pip. That being said, a 50% regression for every distfile we install is not nothing. I am going to ask @abn to weigh in here as he drove this change and it was intended to significantly reduce our support burden vis-a-vis broken (or missing, Debian-style) pips, which it appears to me to have succeeded in.

@dimbleby
Copy link
Contributor Author

You may be right re support burden, but I very nearly made the opposite argument: the 1.1.x series is not using embedded pip, almost everyone is using that, it's mostly fine. But perhaps I'm just not exposed to all the places where it's not fine...

@neersighted
Copy link
Member

You may be right re support burden, but I very nearly made the opposite argument: the 1.1.x series is not using embedded pip, almost everyone is using that, it's mostly fine. But perhaps I'm just not exposed to all the places where it's not fine...

The default support response is "have you tried the 1.2 betas to see if they resolve your issue and don't present you with regressions," so I suppose we're cheating a bit re: 1.1.

More sophisticated/experienced Python users tend to have no issue with this as they're no strangers to building and managing their own Pythons, but users of Debian/Ubuntu-provided Python distributions typically run into a missing pip and believe it to be a Poetry issue, hence the use of an embedded copy.

@ipmb ipmb mentioned this issue Aug 31, 2022
2 tasks
@mkniewallner mkniewallner removed the status/triage This issue needs to be triaged label Sep 18, 2022
Copy link

github-actions bot commented Mar 1, 2024

This issue has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Mar 1, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
kind/bug Something isn't working as expected
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants