unofficial mirror of bug-guix@gnu.org 
 help / color / mirror / code / Atom feed
From: "Ludovic Courtès" <ludo@gnu.org>
To: 48007@debbugs.gnu.org
Subject: bug#48007: [PATCH 3/4] inferior: Inferior caches store connections.
Date: Thu, 27 Jan 2022 09:47:42 +0100	[thread overview]
Message-ID: <20220127084743.27130-3-ludo@gnu.org> (raw)
In-Reply-To: <20220127084743.27130-1-ludo@gnu.org>

Fixes <https://issues.guix.gnu.org/48007>.
Reported by Ricardo Wurmus <rekado@elephly.net>.

Previously, at each 'inferior-eval-with-store' call, the inferior would
create a new <store-connection> object with empty caches.  Consequently,
when repeatedly calling 'inferior-package-derivation', we would not
benefit from any caching and instead recompute all the derivations for
every package.  This patch fixes it by caching <store-connection>
objects in the inferior.

* guix/inferior.scm (port->inferior): Define '%store-table' in the inferior.
(inferior-eval-with-store): Cache store connections in %STORE-TABLE.
Remove now unneeded 'dynamic-wind' with 'close-port' call.
---
 guix/inferior.scm | 54 +++++++++++++++++++++++++++++------------------
 1 file changed, 33 insertions(+), 21 deletions(-)

diff --git a/guix/inferior.scm b/guix/inferior.scm
index 1c19527b8f..64dd1ce9b6 100644
--- a/guix/inferior.scm
+++ b/guix/inferior.scm
@@ -225,6 +225,8 @@ (define* (port->inferior pipe #:optional (close close-port))
        (inferior-eval '(use-modules (srfi srfi-34)) result)
        (inferior-eval '(define %package-table (make-hash-table))
                       result)
+       (inferior-eval '(define %store-table (make-hash-table))
+                      result)
        result))
     (_
      #f)))
@@ -617,7 +619,12 @@ (define (inferior-eval-with-store inferior store code)
 thus be the code of a one-argument procedure that accepts a store."
   (let* ((major    (store-connection-major-version store))
          (minor    (store-connection-minor-version store))
-         (proto    (logior major minor)))
+         (proto    (logior major minor))
+
+         ;; The address of STORE itself is not a good identifier because it
+         ;; keeps changing through the use of "functional caches".  The
+         ;; address of its socket port makes more sense.
+         (store-id (object-address (store-connection-socket store))))
     (ensure-store-bridge! inferior)
     (send-inferior-request
      `(let ((proc   ,code)
@@ -628,26 +635,31 @@ (define (inferior-eval-with-store inferior store code)
                                store-protocol-error-message
                                nix-protocol-error-message)))
 
-        ;; 'port->connection' appeared in June 2018 and we can hardly
-        ;; emulate it on older versions.  Thus fall back to
-        ;; 'open-connection', at the risk of talking to the wrong daemon or
-        ;; having our build result reclaimed (XXX).
-        (let ((store (if (defined? 'port->connection)
-                         (port->connection %bridge-socket #:version ,proto)
-                         (open-connection))))
-          (dynamic-wind
-            (const #t)
-            (lambda ()
-              ;; Serialize '&store-protocol-error' conditions.  The
-              ;; exception serialization mechanism that
-              ;; 'read-repl-response' expects is unsuitable for SRFI-35
-              ;; error conditions, hence this special case.
-              (guard (c ((error? c)
-                         `(store-protocol-error ,(error-message c))))
-                `(result ,(proc store))))
-            (lambda ()
-              (unless (defined? 'port->connection)
-                (close-port store))))))
+        ;; Cache connections to STORE-ID.  This ensures that the caches within
+        ;; <store-connection> (in particular the object cache) are reused
+        ;; across calls to 'inferior-eval-with-store', which makes a
+        ;; significant different when it is called repeatedly.
+        (let ((store (or (hashv-ref %store-table ,store-id)
+
+                         ;; 'port->connection' appeared in June 2018 and we
+                         ;; can hardly emulate it on older versions.  Thus
+                         ;; fall back to 'open-connection', at the risk of
+                         ;; talking to the wrong daemon or having our build
+                         ;; result reclaimed (XXX).
+                         (let ((store (if (defined? 'port->connection)
+                                          (port->connection %bridge-socket
+                                                            #:version ,proto)
+                                          (open-connection))))
+                           (hashv-set! %store-table ,store-id store)
+                           store))))
+
+          ;; Serialize '&store-protocol-error' conditions.  The
+          ;; exception serialization mechanism that
+          ;; 'read-repl-response' expects is unsuitable for SRFI-35
+          ;; error conditions, hence this special case.
+          (guard (c ((error? c)
+                     `(store-protocol-error ,(error-message c))))
+            `(result ,(proc store)))))
      inferior)
     (proxy inferior store)
 
-- 
2.34.0





  parent reply	other threads:[~2022-01-27  8:56 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-04-24 21:07 bug#48007: computing derivations through inferior takes twice as long Ricardo Wurmus
2022-01-26 20:51 ` Ludovic Courtès
2022-01-26 21:32   ` Ricardo Wurmus
2022-01-27  8:50     ` Ludovic Courtès
2022-01-27  9:56       ` Ricardo Wurmus
2022-01-27 13:33         ` Ludovic Courtès
2022-01-27  8:47   ` bug#48007: [PATCH 1/4] inferior: Create the store proxy listening socket only once Ludovic Courtès
2022-01-27  8:47     ` bug#48007: [PATCH 2/4] inferior: Keep the store bridge connected Ludovic Courtès
2022-02-18 11:30       ` bug#48007: computing derivations through inferior takes twice as long Ludovic Courtès
2022-01-27  8:47     ` Ludovic Courtès [this message]
2022-01-27  8:47     ` bug#48007: [PATCH 4/4] inferior: Move initialization bits away from 'inferior-eval-with-store' Ludovic Courtès

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

  List information: https://guix.gnu.org/

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20220127084743.27130-3-ludo@gnu.org \
    --to=ludo@gnu.org \
    --cc=48007@debbugs.gnu.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this public inbox

	https://git.savannah.gnu.org/cgit/guix.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).