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

feat: Add zlib-ng #332

Merged
merged 4 commits into from
Sep 9, 2024
Merged

feat: Add zlib-ng #332

merged 4 commits into from
Sep 9, 2024

Conversation

1Naim
Copy link
Member

@1Naim 1Naim commented Sep 2, 2024

We aim to replace zlib with zlib-ng to make use of the optimizations from the next generation implementation

Some benchmarks to highlight the benefits of this replacement:

Closes #330

@ptr1337 ptr1337 self-requested a review September 2, 2024 13:42
@1Naim 1Naim force-pushed the feat/zlib-ng branch 3 times, most recently from 4fb8f75 to 1403231 Compare September 2, 2024 13:57
@ptr1337
Copy link
Member

ptr1337 commented Sep 2, 2024

We also need to think about, how we do the integration.

e.g we would need to provide a replace=(zlib) for example, so that user will get when updating automatically the update. But for the testing right now that fits

@1Naim
Copy link
Member Author

1Naim commented Sep 2, 2024

Currently zlib-ng-compat conflicts with zlib so for any users that currently build this manually they will get prompted.

It is true however that when we actually get down to merging this, we need to add that as an option.

@ptr1337
Copy link
Member

ptr1337 commented Sep 2, 2024

We maybe want to add before the build part:

export CFLAGS+=" -fno-semantic-interposition"
export CXXLAGS+=" -fno-semantic-interposition"

@ptr1337
Copy link
Member

ptr1337 commented Sep 2, 2024

Clearlinux uses following to PGO the package:

cat *.c | ./minigzip -6 | ./minigzip -d > /dev/null
cat *.c | ./minigzip -4 | ./minigzip -d > /dev/null
cat *.c | ./minigzip -9 | ./minigzip -d > /dev/null
cat minigzip | ./minigzip -6 | ./minigzip -d > /dev/null
cat minigzip | ./minigzip -4 | ./minigzip -d > /dev/null
cat minigzip | ./minigzip -9 | ./minigzip -d > /dev/null

Since mini(g)zip is provided by zlib. We could still run this as PGO profiling, due the compact package (likely). Needs to be tested.

Following C/CXX Flags should be used for profiling:
-fprofile-generate -fprofile-update=atomic
-fprofile-use -fprofile-dir=$PGODIR -fprofile-correction

@sirlucjan sirlucjan self-assigned this Sep 3, 2024
@sirlucjan
Copy link
Member

Ive compiled and installed it - without any problem.

@1Naim 1Naim marked this pull request as ready for review September 6, 2024 13:15
@sirlucjan
Copy link
Member

I'm testing, using and it works.

zlib-ng/zlib-ng/PKGBUILD Show resolved Hide resolved
Copy link
Member

@ptr1337 ptr1337 left a comment

Choose a reason for hiding this comment

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

So, lets merge this for now and put it into the cachyos-testing-v3 repository, and let some people test.

After that, we can try to push it to stable.

@ptr1337 ptr1337 merged commit 432c087 into master Sep 9, 2024
@ptr1337 ptr1337 deleted the feat/zlib-ng branch September 9, 2024 15:47
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.

[RFC] Consider to replace zlib with zlib-ng
3 participants