GNU bug report logs - #53034
[PATCH] shell: Cache profiles even when using package specs.

Previous Next

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

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

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] shell: Cache profiles even when using package specs.
Date: Wed,  5 Jan 2022 19:45:00 +0100

This enables profile caching not just when '-m' or '-f' is used, but
also when package specs are passed on the command line, as in:

  guix shell -D guix git

It also changes profile cache keys to include the system type, which was
previously ignored.

* guix/scripts/shell.scm (options-with-caching)[single-file-for-caching]:
Remove.
Call 'profile-cached-gc-root' instead; adjust to accept two values.
(profile-cache-primary-key): New procedure.
(profile-cache-key): Remove.
(profile-file-cache-key, profile-spec-cache-key): New procedures.
(profile-cached-gc-root): Rewrite to include functionality formally in
'single-file-for-caching', but extend to handle package specs.
* gnu/packages.scm (cache-is-authoritative?): Export.
* guix/transformations.scm (transformation-option-key?): New procedure.
---
 gnu/packages.scm         |   3 +-
 guix/scripts/shell.scm   | 161 +++++++++++++++++++++++++--------------
 guix/transformations.scm |   9 ++-
 3 files changed, 113 insertions(+), 60 deletions(-)

Hi there!

I was frustrated that I would not benefit from the profile cache when
running:

  guix shell -D guix

or, more interestingly:

  guix shell supertuxkart -- supertuxkart

This patch fixes that.  Thus, on cache hits, any such command runs in ~0.1s.
Well, supertuxkart might run in more like ~1h, but that’s another story.

There are still cases where caching is disabled: when the package cache is
not authoritative (i.e., -L is used or ‘GUIX_PACKAGE_PATH’ is set), when
transformation options are used (because options such as ‘--with-branch’
depend on external state that may change behind our back), and when using
‘-e’ (because we don’t know what the given expression is doing).

Thoughts?

Ludo’.

diff --git a/gnu/packages.scm b/gnu/packages.scm
index ccfc83dd11..65ab7a7c1e 100644
--- a/gnu/packages.scm
+++ b/gnu/packages.scm
@@ -1,5 +1,5 @@
 ;;; GNU Guix --- Functional package management for GNU
-;;; Copyright © 2012, 2013, 2014, 2015, 2016, 2017, 2018, 2019, 2020 Ludovic Courtès <ludo <at> gnu.org>
+;;; Copyright © 2012-2020, 2022 Ludovic Courtès <ludo <at> gnu.org>
 ;;; Copyright © 2013 Mark H Weaver <mhw <at> netris.org>
 ;;; Copyright © 2014 Eric Bavier <bavier <at> member.fsf.org>
 ;;; Copyright © 2016, 2017 Alex Kost <alezost <at> gmail.com>
@@ -51,6 +51,7 @@ (define-module (gnu packages)
             %auxiliary-files-path
             %package-module-path
             %default-package-module-path
+            cache-is-authoritative?
 
             fold-packages
             fold-available-packages
diff --git a/guix/scripts/shell.scm b/guix/scripts/shell.scm
index 546639818f..12b6f18200 100644
--- a/guix/scripts/shell.scm
+++ b/guix/scripts/shell.scm
@@ -1,5 +1,5 @@
 ;;; GNU Guix --- Functional package management for GNU
-;;; Copyright © 2021 Ludovic Courtès <ludo <at> gnu.org>
+;;; Copyright © 2021-2022 Ludovic Courtès <ludo <at> gnu.org>
 ;;;
 ;;; This file is part of GNU Guix.
 ;;;
@@ -21,7 +21,8 @@ (define-module (guix scripts shell)
   #:use-module ((guix diagnostics) #:select (location))
   #:use-module (guix scripts environment)
   #:autoload   (guix scripts build) (show-build-options-help)
-  #:autoload   (guix transformations) (show-transformation-options-help)
+  #:autoload   (guix transformations) (transformation-option-key?
+                                       show-transformation-options-help)
   #:use-module (guix scripts)
   #:use-module (guix packages)
   #:use-module (guix profiles)
@@ -40,6 +41,7 @@ (define-module (guix scripts shell)
   #:use-module ((guix build utils) #:select (mkdir-p))
   #:use-module (guix cache)
   #:use-module ((ice-9 ftw) #:select (scandir))
+  #:autoload   (gnu packages) (cache-is-authoritative?)
   #:export (guix-shell))
 
 (define (show-help)
@@ -201,51 +203,35 @@ (define (authorized-shell-directory? directory)
     (const #f)))
 
 (define (options-with-caching opts)
-  "If OPTS contains exactly one 'load' or one 'manifest' key, automatically
-add a 'profile' key (when a profile for that file is already in cache) or a
-'gc-root' key (to add the profile to cache)."
-  (define (single-file-for-caching opts)
-    (let loop ((opts opts)
-               (file #f))
-      (match opts
-        (() file)
-        ((('package . _) . _) #f)
-        ((('load . ('package candidate)) . rest)
-         (and (not file) (loop rest candidate)))
-        ((('manifest . candidate) . rest)
-         (and (not file) (loop rest candidate)))
-        ((('expression . _) . _) #f)
-        ((_ . rest) (loop rest file)))))
-
-  ;; Check whether there's a single 'load' or 'manifest' option.  When that is
-  ;; the case, arrange to automatically cache the resulting profile.
-  (match (single-file-for-caching opts)
-    (#f opts)
-    (file
-     (let* ((root (profile-cached-gc-root file))
-            (stat (and root (false-if-exception (lstat root)))))
-       (if (and (not (assoc-ref opts 'rebuild-cache?))
-                stat
-                (<= (stat:mtime ((@ (guile) stat) file))
-                    (stat:mtime stat)))
-           (let ((now (current-time)))
-             ;; Update the atime on ROOT to reflect usage.
-             (utime root
-                    now (stat:mtime stat) 0 (stat:mtimensec stat)
-                    AT_SYMLINK_NOFOLLOW)
-             (alist-cons 'profile root
-                         (remove (match-lambda
-                                   (('load . _) #t)
-                                   (('manifest . _) #t)
-                                   (_ #f))
-                                 opts)))          ;load right away
-           (if (and root (not (assq-ref opts 'gc-root)))
-               (begin
-                 (if stat
-                     (delete-file root)
-                     (mkdir-p (dirname root)))
-                 (alist-cons 'gc-root root opts))
-               opts))))))
+  "If OPTS contains only options that allow us to compute a cache key,
+automatically add a 'profile' key (when a profile for that file is already in
+cache) or a 'gc-root' key (to add the profile to cache)."
+  ;; Attempt to compute a file name for use as the cached profile GC root.
+  (let* ((root timestamp (profile-cached-gc-root opts))
+         (stat (and root (false-if-exception (lstat root)))))
+    (if (and (not (assoc-ref opts 'rebuild-cache?))
+             stat
+             (<= timestamp (stat:mtime stat)))
+        (let ((now (current-time)))
+          ;; Update the atime on ROOT to reflect usage.
+          (utime root
+                 now (stat:mtime stat) 0 (stat:mtimensec stat)
+                 AT_SYMLINK_NOFOLLOW)
+          (alist-cons 'profile root
+                      (remove (match-lambda
+                                (('load . _) #t)
+                                (('manifest . _) #t)
+                                (('package . _) #t)
+                                (('ad-hoc-package . _) #t)
+                                (_ #f))
+                              opts)))             ;load right away
+        (if (and root (not (assq-ref opts 'gc-root)))
+            (begin
+              (if stat
+                  (delete-file root)
+                  (mkdir-p (dirname root)))
+              (alist-cons 'gc-root root opts))
+            opts))))
 
 (define (auto-detect-manifest opts)
   "If OPTS do not specify packages or a manifest, load a \"guix.scm\" or
@@ -308,28 +294,87 @@ (define %profile-cache-directory
   (make-parameter (string-append (cache-directory #:ensure? #f)
                                  "/profiles")))
 
-(define (profile-cache-key file)
+(define (profile-cache-primary-key)
+  "Return the \"primary key\" used when computing keys for the profile cache.
+Return #f if no such key can be obtained and caching cannot be
+performed--e.g., because the package cache is not authoritative."
+  (and (cache-is-authoritative?)
+       (match (current-channels)
+         (()
+          #f)
+         (((= channel-commit commits) ...)
+          (string-join commits)))))
+
+(define (profile-file-cache-key file system)
   "Return the cache key for the profile corresponding to FILE, a 'guix.scm' or
 'manifest.scm' file, or #f if we lack channel information."
-  (match (current-channels)
-    (() #f)
-    (((= channel-commit commits) ...)
+  (match (profile-cache-primary-key)
+    (#f #f)
+    (primary-key
      (let ((stat (stat file)))
        (bytevector->base32-string
         ;; Since FILE is not canonicalized, only include the device/inode
         ;; numbers.  XXX: In some rare cases involving Btrfs and NFS, this can
         ;; be insufficient: <https://lwn.net/Articles/866582/>.
         (sha256 (string->utf8
-                 (string-append (string-join commits) ":"
+                 (string-append primary-key ":" system ":"
                                 (number->string (stat:dev stat)) ":"
                                 (number->string (stat:ino stat))))))))))
 
-(define (profile-cached-gc-root file)
-  "Return the cached GC root for FILE, a 'guix.scm' or 'manifest.scm' file, or
-#f if we lack information to cache it."
-  (match (profile-cache-key file)
-    (#f  #f)
-    (key (string-append (%profile-cache-directory) "/" key))))
+(define (profile-spec-cache-key specs system)
+  "Return the cache key corresponding to SPECS built for SYSTEM, where SPECS
+is a list of package specs.  Return #f if caching is not possible."
+  (match (profile-cache-primary-key)
+    (#f #f)
+    (primary-key
+     (bytevector->base32-string
+      (sha256 (string->utf8
+               (string-append primary-key ":" system ":"
+                              (object->string specs))))))))
+
+(define (profile-cached-gc-root opts)
+  "Return two values: the file name of a GC root for use as a profile cache
+for the options in OPTS, and a timestamp which, if greater than the GC root's
+mtime, indicates that the GC root is stale.  If OPTS do not permit caching,
+return #f and #f."
+  (define (key->file key)
+    (string-append (%profile-cache-directory) "/" key))
+
+  (let loop ((opts opts)
+             (system (%current-system))
+             (file #f)
+             (specs '()))
+    (match opts
+      (()
+       (if file
+           (values (and=> (profile-file-cache-key file system) key->file)
+                   (stat:mtime file))
+           (values (and=> (profile-spec-cache-key specs system) key->file)
+                   0)))
+      (((and spec ('package . _)) . rest)
+       (if (not file)
+           (loop rest system file (cons spec specs))
+           (values #f #f)))
+      ((('load . ('package candidate)) . rest)
+       (if (and (not file) (null? specs))
+           (loop rest system candidate specs)
+           (values #f #f)))
+      ((('manifest . candidate) . rest)
+       (if (and (not file) (null? specs))
+           (loop rest system candidate specs)
+           (values #f #f)))
+      ((('expression . _) . _)
+       ;; Arbitrary expressions might be non-deterministic or otherwise depend
+       ;; on external state so do not cache when they're used.
+       (values #f #f))
+      ((((? transformation-option-key?) . _) . _)
+       ;; Transformation options are potentially "non-deterministic", or at
+       ;; least depending on external state (with-source, with-commit, etc.),
+       ;; so do not cache anything when they're used.
+       (values #f #f))
+      ((('system . system) . rest)
+       (loop rest system file specs))
+      ((_ . rest) (loop rest system file specs)))))
 
 
 ;;;
diff --git a/guix/transformations.scm b/guix/transformations.scm
index c43c00cdd3..0976f0d824 100644
--- a/guix/transformations.scm
+++ b/guix/transformations.scm
@@ -1,5 +1,5 @@
 ;;; GNU Guix --- Functional package management for GNU
-;;; Copyright © 2016, 2017, 2018, 2019, 2020, 2021 Ludovic Courtès <ludo <at> gnu.org>
+;;; Copyright © 2016-2022 Ludovic Courtès <ludo <at> gnu.org>
 ;;; Copyright © 2021 Marius Bakke <marius <at> gnu.org>
 ;;;
 ;;; This file is part of GNU Guix.
@@ -56,6 +56,7 @@ (define-module (guix transformations)
             tuned-package
 
             show-transformation-options-help
+            transformation-option-key?
             %transformation-options))
 
 ;;; Commentary:
@@ -796,6 +797,12 @@ (define (transformation-procedure key)
           (and (eq? k key) proc)))
        %transformations))
 
+(define (transformation-option-key? key)
+  "Return true if KEY is an option key (as returned while parsing options with
+%TRANSFORMATION-OPTIONS) corresponding to a package transformation option.
+For example, (transformation-option-key? 'with-input) => #t."
+  (->bool (transformation-procedure key)))
+
 
 ;;;
 ;;; Command-line handling.

base-commit: 861bac1dfbeaaf40b9c11a287ef7607f0fd105a8
-- 
2.33.0

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

From: Ludovic Courtès <ludo <at> gnu.org>
To: 53034-done <at> debbugs.gnu.org
Subject: Re: bug#53034: [PATCH] shell: Cache profiles even when using
 package specs.
Date: Tue, 11 Jan 2022 20:37:52 +0100

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

> This enables profile caching not just when '-m' or '-f' is used, but
> also when package specs are passed on the command line, as in:
>
>   guix shell -D guix git
>
> It also changes profile cache keys to include the system type, which was
> previously ignored.
>
> * guix/scripts/shell.scm (options-with-caching)[single-file-for-caching]:
> Remove.
> Call 'profile-cached-gc-root' instead; adjust to accept two values.
> (profile-cache-primary-key): New procedure.
> (profile-cache-key): Remove.
> (profile-file-cache-key, profile-spec-cache-key): New procedures.
> (profile-cached-gc-root): Rewrite to include functionality formally in
> 'single-file-for-caching', but extend to handle package specs.
> * gnu/packages.scm (cache-is-authoritative?): Export.
> * guix/transformations.scm (transformation-option-key?): New procedure.

Pushed as 0552dcb294bbfed76d7a495f5e368c53f20b852a.  I adjusted
doc/guix.texi, which I had forgotten to do in the patch I posted.

Please let me how caching works for you!

Ludo’.

Previous Next

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