unofficial mirror of guix-patches@gnu.org 
 help / color / mirror / code / Atom feed
From: Christopher Baines <mail@cbaines.net>
To: 47288@debbugs.gnu.org
Subject: [bug#47288] [PATCH v2] guix: http-client: Tweak http-multiple-get error handling.
Date: Sun, 21 Mar 2021 00:56:00 +0000	[thread overview]
Message-ID: <20210321005600.12551-1-mail@cbaines.net> (raw)
In-Reply-To: <20210321004338.31867-1-mail@cbaines.net>

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..a9609445c8 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))1
+                                (memq key
+                                      '(bad-response
+                                        bad-header
+                                        bad-header-component)))
+                       (begin
+                         (close-port p)
+                         (list 'connect
+                               #f
+                               requests
+                               result))
+                       (apply throw key args))))
+             (('connect . args)
+              (apply connect args))
+             (('loop . args)
+              (apply loop args)))))))))
 
 \f
 ;;;
-- 
2.30.1





  reply	other threads:[~2021-03-21  0:57 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-03-21  0:43 [bug#47288] [PATCH] guix: http-client: Tweak http-multiple-get error handling Christopher Baines
2021-03-21  0:56 ` Christopher Baines [this message]
2021-03-24 14:55   ` Ludovic Courtès
2021-03-25 11:09     ` Christopher Baines
2021-03-24 14:55   ` Ludovic Courtès
2021-03-21  8:36 ` Maxime Devos
2021-03-25 11:03 ` [bug#47288] [PATCH v3 1/2] " Christopher Baines
2021-03-25 11:03   ` [bug#47288] [PATCH v3 2/2] guix: http-client: Refactor http-multiple-get Christopher Baines
2021-03-25 22:20   ` [bug#47288] [PATCH] guix: http-client: Tweak http-multiple-get error handling Ludovic Courtès
2021-03-26  8:39     ` Christopher Baines
2021-03-27 17:15       ` 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=20210321005600.12551-1-mail@cbaines.net \
    --to=mail@cbaines.net \
    --cc=47288@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).