Opened 7 years ago
Last modified 7 years ago
#54216 assigned enhancement
zlib +ng (and +cloudflare) accelerated variants
Reported by: | RJVB (René Bertin) | Owned by: | ryandesign (Ryan Carsten Schmidt) |
---|---|---|---|
Priority: | Normal | Milestone: | |
Component: | ports | Version: | |
Keywords: | haspatch | Cc: | |
Port: | zlib |
Description
I've brought this up before, trying again now that zlib-ng has become a lot more mature (and zlib itself has been updated too).
Zlib-ng is the "next generation" zlib that offers SIMD acceleration and a number of less visible improvements. It can be configured to be ABI compatible with zlib, which my variant does. It currently does not always compress exactly as good as zlib but differences are small and the speed gain is appreciable. It might be worth following the project's progress.
I've kept in a +cloudflare variant, which is still based on zlib 1.2.8 but gives a slightly bigger and more consistent speed gain.
I'll just attach the portfile patch, patches can be found at https://github.com/RJVB/macstrop/tree/master/archivers/zlib
Attachments (3)
Change History (14)
Changed 7 years ago by RJVB (René Bertin)
comment:1 Changed 7 years ago by mf2k (Frank Schima)
Cc: | ryandesign removed |
---|---|
Owner: | set to ryandesign |
Status: | new → assigned |
comment:2 Changed 7 years ago by RJVB (René Bertin)
Oversight, my bad, sorry. Done (in git).
Another somewhat more compelling example of the performance gain. On a 2.7 Ghz i7, OS X 10.9, with the system's /usr/lib/libz.1.2.5 and a 262Mb tarball:
> repeat 3 /usr/bin/gzip -9 < test.tar > /dev/null 20.078 user_cpu 0.064 kernel_cpu 0:20.14 total_time 99.9%CPU {1044480M 0F 294R 0I 0O 0k 0w 26c} 20.741 user_cpu 0.066 kernel_cpu 0:20.80 total_time 100.0%CPU {1044480M 0F 294R 0I 0O 0k 0w 29c} 20.745 user_cpu 0.071 kernel_cpu 0:20.82 total_time 99.9%CPU {1044480M 0F 294R 0I 0O 0k 0w 36c} > repeat 3 /usr/bin/gzip -d < test.tar.gz > /dev/null 1.616 user_cpu 0.015 kernel_cpu 0:01.63 total_time 99.3%CPU {872448M 0F 249R 0I 1O 0k 0w 9c} 1.620 user_cpu 0.016 kernel_cpu 0:01.63 total_time 100.0%CPU {831488M 0F 239R 0I 0O 0k 0w 3c} 1.609 user_cpu 0.015 kernel_cpu 0:01.62 total_time 99.3%CPU {819200M 0F 236R 0I 0O 0k 0w 2c}
after installing /opt/local/lib/libz.1.2.8.dylib in /usr/lib, changing its id and pointing /usr/lib/libz.1.dylib to it:
> repeat 3 /usr/bin/gzip -9 < test.tar > /dev/null 11.614 user_cpu 0.064 kernel_cpu 0:11.70 total_time 99.7%CPU {1097728M 0F 306R 0I 0O 0k 0w 165c} 11.530 user_cpu 0.062 kernel_cpu 0:11.59 total_time 100.0%CPU {1097728M 0F 306R 0I 0O 0k 0w 11c} 11.545 user_cpu 0.062 kernel_cpu 0:11.60 total_time 100.0%CPU {1097728M 0F 306R 0I 0O 0k 0w 12c} > repeat 3 /usr/bin/gzip -d < test.tar.gz > /dev/null 0.805 user_cpu 0.013 kernel_cpu 0:00.81 total_time 100.0%CPU {872448M 0F 251R 0I 0O 0k 0w 3c} 0.806 user_cpu 0.014 kernel_cpu 0:00.82 total_time 98.7%CPU {872448M 0F 251R 0I 0O 0k 0w 1c} 0.818 user_cpu 0.013 kernel_cpu 0:00.83 total_time 98.7%CPU {872448M 0F 251R 0I 0O 0k 0w 5c}
(this with the +cloudflare variant, the +ng variant should be comparable)
comment:3 Changed 7 years ago by raimue (Rainer Müller)
These variants look like complete rewrites of the port and that makes it look very unmaintainable. Are these ports API/ABI compatible? I think we should rather go to with new ports and replace port:zlib
with path:lib/libz.dylib:zlib
or similar in depspecs. I also see no point in adding the benchmark results to the Portfile.
comment:4 follow-up: 5 Changed 7 years ago by RJVB (René Bertin)
In a sense they are indeed complete rewrites, and esp. for the CloudFlare variant there is the issue that it has a lower version (which we could spoof, of course). If this had been a proper port of mine I would have (re)designed it with 3 sections, as if containing 3 subports instead of 3 build variants. The reason I've gone with variants is that port:zlib is among the ports with the largest number of dependents at all levels in the dependency graph. It'd be too easy to break things with a single unhappy commit. And variants make it easier for users to go back to the official version for whatever reason.
FWIW, this is an example where a syntax of the type provides foo
or alternative_for foo
would be useful.
It is stated in the summary that the +NG variant configures zlib-ng to be ABI compliant (I have a PR out upstream for detail they let slip through in that aspect). The CloudFlare patches have always aimed to be ABI compliant but there too I have a PR out for the 32bit build. And yes, they're API compliant too (as far as that doesn't follow from being ABI compliant).
I have replaced my system libz.dylib with the CloudFlare variant at about the time I filed this ticket, and haven't noticed any issues (on Mac and Linux, btw).
Of course there's little interest in adding benchmark results to the final portfile(s). Although ... 8-)
Changed 7 years ago by RJVB (René Bertin)
Attachment: | zlib.2.diff added |
---|
Changed 7 years ago by RJVB (René Bertin)
Attachment: | zlib.3.diff added |
---|
comment:5 follow-up: 6 Changed 7 years ago by raimue (Rainer Müller)
Replying to RJVB:
And variants make it easier for users to go back to the official version for whatever reason.
I would have gone with three ports: zlib, zlib-ng, and zlib-cloudflare, all fulfilling the path:lib/libz.dylib:zlib
depspec. Then you would also not have any problem with different version numbers. Switching between those just requires a simple port activate
, which is even easier than switching variants.
I agree that something like provides foo
would be better, but we just do not have it yet.
comment:6 follow-up: 7 Changed 7 years ago by RJVB (René Bertin)
Replying to raimue:
I would have gone with three ports: zlib, zlib-ng, and zlib-cloudflare, all fulfilling the
path:lib/libz.dylib:zlib
depspec.
They would, but it takes only 1 port that doesn't play nice to mess things up.
> port dependents zlib | wc -l 165
(and that's just the number of installed ports.)
Then you would also not have any problem with different version numbers.
There were concerns about licensing when I brought up the +cloudflare variant before. I've left it in because ATM it still gives a tiny bit better performance than zlib-ng, but I wouldn't insist on maintaining the variant esp. since CloudFlare don't seem to be keeping their code up-to-date.
Switching between those just requires a simple
port activate
, which is even easier than switching variants.
Huh? Switching between ports takes two commands, deactivate the one, activate the other. It's switching variants that takes a simple and unique port activate
call?!
Ultimately it'll be Ryan's call I presume.
comment:7 Changed 7 years ago by raimue (Rainer Müller)
Replying to RJVB:
> port dependents zlib | wc -l 165(and that's just the number of installed ports.)
You can calculate that for all ports with the depends:
pseudo-portname selector.
$ port -q echo depends:zlib |wc -l 765
We already use the path:
syntax for many other ports (especially for *-devel ports) and it works. All these existing Portfiles can be updated with just one search and replace.
Huh? Switching between ports takes two commands, deactivate the one, activate the other. It's switching variants that takes a simple and unique
port activate
call?!
I stand corrected, looks like port install
handles that now. That only works as no other previously selected variants need to be preserved, otherwise it would be the unwieldy sudo port -n upgrade --enforce-variants -ng +cloudflare
.
comment:8 Changed 7 years ago by RJVB (René Bertin)
Of course path:-style depspecs work, and of course you can change them with a single search/replace, that's not the issue.
But they're not very nice to enter or maintain, and it takes only a single portfile change (or new port) to forget using it to break things for people who have an alternative port installed. (You also introduce a platform-dependency through the dylib extension, but that only concerns hoopleheads like me who maintain most of their ports to function on Linux too.)
Maybe we could consider writing a PortGroup that provides depspecs for all commonly used archivers; zlib is likely not the only one for which alternative implementations exist. A batch change to make all ports use that PG will be a lot trickier though, and
> port echo depends:zlib | wc -l 836
otherwise it would be the unwieldy
sudo port -n upgrade --enforce-variants -ng +cloudflare
I think you're still confusing things. Port install
always allowed you to install another build variant, but it won't deactivate a conflicting port - how would it know which to deactivate (as long as "base" doesn't provide a provides foo
or alternative_for foo
syntax)?
The unwieldy form is for upgrading a port and changing variants at the same time (I think, I've seen the hints to use it but never have).
What you need to change between alternatives is something like
> port-swap-active alternative1 alternative2
(https://github.com/RJVB/macstrop/blob/master/macports/bin/port-swap-active)
OT: I should extend that script to allow swapping "families" of ports in a single call.
comment:9 Changed 7 years ago by RJVB (René Bertin)
An alternative that might be easier to deploy than provides
or alternative_for
syntax would be to add a lookup (or depspec rewriter) step.
port:foo -> "foo" if look up "foo" in a table (dict?) (or us the entire port:foo depspec as key) use the returned expression (say path:libz.dylib) to determine if foo is available or not else just use the port:foo depspec
The table or dict would be populated with the (probably few) ports for which their are known/official alternatives that aren't already handled through dedicated portgroups.
Would such an extra step incur a large performance hit?
comment:10 Changed 7 years ago by raimue (Rainer Müller)
Sure, there are plenty of ways to implement provides
or alternative sets of ports, especially after the libsolv implementation from gsoc15 lands in base. However, I would say this discussion is out of the scope of this ticket as it is not specific to the zlib variants.
comment:11 Changed 7 years ago by RJVB (René Bertin)
Of course. Ports with alternatives and this many dependents would be prime candidates though :)
The
# $Id:
line is obsolete, please remove it.