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

Previous Next

Full log

View this message in rfc822 format

From: help-debbugs <at> gnu.org (GNU bug Tracking System)
To: Ludovic Courtès <ludo <at> gnu.org>
Subject: bug#53034: closed (Re: bug#53034: [PATCH] shell: Cache profiles
 even when using package specs.)
Date: Tue, 11 Jan 2022 19:39:02 +0000

[Message part 1 (text/plain, inline)]

Your bug report

#53034: [PATCH] shell: Cache profiles even when using package specs.

which was filed against the guix-patches package, has been closed.

The explanation is attached below, along with your original report.
If you require more details, please reply to 53034 <at> debbugs.gnu.org.

-- 
53034: http://debbugs.gnu.org/cgi/bugreport.cgi?bug=53034
GNU Bug Tracking System
Contact help-debbugs <at> gnu.org with problems

[Message part 2 (message/rfc822, inline)]

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’.

[Message part 3 (message/rfc822, inline)]

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

Previous Next

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