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 base executable when it exists, to avoid recompiling when switching venv #429

Merged

Conversation

matsjoyce-refeyn
Copy link
Contributor

When using a requirements file compilation tool like pip-tools, a lot of temporary virtual environments are created, into which packages are installed. When a package uses setuptools-rust, the value of sys.executable keeps changing, which cause cargo to recompile quite a few crates (pyo3 and anything that depends on it). This slows things down quite a lot.

This PR uses the semi-documented sys._base_executable value, which points to the real Python interpreter when inside a venv. If it doesn't exist, or the provided path is not valid, we fall back to the current behaviour.

I tested this by running a requirements compilation, and checking for rustc invocations: There are a lot before this change, and none after (provided the rust package in question was already compiled).

@davidhewitt
Copy link
Member

Thanks for the PR! This diff seems very reasonable to me, and definitely a desirable goal to reduce recompilation.

A couple of design questions:

  • Are there any downsides we can think of by doing this? For example, PyO3 doesn't yet try to bake information about virtualenvs into binaries. Should it? My instinct is probably not (and it should use runtime detection), but we have known gaps in virtualenv support in PyO3 and I wonder if this change interacts with that.
  • Rather than using _base_executable, should we use the relative difference between sys.prefix and sys.executable, and apply that to sys.base_prefix? Again, probably not a good idea, though I wanted to at least put the idea out for discussion.

So overall this seems reasonable to me. We should add a note in the CHANGELOG for this please.

@matsjoyce-refeyn
Copy link
Contributor Author

  • Are there any downsides we can think of by doing this? For example, PyO3 doesn't yet try to bake information about virtualenvs into binaries. Should it? My instinct is probably not (and it should use runtime detection), but we have known gaps in virtualenv support in PyO3 and I wonder if this change interacts with that.

My assumption is that since PyO3 (in this case) cares about Python from a compilation POV, virtual environments don't affect it as all the interesting header-related things are not touched by the venv.

From what I read online, the issues other users were having with PyO3 was to do with activating a venv when running Python embedded in Rust, so this would have no effect (good or bad) on that. But my search was not very thougher, are there any other known issues?

  • Rather than using _base_executable, should we use the relative difference between sys.prefix and sys.executable, and apply that to sys.base_prefix? Again, probably not a good idea, though I wanted to at least put the idea out for discussion.

We could, my worry would be that there are a lot of special cases, since I don't think there's a requirement that the name of the executable matches (that seems to have been the problem with python/cpython#99204). By using sys._base_executable, we offload that problem onto the stdlib authors, who have a better chance of thinking though all the possibilities than I have 😄

So overall this seems reasonable to me. We should add a note in the CHANGELOG for this please.

Will do!

Copy link
Member

@davidhewitt davidhewitt left a comment

Choose a reason for hiding this comment

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

Thanks 👍

@davidhewitt davidhewitt merged commit 45b095b into PyO3:main May 25, 2024
36 of 39 checks passed
@messense
Copy link
Member

messense commented Jun 5, 2024

This does not seem like a reliable solution, I've tried in maturin, when using with poetry to install an editable pyo3 based dep,

$ /tmp/tmp3uzvm48r/.venv/bin/python
Python 3.10.12 (main, Nov 20 2023, 15:14:05) [GCC 11.4.0] on linux
Type "help", "copyright", "credits" or "license" for more information.
>>> import sys
>>> sys.executable
'/tmp/tmp3uzvm48r/.venv/bin/python'
>>> sys._base_executable
'/tmp/tmp3uzvm48r/.venv/bin/python'

the isolated venv poetry creates has sys.executable == sys._base_executable.

Edit: seems that we just need to use os.path.realpath to resolve symlink.

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.

3 participants