gh-122931 Allow stable API extensions to include a multiarch tuple in the filename#122917
gh-122931 Allow stable API extensions to include a multiarch tuple in the filename#122917stefanor wants to merge 5 commits into
Conversation
stefanor
requested review from
ericsnowcurrently and
erlend-aasland
as code owners
stefanor
force-pushed
the
stable-abi-multiarch
branch
2 times, most recently
from
5e380a1 to
7185a58
Compare
…ple in the filename This permits stable ABI extensions for multiple architectures to be co-installed into the same directory, without clashing with each other, the same way (non-stable ABI) regular extensions can. It is listed below the current .abi3 suffix because setuptools will select the first suffix containing .abi3, as the target filename. We do this to protect older Python versions predating this patch.
stefanor
force-pushed
the
stable-abi-multiarch
branch
from
7185a58 to
2955027
Compare
stefanor
changed the title
…ple in the filename This permits stable ABI extensions for multiple architectures to be co-installed into the same directory, without clashing with each other, the same way (non-stable ABI) regular extensions can. It is listed below the current .abi3 suffix because setuptools will select the first suffix containing .abi3, as the target filename. We do this to protect older Python versions predating this patch.
stefanor
force-pushed
the
stable-abi-multiarch
branch
from
2955027 to
c5b58a4
Compare
vstinner
left a comment
vstinner
left a comment
There was a problem hiding this comment.
You should document this change in https://docs.python.org/dev/whatsnew/3.14.html (Doc/whatsnew/3.14.rst). I expect that you would elaborate a bit on the usage, how to use the feature, why it's needed, etc.
Done. FWIW, in Debian we plan to backport this to 3.13 too. |
|
I'm only aware of Debian who uses "multiarch". Do other operating systems also use it? Maybe Debian variants, Ubuntu, and Ubuntu variants? This change will slow down any "import module". I don't recall if there is a cache for that or not. |
All Debian derivatives, yes. They typically don't deviate very much, when it comes to plumbing. |
What do you want to do about that? Is there a benchmark you'd like to see results for? I see a table of 5 entries (including NULL) increasing to 6. That is one extra item to search, when:
|
Example of command: strace output: Internally, when Python imports the If we add a new entry to the import suffix, it will add one fstat() syscall per import (at least, to import a package). We should consider the impact on performance if we decide to add this feature. |
FWIW: macOS, iOS and Android all use However, on iOS and Android, there's limited need to keep binary artefacts in the same folder, as any given executable can only have a single architecture's executables. In addition, in the case of iOS, the binaries need to be migrated to the Frameworks folder and named as Frameworks, so any "side-by-side" benefits would be lost. Any "other platform" executables need to be stripped out as part of the build process, at which point there's no naming conflict. macOS uses the multiarch config value, but the value is always "darwin", and the universal binary format exists to support multiple architectures in a single binary file. I guess it might be useful to be able to have x86_64 and ARM64 binaries side-by-side... but there's probably only a year or two left in the official supported life of x86_64, so I don't think adding this feature will ultimately be that helpful for the macOS use case. |
|
@stefanor: Did you consider to maintain this change as a downstream-only patch in Debian? If you would like to make it upstream, I would suggest making it optional, disabled by default, and add a configure option to enable it. That's how I added some Fedora specific changes, such as: |
|
Here are some benchmarks: There is no discernable performance difference in minimal interpreter startup import time
import sys
from subprocess import check_call
t1 = time.perf_counter()
for i in range(1000):
check_call([sys.executable, "-c", ""])
t2 = time.perf_counter()
print((t2-t1) / 1000)
Looking at strace, I see only a single extra syscall. We can manufacture an import-intensive benchmark: import pkgutil
import time
t1 = time.perf_counter()
for module in pkgutil.walk_packages(onerror=lambda pkg: None):
if module.name.startswith("test."):
continue
if module.name.endswith(".__main__"):
continue
if module.name in {"antigravity", "idlelib.idle", "this", "zen"}:
continue
try:
__import__(module)
except Exception:
pass
t2 = time.perf_counter()
print((t2 - t1))
Analysing this, I see 116 stat syscalls on |
Also, note that this is used on all linux platforms. The default for (non-stable ABI) extensions is to include the multiarch tuple in the extension filename. Pick a random binary wheel built in manylinux, and you'll see them.
I would be happy to do that. Although I prefer not carrying downstream-only patches long-term if possible. Look at the mess around Without this MR, there's no real point in supporting the multiarch tags in the non-stable ABI extensions. Either we get the benefit from doing it everywhere, or you say you don't need to support the feature, and we rip it out everywhere. We've got half a feature at the moment. Would you prefer it to all be behind a config argument?
These are not entirely Fedora-specific. And that's a good argument for always trying to upstream.
In Debian we use those paths for our cross-compilers. I imagine there is a scenario where it's useful to have Python headers in there.
We're a happy customer of this too, It meant one less patch to carry. |
Maybe 0.5% slowdown with PGO. For 1 extra syscall, I'm surprised it's visible. This may still not be significant.
|
|
@erlend-aasland @encukou: Would you mind to have a look at this issue? What do you think? Should it become the default behavior, or should it be a configure option? |
|
IMO, the most important thing here is to keep this in mind for |
Yeah, that sounds sensible.
Is that expected? We've been on abi3 for quite a while. |
If we want that for the stable ABI, do we want to do the same thing for the regular ABI? It would probably affect a lot of build systems that assume they can name their output |
Yes, over a decade. abi3 showing its age, and since it's incompatible free-threading we'll need a new alternative soon.
I don't see much reason to deprecate and remove that, so that we don't break the build systems you mention. (Note that even abi3/abi4 extensions will work with a bare |
I'm going to include it in our 3.13.0 upload. |
|
🔔 (I'd like to remind people to please consider this) |
|
This PR is stale because it has been open for 30 days with no activity. |
…ple in the filename This permits stable ABI extensions for multiple architectures to be co-installed into the same directory, without clashing with each other, the same way (non-stable ABI) regular extensions can. It is listed below the current .abi3 suffix because setuptools will select the first suffix containing .abi3, as the target filename. We do this to protect older Python versions predating this patch.
stefanor
force-pushed
the
stable-abi-multiarch
branch
from
eadad30 to
735f8af
Compare
Documentation build overview
25 files changed ·
|
encukou
left a comment
encukou
left a comment
There was a problem hiding this comment.
@vstinner, what would it take for you to reconsider?
This is rather important for Debian/Ubuntu, doesn't hurt others (the stat call is insignificant as shown above), and makes things more consistent with the non-stable ABI suffix.
AFAIK, you are the only ones shipping such extensions, no? |
At the moment, we're the only ones with this patch. So, the only ones producing such extensions. But, adding it and making it default (which this branch now does), means all Linux have a multiarch tag in linux |
|
But that's something IMO we want to discourage... If you need to share I am okay with supporting your use-case in a best-effort capacity, but I do not think it should be the default behavior. |
|
That said, I don't want to block this PR, so if the rest of the maintainers agree, please go ahead. The only thing I ask is to add tests. |
If that's the case, PEP3149 should be overturned. We implemented PEP3149, to allow us to do this. But stable-abi was forgotten, and when people started to use them, we ran into problems and filed this MR. From my position, PEP3149 works well, and I don't see any need to discourage its use.
It's currently the default behaviour for non-stable ABI. Why should stable-abi be different? Surely we want to align? |
stefanor
force-pushed
the
stable-abi-multiarch
branch
from
3e17279 to
81f5862
Compare
Added a basic test for the presence of the tags. Also, updated the NEWS text for the re-ordering of extensions. |
It is a continuous source of friction and latent bugs. From a packaging tool author as well as package-with-c-exts author perspective, it's definitely not ideal. So @FFY00 is right I think to recommend that if one starts fresh (e.g., a new distro from scratch), please don't do this.
That doesn't follow. Your use case is supported by that PEP, and is rooted in a long history - which is okay. Doesn't mean it's the recommended or optimal choice for new designs. Non-interleaved packages are clearly more robust, and the disk space / robustness tradeoffs are quite different today than 15-20 years ago.
That I can agree with. For design consistency, it's quite reasonable to treat default/unstable and stable ABI extension names the same. Beyond just some work and having to maintain a "it's different from 3.15 onwards" code in packaging tools, I don't see a significant downside to accepting this PR. |
Noted that you wouldn't recommand anybody else implement this interface. Being closer to it, I have a much more positive view. If you look at the problems it's solving, I really don't see any other obvious way to go about it that cause less friction. It massively reduces the amount of duplication and churn we have to do in the distro. It should also be a useful tool for anybody who wants to ship something for multiple Python releases and/or architectures. How would you like to update the NEWS messaging to represent your view here? If this is all something that Python wants to deprecate, we should design a replacement for it. We just had a packaging summit at PyCon. If that was something we should have discussed, I wish I'd known that. |
stefanor
force-pushed
the
stable-abi-multiarch
branch
from
0117088 to
7b071c2
Compare
I think that entry is fine, it's just a factual description of the technical change in this PR. I don't think it needs to touch on anything bigger-picture.
I certainly don't speak for Python (I'm not even a core dev), but: I don't think anyone wants to push for that. It's fine to exist.
The key thing that seems needed is integration testing for interleaved installs CPython, so that the next time any requirements get captured in time, rather than afterwards. That came up on Discourse before, can't find the link right now but it was recent so I guess you remember? |
9 participants
This permits stable ABI extensions for multiple architectures to be co-installed into the same directory, without clashing with each other, the same way (non-stable ABI) regular extensions can.
It is listed below the current .abi3 suffix because setuptools will select the first suffix containing .abi3, as the target filename. We do this to protect older Python versions predating this patch.