| Message ID | bef2c9a9c67b2f6d6c50aa7f9fab3f263290d1b2.camel@gmail.com |
|---|---|
| State | New |
| Headers | show |
| Series | [bug#57704,v2] guix: packages: Remove #f from inputs when sanitizing. | expand |
| Context | Check | Description |
|---|---|---|
| cbaines/comparison | success | View comparision |
| cbaines/git-branch | success | View Git branch |
| cbaines/applying patch | success | View Laminar job |
| cbaines/issue | success | View issue |
On 09-09-2022 17:56, Liliana Marie Prikler wrote: > This makes it so that new-style inputs can be optional using regular Guile > patterns, e.g. (and (target-x86-64?) rust). > > * guix/packages.scm (sanitize-inputs): Filter inputs by identity before adding > labels. > --- > As noted by Maxime, this doesn't seem to be cause any rebuilds, so retargeting > master. Also added missing documentation. The docstring is nice, but with documentation, I meant the manual, presumably in ‘(guix)package Reference’, maybe also in the packaging tutorial in the cookbook. Also, something I forgot: performance. sanitize-inputs is called for every package, and the change adds additional memory allocations (due to the use of 'filter'), is there an observable performance impact (maybe "GUIX_PROFILING=gc time guix refresh -l pkg-config" would be a good test)? If there is, some optimisations may be in order Greetings, Maxime.
Am Samstag, dem 10.09.2022 um 02:33 +0200 schrieb Maxime Devos: > The docstring is nice, but with documentation, I meant the manual, > presumably in ‘(guix)package Reference’, maybe also in the packaging > tutorial in the cookbook. I don't see the current practice documented, so I think we're actually good on this front. > Also, something I forgot: performance. sanitize-inputs is called for > every package, and the change adds additional memory allocations (due > to the use of 'filter'), is there an observable performance impact > (maybe "GUIX_PROFILING=gc time guix refresh -l pkg-config" would be a > good test)? If there is, some optimisations may be in order Looking at the numbers below Garbage collection statistics: heap size: 212.66 MiB allocated: 739.15 MiB GC times: 20 time spent in GC: 5.30 seconds (65% of user time) real 0m3,606s user 0m8,140s sys 0m0,109s Garbage collection statistics: heap size: 276.29 MiB allocated: 1292.10 MiB GC times: 28 time spent in GC: 10.48 seconds (64% of user time) real 0m11,638s user 0m16,422s sys 0m0,308s it does appear that this increases times by a factor of two. Use of filter! instead of filter brings only marginal benefits. I'll check if we could instead simply ignore unspecified? values when collecting the inputs – that would allow the natural use of (when) and (unless). Cheers
Am Samstag, dem 10.09.2022 um 08:40 +0200 schrieb Liliana Marie Prikler: > Looking at the numbers below > [...] > it does appear that this increases times by a factor of two. It seems I've been comparing apples to oranges. Running ./pre-inst-env already increases the times for guix: Garbage collection statistics: heap size: 276.29 MiB allocated: 1291.91 MiB GC times: 28 time spent in GC: 9.39 seconds (66% of user time) real 0m6,069s user 0m14,172s sys 0m0,140s An alternative patch that I'll submit as v3 adds little to these times: Garbage collection statistics: heap size: 276.29 MiB allocated: 1291.96 MiB GC times: 28 time spent in GC: 9.32 seconds (66% of user time) real 0m6,124s user 0m14,138s sys 0m0,147s Cheers
diff --git a/guix/packages.scm b/guix/packages.scm index 94e464cd01..7569380610 100644 --- a/guix/packages.scm +++ b/guix/packages.scm @@ -428,13 +428,14 @@ (define %cuirass-supported-systems (fold delete %supported-systems '("mips64el-linux" "powerpc-linux" "riscv64-linux"))) (define-inlinable (sanitize-inputs inputs) - "Sanitize INPUTS by turning it into a list of name/package tuples if it's -not already the case." - (cond ((null? inputs) inputs) - ((and (pair? (car inputs)) - (string? (caar inputs))) - inputs) - (else (map add-input-label inputs)))) + "Sanitize INPUTS by removing falsy elements and turning it into a list of +name/package tuples if it's not already the case." + (let ((inputs (filter identity inputs))) + (cond ((null? inputs) inputs) + ((and (pair? (car inputs)) + (string? (caar inputs))) + inputs) + (else (map add-input-label inputs))))) (define-syntax current-location-vector (lambda (s)