GNU bug report logs -
#69291
[PATCH 0/5] Start making substitute code less coupled
Previous Next
Reported by: Christopher Baines <mail <at> cbaines.net>
Date: Tue, 20 Feb 2024 19:21:02 UTC
Severity: normal
Tags: patch
Done: Christopher Baines <mail <at> cbaines.net>
Bug is archived. No further changes may be made.
Full log
Message #32 received at 69291 <at> debbugs.gnu.org (full text, mbox):
Christopher Baines <mail <at> cbaines.net> skribis:
> Pulling the logic up to the script makes this code more portable and not
> reliant on setting a global variable.
>
> * guix/scripts/substitute.scm (%prefer-fast-decompression?): Rename to…
> (%default-prefer-fast-decompression?): this.
> (display-narinfo-data): Update accordingly.
> (download-nar): Add prefer-fast-decompression? as a keyword argument, remove
> code to set! it and return the cpu-usage recorded.
> (process-substitution, process-substitution/fallback): Accept and pass through
> prefer-fast-decompression? to download-nar.
> (guix-substitute): Move the prefer fast decompression switching logic here.
>
> Change-Id: I4e80b457b55bcda8c0ff4ee224dd94a55e1b24fb
[...]
> -(define %prefer-fast-decompression?
> - ;; Whether to prefer fast decompression over good compression ratios. This
> - ;; serves in particular to choose between lzip (high compression ratio but
> - ;; low decompression throughput) and zstd (lower compression ratio but high
> - ;; decompression throughput).
> - #f)
> +(define %default-prefer-fast-decompression? #f)
I would either remove this variable or add a comment describing it (we
should do that for all top-level variables).
> @@ -604,7 +585,9 @@ (define* (download-nar narinfo destination
> (format status-port "hash-mismatch ~a ~a ~a~%"
> (hash-algorithm-name algorithm)
> (bytevector->nix-base32-string expected)
> - (bytevector->nix-base32-string actual)))))))
> + (bytevector->nix-base32-string actual))))
> +
> + cpu-usage)))
[...]
> + (let ((cpu-usage
> + (process-substitution reply-port store-path destination
> + #:cache-urls (substitute-urls)
> + #:acl (current-acl)
> + #:deduplicate? deduplicate?
> + #:print-build-trace?
> + print-build-trace?
> + #:prefer-fast-decompression?
> + prefer-fast-decompression?)))
> +
> + ;; Create a hysteresis: depending on CPU usage, favor
> + ;; compression methods with faster decompression (like ztsd)
> + ;; or methods with better compression ratios (like lzip).
> + ;; This stems from the observation that substitution can be
> + ;; CPU-bound when high-speed networks are used:
> + ;; <https://lists.gnu.org/archive/html/guix-devel/2020-12/msg00177.html>.
> + ;; To simulate "slow" networking or changing conditions, run:
> + ;; sudo tc qdisc add dev eno1 root tbf rate 512kbit latency
> + ;; 50ms burst 1540 and then cancel with: sudo tc qdisc del
> + ;; dev eno1 root
> + (loop (cond
> + ;; Whether to prefer fast decompression over good
> + ;; compression ratios. This serves in particular to
> + ;; choose between lzip (high compression ratio but low
> + ;; decompression throughput) and zstd (lower
> + ;; compression ratio but high decompression
> + ;; throughput).
> + ((> cpu-usage .8) #t)
> + ((< cpu-usage .2) #f)
> + (else prefer-fast-decompression?)))))))))
Instead of having ‘download-nar’ return its CPU usage, which is
surprising, maybe should wrap the ‘process-substitution’ call in
‘guix-substitute’ in ‘with-cpu-usage-monitoring’ and keep all the logic
in ‘guix-substitute’?
Ludo’.
This bug report was last modified 1 year and 29 days ago.
Previous Next
GNU bug tracking system
Copyright (C) 1999 Darren O. Benham,
1997,2003 nCipher Corporation Ltd,
1994-97 Ian Jackson.