Message ID | 87wnn7ouq3.fsf@airmail.cc |
---|---|
State | New |
Headers | show |
Series | [bug#50755,v2] import: Generate list of importers based on available modules | 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 |
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, Thanks. Two comments. On Thu, 23 Sept 2021 at 23:20, pinoaffe <pinoaffe@airmail.cc> wrote: > -(define importers '("gnu" "pypi" "cpan" "hackage" "stackage" "egg" "elpa" > - "gem" "go" "cran" "crate" "texlive" "json" "opam" > - "minetest")) > +(define importers (filter-map (lambda (module) > + (match (module-name module) > + (`(guix scripts import ,importer) > + (symbol->string importer)) > + ( #t #f))) > + (all-modules (map (lambda (entry) > + `(,entry . "guix/scripts/import")) > + %load-path)))) > > (define (resolve-importer name) > (let ((module (resolve-interface First, I think, it breaks "guix import --help". Therefore, this patch needs a v3. :-) Second, what is the average extra time added on cold cache? On my machine, for hot cache, I get: --8<---------------cut here---------------start------------->8--- $ time guix import cran -h real 0m0.113s user 0m0.110s sys 0m0.025s $ time ./pre-inst-env guix import cran -h real 0m0.470s user 0m0.529s sys 0m0.054s --8<---------------cut here---------------end--------------->8--- which is something. On cold cache, it is: real 0m10.438s user 0m0.164s sys 0m0.082s vs real 0m12.226s user 0m0.897s sys 0m0.190s but these numbers are not so much meaningful because there is a strong variability; hence on average. :-) Because of 'filter-map', it walks all the modules, so there is a performance loss. The question is: which performance loss is acceptable here? Other said, is the code improvement worth compared to the performance decrease? All the best, simon
Hi, thank you for your feedback! zimoun writes: > First, I think, it breaks "guix import --help". Therefore, this patch > needs a v3. :-) I sent a v3, thanks :) > The question is: which performance loss is acceptable here? To me a performance loss similar to the one you describe would be acceptable, particularly since it should be a constant performance hit for every time ~guix import~ is called, aka it won't significantly impact long-running import commands. However, I think I may be somewhat biased on this one, so it'd be great if others could weigh in :) Kind regards, pinoaffe
diff --git a/guix/scripts/import.scm b/guix/scripts/import.scm index 40fa6759ae..ed702d3bff 100644 --- a/guix/scripts/import.scm +++ b/guix/scripts/import.scm @@ -23,6 +23,7 @@ (define-module (guix scripts import) #:use-module (guix ui) + #:use-module (guix discovery) #:use-module (guix scripts) #:use-module (guix utils) #:use-module (srfi srfi-1) @@ -78,9 +79,14 @@ rather than \\n." ;;; Entry point. ;;; -(define importers '("gnu" "pypi" "cpan" "hackage" "stackage" "egg" "elpa" - "gem" "go" "cran" "crate" "texlive" "json" "opam" - "minetest")) +(define importers (filter-map (lambda (module) + (match (module-name module) + (`(guix scripts import ,importer) + (symbol->string importer)) + ( #t #f))) + (all-modules (map (lambda (entry) + `(,entry . "guix/scripts/import")) + %load-path)))) (define (resolve-importer name) (let ((module (resolve-interface