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

Poetry install performance enhancement for low-resource environments #27

Merged
merged 14 commits into from
Dec 11, 2023

Conversation

airportyh
Copy link
Collaborator

@airportyh airportyh commented Dec 7, 2023

Why?

poetry install is slow for large libraries especially for free accounts. This PR should cut install time by ~50% on average.

Context:

Changes

  1. Previously we switched poetry's modern installation option off, but we found turning this on to perform much faster, so we'll switch it on
  2. In order for modern installation to install properly to .pythonlibs (with pip use the --user option and PYTHONUSERBASE to do this), we made a change to wheel_installer.py that routes file writes to the userbase and usersite directories. This as flagged behind POETRY_USE_USER_SITE.
  3. when poetry downloads large files using request, it can run out of memory in free accounts. We work around this by shelling out to curl for the download. This is flagged behind POETRY_DOWNLOAD_WITH_CURL and it is only activated for URLs starting https://files.pythonhosted.org/, because otherwise it could be a private package repo that requires authentication.

The plan is to turn on POETRY_DOWNLOAD_WITH_CURL and POETRY_USE_USER_SITE for all repls and turn off POETRY_INSTALLER_PARALLEL for free accounts (running install jobs in parallel hurts repls with only 1 cpu) via a wrapper script for poetry.

Testing

  1. regression testing for a number of well known python libraries is recommended
  2. compare install speed for core and free accounts

@airportyh airportyh marked this pull request as ready for review December 7, 2023 22:30
@airportyh airportyh requested a review from a team as a code owner December 7, 2023 22:30
@airportyh airportyh requested review from masad-frost and removed request for a team December 7, 2023 22:30
@airportyh airportyh changed the title Th test modern install Poetry install performance enhancement for low-resource environments Dec 7, 2023
Copy link

@lhchavez lhchavez left a comment

Choose a reason for hiding this comment

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

thanks for making this and backing all choices with data!

Comment on lines +874 to +875
download_file_with_curl(url, str(archive))
return archive
Copy link

Choose a reason for hiding this comment

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

Suggested change
download_file_with_curl(url, str(archive))
return archive
try:
download_file_with_curl(url, str(archive))
return archive
except:
# If we failed to download with curl, give it another try with the local implementation
logging.exception('failed to download archive with curl. trying again')

Choose a reason for hiding this comment

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

uh-oh, this wasn't addressed.

src/poetry/utils/helpers.py Outdated Show resolved Hide resolved
src/poetry/utils/helpers.py Show resolved Hide resolved
@airportyh airportyh merged commit 6157674 into replit-1.5 Dec 11, 2023
12 checks passed

url = str(link)
if os.getenv("POETRY_DOWNLOAD_WITH_CURL") == "1" and url.startswith("https://files.pythonhosted.org/"):
if self.supports_fancy_output():

Choose a reason for hiding this comment

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

fancy!

Comment on lines +874 to +875
download_file_with_curl(url, str(archive))
return archive

Choose a reason for hiding this comment

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

uh-oh, this wasn't addressed.

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