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 cached environments in PEP 723 execution #4789

Merged
merged 2 commits into from
Jul 4, 2024

Conversation

charliermarsh
Copy link
Member

Summary

This seems like another good candidate for environment caching. If you run a script repeatedly, we can just use the existing cached environment.

@charliermarsh charliermarsh added performance Potential performance improvement preview Experimental behavior labels Jul 3, 2024
@charliermarsh charliermarsh force-pushed the charlie/cache-pep723-script branch 2 times, most recently from a9faa43 to 636d23e Compare July 3, 2024 21:10
@charliermarsh charliermarsh requested review from zanieb and konstin July 3, 2024 21:17
@charliermarsh charliermarsh force-pushed the charlie/cache-tool-run branch from 0abdc3a to 180562e Compare July 3, 2024 23:15
Base automatically changed from charlie/cache-tool-run to main July 3, 2024 23:25
@charliermarsh charliermarsh force-pushed the charlie/cache-pep723-script branch from 636d23e to 10838d9 Compare July 3, 2024 23:26
Copy link
Member

@konstin konstin left a comment

Choose a reason for hiding this comment

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

We should think about a plan for cached environment garbage collection

@charliermarsh charliermarsh force-pushed the charlie/cache-pep723-script branch from 10838d9 to 0949afe Compare July 4, 2024 13:45
@charliermarsh charliermarsh force-pushed the charlie/cache-pep723-script branch 3 times, most recently from e92b082 to 84a2da6 Compare July 4, 2024 17:14
@charliermarsh charliermarsh force-pushed the charlie/cache-pep723-script branch from 84a2da6 to ab3b621 Compare July 4, 2024 17:14
@charliermarsh charliermarsh enabled auto-merge (squash) July 4, 2024 17:22
@charliermarsh charliermarsh merged commit 6a27135 into main Jul 4, 2024
47 checks passed
@charliermarsh charliermarsh deleted the charlie/cache-pep723-script branch July 4, 2024 17:23
@@ -126,4 +126,9 @@ impl CachedEnvironment {

Ok(Self(venv))
}

/// Convert the [`EphemeralEnvironment`] into an [`Interpreter`].
Copy link
Contributor

@Peiffap Peiffap Jul 4, 2024

Choose a reason for hiding this comment

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

This reintroduced EphemeralEnvironment after you changed it in #4804. There's a few other places where the comments still need to be changed but given that the file is littered with TODOs I'll just comment here rather than open a PR and create merge conflicts.

Copy link
Member Author

Choose a reason for hiding this comment

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

Thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
performance Potential performance improvement preview Experimental behavior
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants