zlib-api: use zlib-ng +compat by default#39358
Merged
tgamblin merged 3 commits intospack:developfrom Aug 17, 2023
Merged
Conversation
haampie
changed the title
zlib-api: use zlib-ng +compat by default
haampie
force-pushed
the
fix/switch-default-provider-zlib-api
branch
from
408e1e8 to
886688d
Compare
Member
|
Merged! I added the rationale to the description and the commit, and I also added this here: So that people can easily surface it. Hoping it just works as @haampie describes. |
(I'd open an issue if not for the comment in the discussion thread. Let me know if y'all would still like this to be a separate issue). I'm currently getting the following build error: I've narrowed it down to a configure problem. For some reason, on a machine I use, the configure process concludes that my CPU has AVX512 instructions when it does not. From the build/configure log:
Oddly though, if I manually clone and build zlib-ng, Any ideas what's happening here and how to fix it? |
|
Update: Loading a newer compiler (10.2.0 vs 4.9.2) does cause the manual build to fail. It looks like their configure script is just going off of whether the given gcc will compile with AVX512 flags without actually checking whether they'll run on the given hardware. (Edit: More accurately, the assembler fails to work with AVX512 instructions) Regardless, this issue is definitely more of a zlib-ng problem (I'll submit an issue there), but until that's resolved I'd say the switch to zlib-ng might be a bit too soon? Edit: Here's the zlib-ng issue zlib-ng/zlib-ng#1559 |
Member
|
@haampie if we can’t patch the build easily to work with more compilers I’m inclined to revert this. Thoughts? |
Member
Can you post the output of: spack debug report? I think chances are high you are on a very old system, in which case setting these flags https://github.com/zlib-ng/zlib-ng#advanced-build-options based on the |
Member
Author
|
Sounds like a fixable problem. I'd rather collect more issues (if any) than revert the PR, cause otherwise we probably never discover them. We could decide closer to the 0.21 release whether to revert? |
Member
Author
It's not about your CPU, it's about whether your compiler can emit those instructions, which it can. Apparently your assembler cannot deal with those. At runtime Using the right combination of GCC & binutils is definitely a good idea, I don't think zlib-ng will be the only project where you'll have issues. You can also run into issues where binutils doesn't understand gcc's debug info for example. However, we could check how to make the configure check in zlib-ng more strict. |
Member
Author
|
The configure script is generally fine, it's just that the configure test uses |
Member
Author
|
Fixed by zlib-ng/zlib-ng#1562. On Centos 7 with spack-built |
I can do that once I get to my computer, but I can go ahead and confirm that I am on a very old system (Debian from like 2015ish). |
Member
Author
mpokorny
pushed a commit
to mpokorny/spack
that referenced
this pull request
Sep 18, 2023
In the HPC package manager, we want the fastest `zlib` implementation by default. `zlib-ng` is up to 4x faster than stock `zlib`, and it can do things like take advantage of AVX-512 instructions. This PR makes `zlib-ng` the default `zlib-api` provider (`zlib-api` was introduced earlier, in spack#37372). As far as I can see, the only issues you can encounter are: 1. Build issues with packages that heavily rely on `zlib` internals. In Gitlab CI only one out of hundreds of packages had that issue (it extended zlib with deflate stuff, and used its own copy of zlib sources). 2. Packages that like to detect `zlib-ng` separately and rely on `zlib-ng` internals. The only issue I've found with this among the hundreds of packages built in CI is `perl` trying to report more specific zlib-ng version details, and relied on some internals that got refactored. But yeah... that warrants a patch / conflict and is nothing special. At runtime, you cannot really have any issues, given that zlib and zlib-ng export the exact same symbols (and zlib-ng tests this in their CI). You can't really have issues with externals when using zlib-ng either. The only type of issue is when system zlib is rather new, and not marked as external; if another external uses new symbols, and Spack builds an older zlib/zlib-ng, then the external might not find the new symbols. But this is a configuration issue, and it's not an issue caused by zlib-ng, as the same would happen with older Spack zlib. * zlib-api: use zlib-ng +compat by default * make a trivial change to zlib-ng to trigger a rebuild * add `haampie` as maintainer
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
In the HPC package manager, we want the fastest
zlibimplementation by default.zlib-ngis up to 4x faster than stockzlib, and it can do things like take advantage of AVX-512 instructions. This PR makeszlib-ngthe defaultzlib-apiprovider (zlib-apiwas introduced earlier, in #37372).As far as I can see, the only issues you can encounter are:
zlibinternals. In Gitlab CI only one out of hundreds of packages had that issue (it extended zlib with deflate stuff, and used its own copy of zlib sources).zlib-ngseparately and rely onzlib-nginternals. The only issue I've found with this among the hundreds of packages built in CI isperltrying to report more specific zlib-ng version details, and relied on some internals that got refactored. But yeah... that warrants a patch / conflict and is nothing special.At runtime, you cannot really have any issues, given that zlib and zlib-ng export the exact same symbols (and zlib-ng tests this in their CI).
You can't really have issues with externals when using zlib-ng either. The only type of issue is when system zlib is rather new, and not marked as external; if another external uses new symbols, and Spack builds an older zlib/zlib-ng, then the external might not find the new symbols. But this is a configuration issue, and it's not an issue caused by zlib-ng, as the same would happen with older Spack zlib.