GNU bug report logs - #47288
[PATCH] guix: http-client: Tweak http-multiple-get error handling.

Previous Next

Package: guix-patches;

Reported by: Christopher Baines <mail <at> cbaines.net>

Date: Sun, 21 Mar 2021 00:44:02 UTC

Severity: normal

Tags: patch

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

Bug is archived. No further changes may be made.

Full log


View this message in rfc822 format

From: Christopher Baines <mail <at> cbaines.net>
To: 47288 <at> debbugs.gnu.org
Subject: [bug#47288] [PATCH v3 1/2] guix: http-client: Tweak http-multiple-get error handling.
Date: Thu, 25 Mar 2021 11:03:15 +0000
This isn't meant to change the way errors are handled, and arguably makes the
code harder to read, but it's a uninformed attempt to improve the
performance (following on from a performance regression in
205833b72c5517915a47a50dbe28e7024dc74e57).

I'm guessing something about Guile internals makes calling (loop ...) within
the catch bit less performant than avoiding this and calling (loop ...) after
the catch bit has finished. Since this happens lots, this seems to be
sufficient to make guix weather a lot slower than it was before.

Anecdotal testing of guix weather suggests this change might work.

* guix/http-client.scm (http-multiple-get): Tweak how the second catch
statement works.
---
 guix/http-client.scm | 77 +++++++++++++++++++++++++-------------------
 1 file changed, 43 insertions(+), 34 deletions(-)

diff --git a/guix/http-client.scm b/guix/http-client.scm
index 4b4c14ed0b..adbfbc0d6e 100644
--- a/guix/http-client.scm
+++ b/guix/http-client.scm
@@ -219,42 +219,51 @@ returning."
              (remainder
               (connect p remainder result))))
           ((head tail ...)
-           (catch #t
-             (lambda ()
-               (let* ((resp   (read-response p))
-                      (body   (response-body-port resp))
-                      (result (proc head resp body result)))
-                 ;; The server can choose to stop responding at any time,
-                 ;; in which case we have to try again.  Check whether
-                 ;; that is the case.  Note that even upon "Connection:
-                 ;; close", we can read from BODY.
-                 (match (assq 'connection (response-headers resp))
-                   (('connection 'close)
-                    (close-port p)
-                    (connect #f                       ;try again
-                             (drop requests (+ 1 processed))
-                             result))
-                   (_
-                    (loop tail (+ 1 processed) result))))) ;keep going
-             (lambda (key . args)
-               ;; If PORT was cached and the server closed the connection
-               ;; in the meantime, we get EPIPE.  In that case, open a
-               ;; fresh connection and retry.  We might also get
-               ;; 'bad-response or a similar exception from (web response)
-               ;; later on, once we've sent the request, or a
-               ;; ERROR/INVALID-SESSION from GnuTLS.
-               (if (or (and (eq? key 'system-error)
-                            (= EPIPE (system-error-errno `(,key ,@args))))
-                       (and (eq? key 'gnutls-error)
-                            (eq? (first args) error/invalid-session))
-                       (memq key
-                             '(bad-response bad-header bad-header-component)))
-                   (begin
-                     (close-port p)
-                     (connect #f      ; try again
+           (match
+               (catch #t
+                 (lambda ()
+                   (let* ((resp   (read-response p))
+                          (body   (response-body-port resp))
+                          (result (proc head resp body result)))
+                     ;; The server can choose to stop responding at any time,
+                     ;; in which case we have to try again.  Check whether
+                     ;; that is the case.  Note that even upon "Connection:
+                     ;; close", we can read from BODY.
+                     (match (assq 'connection (response-headers resp))
+                       (('connection 'close)
+                        (close-port p)
+                        (list 'connect
+                              #f
                               (drop requests (+ 1 processed))
                               result))
-                   (apply throw key args))))))))))
+                       (_
+                        (list 'loop tail (+ 1 processed) result)))))
+                 (lambda (key . args)
+                   ;; If PORT was cached and the server closed the connection
+                   ;; in the meantime, we get EPIPE.  In that case, open a
+                   ;; fresh connection and retry.  We might also get
+                   ;; 'bad-response or a similar exception from (web response)
+                   ;; later on, once we've sent the request, or a
+                   ;; ERROR/INVALID-SESSION from GnuTLS.
+                   (if (or (and (eq? key 'system-error)
+                                (= EPIPE (system-error-errno `(,key ,@args))))
+                           (and (eq? key 'gnutls-error)
+                                (eq? (first args) error/invalid-session))
+                           (memq key
+                                 '(bad-response
+                                   bad-header
+                                   bad-header-component)))
+                       (begin
+                         (close-port p)
+                         (list 'connect
+                               #f
+                               (drop requests processed)
+                               result))
+                       (apply throw key args))))
+             (('connect . args)
+              (apply connect args))
+             (('loop . args)
+              (apply loop args)))))))))
 
 
 ;;;
-- 
2.30.1





This bug report was last modified 4 years and 59 days ago.

Previous Next


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