[PATCH 0/5] Zstd support for 'guix publish' and 'guix substitute'

DoneSubmitted by Ludovic Courtès.
Details
3 participants
  • Jonathan Brielmaier
  • Ludovic Courtès
  • Mathieu Othacehe
Owner
unassigned
Severity
normal
L
L
Ludovic Courtès wrote on 27 Dec 2020 15:13
(address . guix-patches@gnu.org)(name . Ludovic Courtès)(address . ludo@gnu.org)
20201227141327.10827-1-ludo@gnu.org
Hello Guix!
This adds zstd compression support for ‘guix publish’ and for‘guix substitute’.
Currently ‘guix substitute’ implements the same policy has before,which is to pick the smallest archive when several compressionmethods are proposed. The next step will be to make thatconfigurable.
Thoughts?
Ludo’.
Ludovic Courtès (5): utils: Remove 'compressed-output-port'. utils: Support zstd compression via Guile-zstd. publish: Add support for zstd compression. substitute: Add zstd support. doc: Mention optional dependency on Guile-zstd.
doc/guix.texi | 23 ++++++++++++----- guix/scripts/publish.scm | 31 +++++++++++++---------- guix/scripts/substitute.scm | 3 +++ guix/utils.scm | 23 ++++++++--------- tests/publish.scm | 16 ++++++++++++ tests/utils.scm | 49 +++++++++++++++++++++++++------------ 6 files changed, 97 insertions(+), 48 deletions(-)
-- 2.29.2
L
L
Ludovic Courtès wrote on 27 Dec 2020 15:38
[PATCH 1/5] utils: Remove 'compressed-output-port'.
(address . 45460@debbugs.gnu.org)(name . Ludovic Courtès)(address . ludo@gnu.org)
20201227143809.18554-1-ludo@gnu.org
This procedure was unused except in one test.
* guix/utils.scm (compressed-port): Remove.* tests/utils.scm (test-compression/decompression): Rewrite to use'compressed-output-port' instead.--- guix/utils.scm | 13 ------------- tests/utils.scm | 43 +++++++++++++++++++++++++++++-------------- 2 files changed, 29 insertions(+), 27 deletions(-)
Toggle diff (92 lines)diff --git a/guix/utils.scm b/guix/utils.scmindex a591b62f30..e3c78959ed 100644--- a/guix/utils.scm+++ b/guix/utils.scm@@ -109,7 +109,6 @@ edit-expression filtered-port- compressed-port decompressed-port call-with-decompressed-port compressed-output-port@@ -224,18 +223,6 @@ a symbol such as 'xz." '())) (_ (error "unsupported compression scheme" compression)))) -(define (compressed-port compression input)- "Return an input port where INPUT is compressed according to COMPRESSION,-a symbol such as 'xz."- (match compression- ((or #f 'none) (values input '()))- ('bzip2 (filtered-port `(,%bzip2 "-c") input))- ('xz (filtered-port `(,%xz "-c") input))- ('gzip (filtered-port `(,%gzip "-c") input))- ('lzip (values (lzip-port 'make-lzip-input-port/compressed input)- '()))- (_ (error "unsupported compression scheme" compression))))- (define (call-with-decompressed-port compression port proc) "Call PROC with a wrapper around PORT, a file port, that decompresses data read from PORT according to COMPRESSION, a symbol such as 'xz."diff --git a/tests/utils.scm b/tests/utils.scmindex 009e2121ab..c278b2a277 100644--- a/tests/utils.scm+++ b/tests/utils.scm@@ -1,5 +1,5 @@ ;;; GNU Guix --- Functional package management for GNU-;;; Copyright © 2012, 2013, 2014, 2015, 2016, 2017, 2018, 2019 Ludovic Courtès <ludo@gnu.org>+;;; Copyright © 2012, 2013, 2014, 2015, 2016, 2017, 2018, 2019, 2020 Ludovic Courtès <ludo@gnu.org> ;;; Copyright © 2014 Eric Bavier <bavier@member.fsf.org> ;;; Copyright © 2016 Mathieu Lirzin <mthl@gnu.org> ;;;@@ -182,19 +182,34 @@ skip these tests." method) (let ((data (call-with-input-file (search-path %load-path "guix.scm") get-bytevector-all)))- (let*-values (((compressed pids1)- (compressed-port method (open-bytevector-input-port data)))- ((decompressed pids2)- (decompressed-port method compressed)))- (and (every (compose zero? cdr waitpid)- (pk 'pids method (append pids1 pids2)))- (let ((result (get-bytevector-all decompressed)))- (pk 'len method- (if (bytevector? result)- (bytevector-length result)- result)- (bytevector-length data))- (equal? result data))))))+ (call-with-temporary-output-file+ (lambda (output port)+ (close-port port)+ (let*-values (((compressed pids)+ ;; Note: 'compressed-output-port' only supports file+ ;; ports.+ (compressed-output-port method+ (open-file output "w0"))))+ (put-bytevector compressed data)+ (close-port compressed)+ (and (every (compose zero? cdr waitpid)+ (pk 'pids method pids))+ (let*-values (((decompressed pids)+ (decompressed-port method+ (open-bytevector-input-port+ (call-with-input-file output+ get-bytevector-all))))+ ((result)+ (get-bytevector-all decompressed)))+ (close-port decompressed)+ (pk 'len method+ (if (bytevector? result)+ (bytevector-length result)+ result)+ (bytevector-length data))+ (and (every (compose zero? cdr waitpid)+ (pk 'pids method pids))+ (equal? result data))))))))) (false-if-exception (delete-file temp-file)) (unless (run?) (test-skip 1))-- 2.29.2
L
L
Ludovic Courtès wrote on 27 Dec 2020 15:38
[PATCH 2/5] utils: Support zstd compression via Guile-zstd.
(address . 45460@debbugs.gnu.org)(name . Ludovic Courtès)(address . ludo@gnu.org)
20201227143809.18554-2-ludo@gnu.org
* guix/utils.scm (lzip-port): Return a single value.(zstd-port): New procedure.(decompressed-port, compressed-output-port): Add 'zstd' case.* tests/utils.scm (test-compression/decompression): Test 'zstd' whenthe (zstd) module is available.--- guix/utils.scm | 12 +++++++++++- tests/utils.scm | 6 ++++-- 2 files changed, 15 insertions(+), 3 deletions(-)
Toggle diff (56 lines)diff --git a/guix/utils.scm b/guix/utils.scmindex e3c78959ed..09eee0ecfb 100644--- a/guix/utils.scm+++ b/guix/utils.scm@@ -209,7 +209,13 @@ buffered data is lost." "Return the lzip port produced by calling PROC (a symbol) on PORT and ARGS. Raise an error if lzlib support is missing." (let ((make-port (module-ref (resolve-interface '(lzlib)) proc)))- (values (make-port port) '())))+ (make-port port)))++(define (zstd-port proc port . args)+ "Return the zstd port produced by calling PROC (a symbol) on PORT and ARGS.+Raise an error if zstd support is missing."+ (let ((make-port (module-ref (resolve-interface '(zstd)) proc)))+ (make-port port))) (define (decompressed-port compression input) "Return an input port where INPUT is decompressed according to COMPRESSION,@@ -221,6 +227,8 @@ a symbol such as 'xz." ('gzip (filtered-port `(,%gzip "-dc") input)) ('lzip (values (lzip-port 'make-lzip-input-port input) '()))+ ('zstd (values (zstd-port 'make-zstd-input-port input)+ '())) (_ (error "unsupported compression scheme" compression)))) (define (call-with-decompressed-port compression port proc)@@ -280,6 +288,8 @@ program--e.g., '(\"--fast\")." ('gzip (filtered-output-port `(,%gzip "-c" ,@options) output)) ('lzip (values (lzip-port 'make-lzip-output-port output) '()))+ ('zstd (values (zstd-port 'make-zstd-output-port output)+ '())) (_ (error "unsupported compression scheme" compression)))) (define* (call-with-compressed-output-port compression port procdiff --git a/tests/utils.scm b/tests/utils.scmindex c278b2a277..9bce446d98 100644--- a/tests/utils.scm+++ b/tests/utils.scm@@ -228,8 +228,10 @@ skip these tests." get-bytevector-all))))) (for-each test-compression/decompression- '(gzip xz lzip)- (list (const #t) (const #t) (const #t)))+ `(gzip xz lzip zstd)+ (list (const #t) (const #t) (const #t)+ (lambda ()+ (resolve-module '(zstd) #t #f #:ensure #f)))) ;; This is actually in (guix store). (test-equal "store-path-package-name"-- 2.29.2
L
L
Ludovic Courtès wrote on 27 Dec 2020 15:38
[PATCH 3/5] publish: Add support for zstd compression.
(address . 45460@debbugs.gnu.org)(name . Ludovic Courtès)(address . ludo@gnu.org)
20201227143809.18554-3-ludo@gnu.org
* guix/scripts/publish.scm (compress-nar)[write-compressed-file]: Newprocedure.Use it for 'gzip' and 'lzip'. Add 'zstd.(nar-response-port, string->compression-type): Add case for 'zstd'.* tests/publish.scm (zstd-supported?): New procedure.("/nar/zstd/*"): New test.* doc/guix.texi (Invoking guix publish): Document zstd compression.--- doc/guix.texi | 18 ++++++++++++------ guix/scripts/publish.scm | 31 ++++++++++++++++++------------- tests/publish.scm | 16 ++++++++++++++++ 3 files changed, 46 insertions(+), 19 deletions(-)
Toggle diff (144 lines)diff --git a/doc/guix.texi b/doc/guix.texiindex b12cb11bdf..ed38f2e37b 100644--- a/doc/guix.texi+++ b/doc/guix.texi@@ -12329,17 +12329,23 @@ server socket is open and the signing key has been read. @item --compression[=@var{method}[:@var{level}]] @itemx -C [@var{method}[:@var{level}]] Compress data using the given @var{method} and @var{level}. @var{method} is-one of @code{lzip} and @code{gzip}; when @var{method} is omitted, @code{gzip}-is used.+one of @code{lzip}, @code{zstd}, and @code{gzip}; when @var{method} is+omitted, @code{gzip} is used. When @var{level} is zero, disable compression. The range 1 to 9 corresponds to different compression levels: 1 is the fastest, and 9 is the best (CPU-intensive). The default is 3. -Usually, @code{lzip} compresses noticeably better than @code{gzip} for a small-increase in CPU usage; see-@uref{https://nongnu.org/lzip/lzip_benchmark.html,benchmarks on the lzip Web-page}.+Usually, @code{lzip} compresses noticeably better than @code{gzip} for a+small increase in CPU usage; see+@uref{https://nongnu.org/lzip/lzip_benchmark.html,benchmarks on the lzip+Web page}. However, @code{lzip} achieves low decompression throughput+(on the order of 50@tie{}MiB/s on modern hardware), which can be a+bottleneck for someone who downloads over a fast network connection.++The compression ratio of @code{zstd} is between that of @code{lzip} and+that of @code{gzip}; its main advantage is a+@uref{https://facebook.github.io/zstd/,high decompression speed}. Unless @option{--cache} is used, compression occurs on the fly and the compressed streams are notdiff --git a/guix/scripts/publish.scm b/guix/scripts/publish.scmindex 5a865c838d..fa85088ed0 100644--- a/guix/scripts/publish.scm+++ b/guix/scripts/publish.scm@@ -56,6 +56,8 @@ #:use-module (zlib) #:autoload (lzlib) (call-with-lzip-output-port make-lzip-output-port)+ #:autoload (zstd) (call-with-zstd-output-port+ make-zstd-output-port) #:use-module (guix cache) #:use-module (guix ui) #:use-module (guix scripts)@@ -588,23 +590,22 @@ requested using POOL." (define nar (nar-cache-file cache item #:compression compression)) + (define (write-compressed-file call-with-compressed-output-port)+ ;; Note: the file port gets closed along with the compressed port.+ (call-with-compressed-output-port (open-output-file (string-append nar ".tmp"))+ (lambda (port)+ (write-file item port))+ #:level (compression-level compression))+ (rename-file (string-append nar ".tmp") nar))+ (mkdir-p (dirname nar)) (match (compression-type compression) ('gzip- ;; Note: the file port gets closed along with the gzip port.- (call-with-gzip-output-port (open-output-file (string-append nar ".tmp"))- (lambda (port)- (write-file item port))- #:level (compression-level compression)- #:buffer-size %default-buffer-size)- (rename-file (string-append nar ".tmp") nar))+ (write-compressed-file call-with-gzip-output-port)) ('lzip- ;; Note: the file port gets closed along with the lzip port.- (call-with-lzip-output-port (open-output-file (string-append nar ".tmp"))- (lambda (port)- (write-file item port))- #:level (compression-level compression))- (rename-file (string-append nar ".tmp") nar))+ (write-compressed-file call-with-lzip-output-port))+ ('zstd+ (write-compressed-file call-with-zstd-output-port)) ('none ;; Cache nars even when compression is disabled so that we can ;; guarantee the TTL (see <https://bugs.gnu.org/28664>.)@@ -871,6 +872,9 @@ example: \"/foo/bar\" yields '(\"foo\" \"bar\")." (($ <compression> 'lzip level) (make-lzip-output-port (response-port response) #:level level))+ (($ <compression> 'zstd level)+ (make-zstd-output-port (response-port response)+ #:level level)) (($ <compression> 'none) (response-port response)) (#f@@ -953,6 +957,7 @@ blocking." (match string ("gzip" 'gzip) ("lzip" 'lzip)+ ("zstd" 'zstd) (_ #f))) (define (effective-compression requested-type compressions)diff --git a/tests/publish.scm b/tests/publish.scmindex cafd0f13a2..52101876b5 100644--- a/tests/publish.scm+++ b/tests/publish.scm@@ -38,6 +38,7 @@ #:use-module ((guix pki) #:select (%public-key-file %private-key-file)) #:use-module (zlib) #:use-module (lzlib)+ #:autoload (zstd) (call-with-zstd-input-port) #:use-module (web uri) #:use-module (web client) #:use-module (web response)@@ -54,6 +55,9 @@ (define %store (open-connection-for-tests)) +(define (zstd-supported?)+ (resolve-module '(zstd) #t #f #:ensure #f))+ (define %reference (add-text-to-store %store "ref" "foo")) (define %item (add-text-to-store %store "item" "bar" (list %reference)))@@ -237,6 +241,18 @@ References: ~%" (cut restore-file <> temp))) (call-with-input-file temp read-string)))) +(unless (zstd-supported?) (test-skip 1))+(test-equal "/nar/zstd/*"+ "bar"+ (call-with-temporary-output-file+ (lambda (temp port)+ (let ((nar (http-get-port+ (publish-uri+ (string-append "/nar/zstd/" (basename %item))))))+ (call-with-zstd-input-port nar+ (cut restore-file <> temp)))+ (call-with-input-file temp read-string))))+ (test-equal "/*.narinfo with compression" `(("StorePath" . ,%item) ("URL" . ,(string-append "nar/gzip/" (basename %item)))-- 2.29.2
L
L
Ludovic Courtès wrote on 27 Dec 2020 15:38
[PATCH 4/5] substitute: Add zstd support.
(address . 45460@debbugs.gnu.org)(name . Ludovic Courtès)(address . ludo@gnu.org)
20201227143809.18554-4-ludo@gnu.org
* guix/scripts/substitute.scm (%compression-methods): Add zstd.(compresses-better?): "lzip" always wins.--- guix/scripts/substitute.scm | 3 +++ 1 file changed, 3 insertions(+)
Toggle diff (23 lines)diff --git a/guix/scripts/substitute.scm b/guix/scripts/substitute.scmindex 8084c89ae5..32ebcf1bb9 100755--- a/guix/scripts/substitute.scm+++ b/guix/scripts/substitute.scm@@ -944,6 +944,8 @@ authorized substitutes." ;; supported. See 'decompressed-port' in (guix utils). `(("gzip" . ,(const #t)) ("lzip" . ,(const #t))+ ("zstd" . ,(lambda ()+ (resolve-module '(zstd) #t #f #:ensure #f))) ("xz" . ,(const #t)) ("bzip2" . ,(const #t)) ("none" . ,(const #t))))@@ -961,6 +963,7 @@ this is a rough approximation." (match compression1 ("none" #f) ("gzip" (string=? compression2 "none"))+ ("lzip" #t) (_ (or (string=? compression2 "none") (string=? compression2 "gzip"))))) -- 2.29.2
L
L
Ludovic Courtès wrote on 27 Dec 2020 15:38
[PATCH 5/5] doc: Mention optional dependency on Guile-zstd.
(address . 45460@debbugs.gnu.org)(name . Ludovic Courtès)(address . ludo@gnu.org)
20201227143809.18554-5-ludo@gnu.org
* doc/guix.texi (Requirements): Add Guile-zstd.--- doc/guix.texi | 5 +++++ 1 file changed, 5 insertions(+)
Toggle diff (18 lines)diff --git a/doc/guix.texi b/doc/guix.texiindex ed38f2e37b..dafa1ffc56 100644--- a/doc/guix.texi+++ b/doc/guix.texi@@ -850,6 +850,11 @@ Support for build offloading (@pxref{Daemon Offload Setup}) and @uref{https://github.com/artyom-poptsov/guile-ssh, Guile-SSH}, version 0.13.0 or later. +@item+@uref{https://notabug.org/guile-zstd/guile-zstd, Guile-zstd}, for zstd+compression and decompression in @command{guix publish} and for+substitutes (@pxref{Invoking guix publish}).+ @item @uref{https://ngyro.com/software/guile-semver.html, Guile-Semver} for the @code{crate} importer (@pxref{Invoking guix import}).-- 2.29.2
J
J
Jonathan Brielmaier wrote on 28 Dec 2020 10:17
Re: [bug#45460] [PATCH 3/5] publish: Add support for zstd compression.
0ccd6512-566f-3c5a-cd62-65a2d320d919@web.de
Nice thing, only one nitpick
On 27.12.20 15:38, Ludovic Courtès wrote:
Toggle quote (42 lines)> * doc/guix.texi (Invoking guix publish): Document zstd compression.> ---> doc/guix.texi | 18 ++++++++++++------> guix/scripts/publish.scm | 31 ++++++++++++++++++-------------> tests/publish.scm | 16 ++++++++++++++++> 3 files changed, 46 insertions(+), 19 deletions(-)>> diff --git a/doc/guix.texi b/doc/guix.texi> index b12cb11bdf..ed38f2e37b 100644> --- a/doc/guix.texi> +++ b/doc/guix.texi> @@ -12329,17 +12329,23 @@ server socket is open and the signing key has been read.> @item --compression[=@var{method}[:@var{level}]]> @itemx -C [@var{method}[:@var{level}]]> Compress data using the given @var{method} and @var{level}. @var{method} is> -one of @code{lzip} and @code{gzip}; when @var{method} is omitted, @code{gzip}> -is used.> +one of @code{lzip}, @code{zstd}, and @code{gzip}; when @var{method} is> +omitted, @code{gzip} is used.>> When @var{level} is zero, disable compression. The range 1 to 9 corresponds> to different compression levels: 1 is the fastest, and 9 is the best> (CPU-intensive). The default is 3.>> -Usually, @code{lzip} compresses noticeably better than @code{gzip} for a small> -increase in CPU usage; see> -@uref{https://nongnu.org/lzip/lzip_benchmark.html,benchmarks on the lzip Web> -page}.> +Usually, @code{lzip} compresses noticeably better than @code{gzip} for a> +small increase in CPU usage; see> +@uref{https://nongnu.org/lzip/lzip_benchmark.html,benchmarks on the lzip> +Web page}. However, @code{lzip} achieves low decompression throughput> +(on the order of 50@tie{}MiB/s on modern hardware), which can be a> +bottleneck for someone who downloads over a fast network connection.> +> +The compression ratio of @code{zstd} is between that of @code{lzip} and> +that of @code{gzip}; its main advantage is a> +@uref{https://facebook.github.io/zstd/,high decompression speed}.>> Unless @option{--cache} is used, compression occurs on the fly and> the compressed streams are not
It should be also documented at the `guix-publish-configuration` sectionin the manual. As I guess that a lot of publish server admins look there...
M
M
Mathieu Othacehe wrote on 30 Dec 2020 10:30
Re: [bug#45460] [PATCH 0/5] Zstd support for 'guix publish' and 'guix substitute'
(name . Ludovic Courtès)(address . ludo@gnu.org)(address . 45460@debbugs.gnu.org)
87bleb7i9u.fsf@gnu.org
Hello Ludo,
Toggle quote (7 lines)> Currently ‘guix substitute’ implements the same policy has before,> which is to pick the smallest archive when several compression> methods are proposed. The next step will be to make that> configurable.>> Thoughts?
Nice one! What do you have in mind to make it configurable? Pass thepreferred compression types to the daemon and propagate it to "guixsubstitute"?
Once we have better visibility on zstd, I think we should considermaking those bindings mandatory for consistency with the otherGuile compression libraries.
Otherwise, the whole patchset seems fine :).
Thanks,
Mathieu
L
L
Ludovic Courtès wrote on 3 Jan 16:17 +0100
(name . Mathieu Othacehe)(address . othacehe@gnu.org)(address . 45460@debbugs.gnu.org)
87y2hayrq6.fsf@gnu.org
Hi,
Mathieu Othacehe <othacehe@gnu.org> skribis:
Toggle quote (11 lines)>> Currently ‘guix substitute’ implements the same policy has before,>> which is to pick the smallest archive when several compression>> methods are proposed. The next step will be to make that>> configurable.>>>> Thoughts?>> Nice one! What do you have in mind to make it configurable? Pass the> preferred compression types to the daemon and propagate it to "guix> substitute"?
I’m not sure. I thought that perhaps there could be:
guix-daemon --download-strategy=[speed|bandwidth]
where ‘bandwidth’ would always optimize for bandwidth and thus use theexisting ‘file-size<?’ strategy in ‘narinfo-best-uri’.
For ‘speed’ it’s a bit more complicated though: you have to measurewhether downloads are actually CPU-bound to choose between (say) lzipand zstd/gzip. It’s doable but we’d have to say whether it can be maderobust enough.
Letting the user choose the compression type doesn’t have this drawbackbut it requires expertise from the user.
Toggle quote (4 lines)> Once we have better visibility on zstd, I think we should consider> making those bindings mandatory for consistency with the other> Guile compression libraries.
Yes.
Toggle quote (2 lines)> Otherwise, the whole patchset seems fine :).
Cool, thanks! I guess I’ll wait until Chris’ patch series has landedbefore pushing it, or at least the bit that touches ‘guix substitute’.
Ludo’.
L
L
Ludovic Courtès wrote on 3 Jan 16:18 +0100
Re: [bug#45460] [PATCH 3/5] publish: Add support for zstd compression.
(name . Jonathan Brielmaier)(address . jonathan.brielmaier@web.de)(address . 45460@debbugs.gnu.org)
87turyyrp5.fsf@gnu.org
Hallo!
Jonathan Brielmaier <jonathan.brielmaier@web.de> skribis:
Toggle quote (2 lines)> Nice thing, only one nitpick
[...]
Toggle quote (10 lines)>> +The compression ratio of @code{zstd} is between that of @code{lzip} and>> +that of @code{gzip}; its main advantage is a>> +@uref{https://facebook.github.io/zstd/,high decompression speed}.>>>> Unless @option{--cache} is used, compression occurs on the fly and>> the compressed streams are not>> It should be also documented at the `guix-publish-configuration` section> in the manual. As I guess that a lot of publish server admins look there...
Good point, I’ll add a cross-reference there.
Ludo’.
L
L
Ludovic Courtès wrote on 13 Jan 23:08 +0100
Re: bug#45460: [PATCH 0/5] Zstd support for 'guix publish' and 'guix substitute'
(name . Mathieu Othacehe)(address . othacehe@gnu.org)
87mtxc7ani.fsf_-_@gnu.org
Pushed as e28d2cdd753f5c224853f3d9ffe63f848709b41a, along withJonathan’s suggestion regarding the manual.
I’ll follow up with a ‘guix’ package update and adding the dependency onguile-zstd.
Ludo’.
L
L
Ludovic Courtès wrote on 13 Jan 23:08 +0100
control message for bug #45460
(address . control@debbugs.gnu.org)
87lfcw7amy.fsf@gnu.org
tags 45460 fixedclose 45460 quit
?
Your comment

Commenting via the web interface is currently disabled.

To comment on this conversation send email to 45460@debbugs.gnu.org