GNU bug report logs - #48853
[PATCH] profiles: Move some of the work to the build side.

Previous Next

Package: guix-patches;

Reported by: Ludovic Courtès <ludo <at> gnu.org>

Date: Sat, 5 Jun 2021 21:10:01 UTC

Severity: normal

Tags: patch

Done: Ludovic Courtès <ludo <at> gnu.org>

Bug is archived. No further changes may be made.

To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 48853 in the body.
You can then email your comments to 48853 AT debbugs.gnu.org in the normal way.

Toggle the display of automated, internal messages from the tracker.

View this report as an mbox folder, status mbox, maintainer mbox


Report forwarded to guix-patches <at> gnu.org:
bug#48853; Package guix-patches. (Sat, 05 Jun 2021 21:10:01 GMT) Full text and rfc822 format available.

Acknowledgement sent to Ludovic Courtès <ludo <at> gnu.org>:
New bug report received and forwarded. Copy sent to guix-patches <at> gnu.org. (Sat, 05 Jun 2021 21:10:02 GMT) Full text and rfc822 format available.

Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Ludovic Courtès <ludo <at> gnu.org>
To: guix-patches <at> gnu.org
Cc: Ludovic Courtès <ludo <at> gnu.org>
Subject: [PATCH] profiles: Move some of the work to the build side.
Date: Sat,  5 Jun 2021 23:08:56 +0200
When running:

  guix environment --ad-hoc gnome --no-grafts --search-paths

this reduces wall-clock time by ~5%.  The number of object cache lookups
goes down from 96K to 89K.  (Note that 'gnome' is an interesting example
because it has many propagated inputs, which themselves have propagated
inputs too, which would lead to a long input list and a long manifest in
the 'profile-derivation' gexp.)

* guix/profiles.scm (profile-derivation)[inputs, search-paths]: Remove.
[extra-inputs]: New variable.
[builder]: Adjust call to 'build-profile'.
* guix/build/profiles.scm (manifest-sexp->inputs+search-paths): New
procedure.
(build-profile): Remove 'inputs' parameter; make 'manifest' the 2nd
positional parameter and add #:extra-inputs.  Call
'manifest-sexp->inputs+search-paths' to obtain 'inputs' and
'search-paths'.
---
 guix/build/profiles.scm | 86 +++++++++++++++++++++++++++--------------
 guix/profiles.scm       | 25 ++++--------
 2 files changed, 64 insertions(+), 47 deletions(-)

Hi!

This is a rather modest optimization of the code that computes the
derivation of a profile, probably only measurable for profiles with
lots of packages or lots of propagated inputs (as can happen with
R/Python packages).

Let me know what difference it makes for your use cases!

Thanks,
Ludo'.

diff --git a/guix/build/profiles.scm b/guix/build/profiles.scm
index a40c3f96de..9249977bed 100644
--- a/guix/build/profiles.scm
+++ b/guix/build/profiles.scm
@@ -20,6 +20,8 @@
   #:use-module (guix build union)
   #:use-module (guix build utils)
   #:use-module (guix search-paths)
+  #:use-module (srfi srfi-1)
+  #:use-module (srfi srfi-11)
   #:use-module (srfi srfi-26)
   #:use-module (ice-9 ftw)
   #:use-module (ice-9 match)
@@ -143,45 +145,71 @@ instead make DIRECTORY a \"real\" directory containing symlinks."
                         directory))))
             (apply throw args))))))
 
-(define* (build-profile output inputs
-                        #:key manifest search-paths
-                        (symlink symlink))
-  "Build a user profile from INPUTS in directory OUTPUT, using SYMLINK to
-create symlinks.  Write MANIFEST, an sexp, to OUTPUT/manifest.  Create
-OUTPUT/etc/profile with Bash definitions for -all the variables listed in
-SEARCH-PATHS."
+(define (manifest-sexp->inputs+search-paths manifest)
+  "Parse MANIFEST, an sexp as produced by 'manifest->gexp', and return two
+values: the list of store items of its manifest entries, and the list of
+search path specifications."
+  (match manifest                            ;this must match 'manifest->gexp'
+    (('manifest ('version 3)
+                ('packages (entries ...)))
+     (let loop ((entries entries)
+                (inputs '())
+                (search-paths '()))
+       (match entries
+         (((name version output item
+                 ('propagated-inputs deps)
+                 ('search-paths paths) _ ...) . rest)
+          (loop (append deps rest)
+                (cons item inputs)
+                (append paths search-paths)))
+         (()
+          (values inputs
+                  (delete-duplicates
+                   (cons $PATH
+                         (map sexp->search-path-specification
+                              search-paths))))))))))
+
+(define* (build-profile output manifest
+                        #:key (extra-inputs '()) (symlink symlink))
+  "Build a user profile from MANIFEST, an sexp, and EXTRA-INPUTS, a list of
+store items, in directory OUTPUT, using SYMLINK to create symlinks.  Create
+OUTPUT/etc/profile with Bash definitions for all the variables listed in the
+search paths of MANIFEST's entries."
   (define manifest-file
     (string-append output "/manifest"))
 
-  ;; Make the symlinks.
-  (union-build output inputs
-               #:symlink symlink
-               #:log-port (%make-void-port "w"))
+  (let-values (((inputs search-paths)
+                (manifest-sexp->inputs+search-paths manifest)))
 
-  ;; If one of the INPUTS provides a '/manifest' file, delete it.  That can
-  ;; happen if MANIFEST contains something such as a Guix instance, which is
-  ;; ultimately built as a profile.
-  (when (file-exists? manifest-file)
-    (delete-file manifest-file))
+    ;; Make the symlinks.
+    (union-build output (append extra-inputs inputs)
+                 #:symlink symlink
+                 #:log-port (%make-void-port "w"))
 
-  ;; Store meta-data.
-  (call-with-output-file manifest-file
-    (lambda (p)
-      (display "\
+    ;; If one of the INPUTS provides a '/manifest' file, delete it.  That can
+    ;; happen if MANIFEST contains something such as a Guix instance, which is
+    ;; ultimately built as a profile.
+    (when (file-exists? manifest-file)
+      (delete-file manifest-file))
+
+    ;; Store meta-data.
+    (call-with-output-file manifest-file
+      (lambda (p)
+        (display "\
 ;; This file was automatically generated and is for internal use only.
 ;; It cannot be passed to the '--manifest' option.
 ;; Run 'guix package --export-manifest' if you want to export a file
 ;; suitable for '--manifest'.\n\n"
-               p)
-      (pretty-print manifest p)))
+                 p)
+        (pretty-print manifest p)))
 
-  ;; Make sure we can write to 'OUTPUT/etc'.  'union-build' above could have
-  ;; made 'etc' a symlink to a read-only sub-directory in the store so we need
-  ;; to work around that.
-  (ensure-writable-directory (string-append output "/etc")
-                             #:symlink symlink)
+    ;; Make sure we can write to 'OUTPUT/etc'.  'union-build' above could have
+    ;; made 'etc' a symlink to a read-only sub-directory in the store so we
+    ;; need to work around that.
+    (ensure-writable-directory (string-append output "/etc")
+                               #:symlink symlink)
 
-  ;; Write 'OUTPUT/etc/profile'.
-  (build-etc/profile output search-paths))
+    ;; Write 'OUTPUT/etc/profile'.
+    (build-etc/profile output search-paths)))
 
 ;;; profile.scm ends here
diff --git a/guix/profiles.scm b/guix/profiles.scm
index ed5c10315a..8cbffa4d2b 100644
--- a/guix/profiles.scm
+++ b/guix/profiles.scm
@@ -1812,12 +1812,10 @@ are cross-built for TARGET."
                                    (mapm/accumulate-builds (lambda (hook)
                                                              (hook manifest))
                                                            hooks))))
-    (define inputs
-      (append (filter-map (lambda (drv)
-                            (and (derivation? drv)
-                                 (gexp-input drv)))
-                          extras)
-              (manifest-inputs manifest)))
+    (define extra-inputs
+      (filter-map (lambda (drv)
+                    (and (derivation? drv) (gexp-input drv)))
+                  extras))
 
     (define glibc-utf8-locales                    ;lazy reference
       (module-ref (resolve-interface '(gnu packages base))
@@ -1851,20 +1849,11 @@ are cross-built for TARGET."
 
             #+(if locales? set-utf8-locale #t)
 
-            (define search-paths
-              ;; Search paths of MANIFEST's packages, converted back to their
-              ;; record form.
-              (map sexp->search-path-specification
-                   (delete-duplicates
-                    '#$(map search-path-specification->sexp
-                            (manifest-search-paths manifest)))))
-
-            (build-profile #$output '#$inputs
+            (build-profile #$output '#$(manifest->gexp manifest)
+                           #:extra-inputs '#$extra-inputs
                            #:symlink #$(if relative-symlinks?
                                            #~symlink-relative
-                                           #~symlink)
-                           #:manifest '#$(manifest->gexp manifest)
-                           #:search-paths search-paths))))
+                                           #~symlink)))))
 
     (gexp->derivation name builder
                       #:system system
-- 
2.31.1





Reply sent to Ludovic Courtès <ludo <at> gnu.org>:
You have taken responsibility. (Fri, 11 Jun 2021 22:56:01 GMT) Full text and rfc822 format available.

Notification sent to Ludovic Courtès <ludo <at> gnu.org>:
bug acknowledged by developer. (Fri, 11 Jun 2021 22:56:02 GMT) Full text and rfc822 format available.

Message #10 received at 48853-done <at> debbugs.gnu.org (full text, mbox):

From: Ludovic Courtès <ludo <at> gnu.org>
To: 48853-done <at> debbugs.gnu.org
Subject: Re: bug#48853: [PATCH] profiles: Move some of the work to the build
 side.
Date: Sat, 12 Jun 2021 00:55:10 +0200
Ludovic Courtès <ludo <at> gnu.org> skribis:

> When running:
>
>   guix environment --ad-hoc gnome --no-grafts --search-paths
>
> this reduces wall-clock time by ~5%.  The number of object cache lookups
> goes down from 96K to 89K.  (Note that 'gnome' is an interesting example
> because it has many propagated inputs, which themselves have propagated
> inputs too, which would lead to a long input list and a long manifest in
> the 'profile-derivation' gexp.)
>
> * guix/profiles.scm (profile-derivation)[inputs, search-paths]: Remove.
> [extra-inputs]: New variable.
> [builder]: Adjust call to 'build-profile'.
> * guix/build/profiles.scm (manifest-sexp->inputs+search-paths): New
> procedure.
> (build-profile): Remove 'inputs' parameter; make 'manifest' the 2nd
> positional parameter and add #:extra-inputs.  Call
> 'manifest-sexp->inputs+search-paths' to obtain 'inputs' and
> 'search-paths'.
> ---
>  guix/build/profiles.scm | 86 +++++++++++++++++++++++++++--------------
>  guix/profiles.scm       | 25 ++++--------
>  2 files changed, 64 insertions(+), 47 deletions(-)

Pushed as 8cef92d0633850d97c1a1d4521812268f56672be.




bug archived. Request was from Debbugs Internal Request <help-debbugs <at> gnu.org> to internal_control <at> debbugs.gnu.org. (Sat, 10 Jul 2021 11:24:05 GMT) Full text and rfc822 format available.

This bug report was last modified 2 years and 262 days ago.

Previous Next


GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.