| Message ID | 20210715073328.212123-2-zimon.toutoune@gmail.com |
|---|---|
| State | New |
| Headers | show |
| Series | DRAFT "guix search" performances | 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 |
Hi Simon, I understand that one of the things you are trying to do is to have a common interface for the cache and no-cache cases. To achieve this, I think fold-available-packages and fold-packages should have the same function signature. They should both pass a <package> object to PROC. Currently, fold-packages is passing a <package> object whereas fold-available-packages is passing the fields of the <package> object as individual parameters. If fold-packages and fold-available-packages have the same function signature, then the changes in your [PATCH v6 2/2] would be way simpler. Also, why do we need two separate functions---fold-available-packages and fold-packages? Can't fold-available-packages do everything fold-packages can and thus totally replace it? > * gnu/packages.scm (generate-package-cache)[expand-cache]: Add synopsis and > description. > (load-package-cache, find-packages-by-names, find-packages-locations): Adapt > accordingly. A couple of typos here: find-packages-by-names -> find-packages-by-name find-packages-locations -> find-package-locations Regards, Arun
Hi Arun! Arun Isaac <arunisaac@systemreboot.net> skribis: > Also, why do we need two separate functions---fold-available-packages > and fold-packages? Can't fold-available-packages do everything > fold-packages can and thus totally replace it? The initial goal was for ‘fold-available-packages’ to be lightweight. Currently, it doesn’t allocate anything; instead, it passes info as keyword parameters, which the callee is free to ignore. That’s why these two procedures have different signatures. One benchmark is “guix package -A > /dev/null”. This should take ideally 0.5s at most because that’s what’s used by shell completion (the first time); currently it takes 0.82s on my laptop, though. Thanks, Ludo’.
Hi Arun and Ludo, Thanks for the review. On Sat, 17 Jul 2021 at 14:01, Arun Isaac <arunisaac@systemreboot.net> wrote: > I understand that one of the things you are trying to do is to have a > common interface for the cache and no-cache cases. To achieve this, I > think fold-available-packages and fold-packages should have the same > function signature. They should both pass a <package> object to > PROC. Currently, fold-packages is passing a <package> object whereas > fold-available-packages is passing the fields of the <package> object as > individual parameters. If fold-packages and fold-available-packages have > the same function signature, then the changes in your [PATCH v6 2/2] > would be way simpler. I agree. Previously [1], I created ’fold-packages*’ which was a cached ’fold-packages’ and Ludo answered [2]: Did you see ‘fold-available-packages’? It seems you could extend it instead of introducing ‘fold-packages*’, no? therefore, it is somehow another attempt on the other side. :-) 1: <http://issues.guix.gnu.org/39258#91> 2: <http://issues.guix.gnu.org/39258#93> > Also, why do we need two separate functions---fold-available-packages > and fold-packages? Can't fold-available-packages do everything > fold-packages can and thus totally replace it? To be honest, I have been lazy because unifying ’fold-available-packages’ and ’fold-packages’ means to change the signature and so a bit of refactoring. And as Ludo explained, ’fold-available-packages’ has to be as light as possible because it is used by Emacs-Guix and maybe Nyxt for completion. :-) >> * gnu/packages.scm (generate-package-cache)[expand-cache]: Add synopsis and >> description. >> (load-package-cache, find-packages-by-names, find-packages-locations): Adapt >> accordingly. > > A couple of typos here: > > find-packages-by-names -> find-packages-by-name > find-packages-locations -> find-package-locations Thanks for the spot. On Fri, 23 Jul 2021 at 17:30, Ludovic Courtès <ludo@gnu.org> wrote: > One benchmark is “guix package -A > /dev/null”. This should take > ideally 0.5s at most because that’s what’s used by shell completion (the > first time); currently it takes 0.82s on my laptop, though. On cold cache, on my laptop: --8<---------------cut here---------------start------------->8--- $ time guix package -A > /dev/null real 0m1.717s user 0m2.526s sys 0m0.083s --8<---------------cut here---------------end--------------->8--- and on my (slow) desktop: real 0m6.196s user 0m2.008s sys 0m0.093s Warn cache: laptop desktop real 0m1.425s 0m1.217s user 0m2.505s 0m1.901s sys 0m0.033s 0m0.051s Well, another story I guess. :-) Cheers, simon
diff --git a/gnu/packages.scm b/gnu/packages.scm index ccfc83dd11..34c6d73b86 100644 --- a/gnu/packages.scm +++ b/gnu/packages.scm @@ -4,6 +4,7 @@ ;;; Copyright © 2014 Eric Bavier <bavier@member.fsf.org> ;;; Copyright © 2016, 2017 Alex Kost <alezost@gmail.com> ;;; Copyright © 2016 Mathieu Lirzin <mthl@gnu.org> +;;; Copyright © 2021 Simon Tournier <zimon.toutoune@gmail.com> ;;; ;;; This file is part of GNU Guix. ;;; @@ -211,28 +212,45 @@ package module." (vhash-fold (lambda (name vector result) (match vector (#(name version module symbol outputs + synopsis description supported? deprecated? file line column) (proc name version result #:outputs outputs + #:synopsis synopsis + #:description description #:location (and file (location file line column)) + #:module module + #:symbol symbol #:supported? supported? #:deprecated? deprecated?)))) init cache) - (fold-packages (lambda (package result) - (proc (package-name package) - (package-version package) - result - #:outputs (package-outputs package) - #:location (package-location package) - #:supported? - (->bool (supported-package? package)) - #:deprecated? - (->bool - (package-superseded package)))) - init))) + (fold-module-public-variables* + (lambda (module symbol variable result) + (let ((package (false-if-exception + (variable-ref variable)))) + (if (package? package) + (proc (package-name package) + (package-version package) + result + #:outputs (package-outputs package) + #:synopsis (package-synopsis package) + #:description (package-description package) + #:location (package-location package) + #:module (module-name module) + #:symbol symbol + #:supported? + (->bool (supported-package? package)) + #:deprecated? + (->bool + (package-superseded package))) + result))) + init + (all-modules (%package-module-path) + #:warn + warn-about-load-error)))) (define* (fold-packages proc init #:optional @@ -268,6 +286,7 @@ package names. Return #f on failure." (fold (lambda (item vhash) (match item (#(name version module symbol outputs + synopsis description supported? deprecated? file line column) (vhash-cons name item vhash)))) @@ -316,7 +335,7 @@ decreasing version order." (if (and (cache-is-authoritative?) cache) (match (cache-lookup cache name) (#f #f) - ((#(_ versions modules symbols _ _ _ _ _ _) ...) + ((#(_ versions modules symbols _ _ _ _ _ _ _ _) ...) (fold (lambda (version* module symbol result) (if (or (not version) (version-prefix? version version*)) @@ -337,9 +356,8 @@ matching NAME and VERSION." (if (and cache (cache-is-authoritative?)) (match (cache-lookup cache name) (#f '()) - ((#(name versions modules symbols outputs - supported? deprecated? - files lines columns) ...) + ((#(_ versions _ _ _ _ _ _ _ + files lines columns) ...) (fold (lambda (version* file line column result) (if (and file (or (not version) @@ -393,6 +411,8 @@ reducing the memory footprint." ,(module-name module) ,symbol ,(package-outputs package) + ,(package-synopsis package) + ,(package-description package) ,(->bool (supported-package? package)) ,(->bool (package-superseded package)) ,@(let ((loc (package-location package)))