From unknown Sun Jun 22 17:15:27 2025 X-Loop: help-debbugs@gnu.org Subject: [bug#47288] [PATCH] guix: http-client: Tweak http-multiple-get error handling. Resent-From: Christopher Baines Original-Sender: "Debbugs-submit" Resent-CC: guix-patches@gnu.org Resent-Date: Sun, 21 Mar 2021 00:44:02 +0000 Resent-Message-ID: Resent-Sender: help-debbugs@gnu.org X-GNU-PR-Message: report 47288 X-GNU-PR-Package: guix-patches X-GNU-PR-Keywords: patch To: 47288@debbugs.gnu.org X-Debbugs-Original-To: guix-patches@gnu.org Received: via spool by submit@debbugs.gnu.org id=B.161628743112526 (code B ref -1); Sun, 21 Mar 2021 00:44:02 +0000 Received: (at submit) by debbugs.gnu.org; 21 Mar 2021 00:43:51 +0000 Received: from localhost ([127.0.0.1]:53355 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1lNmC6-0003Fx-IX for submit@debbugs.gnu.org; Sat, 20 Mar 2021 20:43:50 -0400 Received: from lists.gnu.org ([209.51.188.17]:58442) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1lNmC3-0003Fn-EB for submit@debbugs.gnu.org; Sat, 20 Mar 2021 20:43:48 -0400 Received: from eggs.gnu.org ([2001:470:142:3::10]:58066) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1lNmC2-0002SU-JG for guix-patches@gnu.org; Sat, 20 Mar 2021 20:43:46 -0400 Received: from mira.cbaines.net ([2a01:7e00:e000:2f8:fd4d:b5c7:13fb:3d27]:38847) by eggs.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1lNmBy-0002t7-De for guix-patches@gnu.org; Sat, 20 Mar 2021 20:43:46 -0400 Received: from localhost (unknown [IPv6:2a02:8010:68c1:0:8ac0:b4c7:f5c8:7caa]) by mira.cbaines.net (Postfix) with ESMTPSA id 0954327BC57 for ; Sun, 21 Mar 2021 00:43:40 +0000 (GMT) Received: from localhost (localhost [local]) by localhost (OpenSMTPD) with ESMTPA id 886c814a for ; Sun, 21 Mar 2021 00:43:38 +0000 (UTC) From: Christopher Baines Date: Sun, 21 Mar 2021 00:43:38 +0000 Message-Id: <20210321004338.31867-1-mail@cbaines.net> X-Mailer: git-send-email 2.30.1 MIME-Version: 1.0 Content-Transfer-Encoding: 8bit Received-SPF: pass client-ip=2a01:7e00:e000:2f8:fd4d:b5c7:13fb:3d27; envelope-from=mail@cbaines.net; helo=mira.cbaines.net X-Spam_score_int: -18 X-Spam_score: -1.9 X-Spam_bar: - X-Spam_report: (-1.9 / 5.0 requ) BAYES_00=-1.9, SPF_HELO_PASS=-0.001, SPF_PASS=-0.001, UNPARSEABLE_RELAY=0.001 autolearn=ham autolearn_force=no X-Spam_action: no action X-Spam-Score: -1.4 (-) X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.18 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: debbugs-submit-bounces@debbugs.gnu.org Sender: "Debbugs-submit" X-Spam-Score: -2.4 (--) 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, 41 insertions(+), 36 deletions(-) diff --git a/guix/http-client.scm b/guix/http-client.scm index 4b4c14ed0b..a189cceecf 100644 --- a/guix/http-client.scm +++ b/guix/http-client.scm @@ -219,42 +219,47 @@ 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 - (drop requests (+ 1 processed)) - result)) - (apply throw key args)))))))))) + (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) + 'try-again-with-new-connection) + (_ + 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) + 'try-again-with-new-connection) + (apply throw key args)))) + ('try-again-with-new-connection + (connect #f + (drop requests (+ 1 processed)) + result)) + (result + (loop tail (+ 1 processed) result))))))))) ;;; -- 2.30.1 From unknown Sun Jun 22 17:15:27 2025 X-Loop: help-debbugs@gnu.org Subject: [bug#47288] [PATCH v2] guix: http-client: Tweak http-multiple-get error handling. References: <20210321004338.31867-1-mail@cbaines.net> In-Reply-To: <20210321004338.31867-1-mail@cbaines.net> Resent-From: Christopher Baines Original-Sender: "Debbugs-submit" Resent-CC: guix-patches@gnu.org Resent-Date: Sun, 21 Mar 2021 00:57:02 +0000 Resent-Message-ID: Resent-Sender: help-debbugs@gnu.org X-GNU-PR-Message: followup 47288 X-GNU-PR-Package: guix-patches X-GNU-PR-Keywords: patch To: 47288@debbugs.gnu.org Received: via spool by 47288-submit@debbugs.gnu.org id=B47288.161628816813656 (code B ref 47288); Sun, 21 Mar 2021 00:57:02 +0000 Received: (at 47288) by debbugs.gnu.org; 21 Mar 2021 00:56:08 +0000 Received: from localhost ([127.0.0.1]:53364 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1lNmNz-0003YB-2P for submit@debbugs.gnu.org; Sat, 20 Mar 2021 20:56:07 -0400 Received: from mira.cbaines.net ([212.71.252.8]:59698) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1lNmNt-0003Xi-70 for 47288@debbugs.gnu.org; Sat, 20 Mar 2021 20:56:05 -0400 Received: from localhost (unknown [IPv6:2a02:8010:68c1:0:8ac0:b4c7:f5c8:7caa]) by mira.cbaines.net (Postfix) with ESMTPSA id 5131E27BC57 for <47288@debbugs.gnu.org>; Sun, 21 Mar 2021 00:56:00 +0000 (GMT) Received: from localhost (localhost [local]) by localhost (OpenSMTPD) with ESMTPA id 87a2253f for <47288@debbugs.gnu.org>; Sun, 21 Mar 2021 00:56:00 +0000 (UTC) From: Christopher Baines Date: Sun, 21 Mar 2021 00:56:00 +0000 Message-Id: <20210321005600.12551-1-mail@cbaines.net> X-Mailer: git-send-email 2.30.1 MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Spam-Score: -0.0 (/) X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.18 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: debbugs-submit-bounces@debbugs.gnu.org Sender: "Debbugs-submit" X-Spam-Score: -1.0 (-) 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))))))))) ;;; -- 2.30.1 From unknown Sun Jun 22 17:15:27 2025 X-Loop: help-debbugs@gnu.org Subject: [bug#47288] [PATCH] guix: http-client: Tweak http-multiple-get error handling. Resent-From: Maxime Devos Original-Sender: "Debbugs-submit" Resent-CC: guix-patches@gnu.org Resent-Date: Sun, 21 Mar 2021 08:37:01 +0000 Resent-Message-ID: Resent-Sender: help-debbugs@gnu.org X-GNU-PR-Message: followup 47288 X-GNU-PR-Package: guix-patches X-GNU-PR-Keywords: patch To: Christopher Baines , 47288@debbugs.gnu.org Received: via spool by 47288-submit@debbugs.gnu.org id=B47288.161631582032170 (code B ref 47288); Sun, 21 Mar 2021 08:37:01 +0000 Received: (at 47288) by debbugs.gnu.org; 21 Mar 2021 08:37:00 +0000 Received: from localhost ([127.0.0.1]:53518 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1lNta0-0008Mm-DD for submit@debbugs.gnu.org; Sun, 21 Mar 2021 04:37:00 -0400 Received: from xavier.telenet-ops.be ([195.130.132.52]:44156) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1lNtZx-0008Mb-Jy for 47288@debbugs.gnu.org; Sun, 21 Mar 2021 04:36:59 -0400 Received: from ptr-bvsjgyjmffd7q9timvx.18120a2.ip6.access.telenet.be ([IPv6:2a02:1811:8c09:9d00:aaf1:9810:a0b8:a55d]) by xavier.telenet-ops.be with bizsmtp id iwcv2400A0mfAB401wcvU0; Sun, 21 Mar 2021 09:36:55 +0100 Message-ID: <70ae9918cfe06c3dcce6764f170a96b55e275d4d.camel@telenet.be> From: Maxime Devos Date: Sun, 21 Mar 2021 09:36:42 +0100 In-Reply-To: <20210321004338.31867-1-mail@cbaines.net> References: <20210321004338.31867-1-mail@cbaines.net> Content-Type: multipart/signed; micalg="pgp-sha256"; protocol="application/pgp-signature"; boundary="=-93KPs4P3me86xyvi8Jnh" User-Agent: Evolution 3.34.2 MIME-Version: 1.0 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=telenet.be; s=r21; t=1616315815; bh=wDJIwqPz5lBhxdv1/NryrzlDvMaBiyQpWTmyOCsPezE=; h=Subject:From:To:Date:In-Reply-To:References; b=VAO+PUzg7PavK78QEJZmQFf6PuvIdxJlngiu2z/BI3sA+TI84R3Qwjf+4vmw7p4/0 e3K/zuAjo3ayagHoVwewS6WXLyVqA9jkreA2drrB9J7negv1qbJgPNf0xacSAFCo3t +kMsNWN6BZdu3KbEEYgJhHWvyZbQSwJfQXgH8gjB9fQJ+8ahvQ3BBG4Vo4mRpLzMyl hoKRSZCiv+iss22OOT/u4HW8l8E+LifT8gUMywzVjqCuGs5gCS73RhlkSJOuq24JjA MbzOY9ZYwdFJWPPDwc1pSrn1Ls1m8zI/CycnFL5Ve/HGZYIZOeO0MgvM2ByuO6g3wn 3xANGBLHa1WWQ== X-Spam-Score: -0.7 (/) X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.18 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: debbugs-submit-bounces@debbugs.gnu.org Sender: "Debbugs-submit" X-Spam-Score: -1.7 (-) --=-93KPs4P3me86xyvi8Jnh Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable On Sun, 2021-03-21 at 00:43 +0000, Christopher Baines wrote: > 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). >=20 > I'm guessing something about Guile internals makes calling (loop ...) wit= hin > the catch bit less performant than avoiding this and calling (loop ...) a= fter > 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. I took a look at the code, and it seems we did somthing like: (let loop VARS (match sent NON-LOOPING-CASES (STUFF (catch #t THUNK-THAT-MIGHT-CALL-LOOP-IN-TAIL-POSITION SOME-HANDLER-THAT-DOES-NOT-CALL-LOOP) A small demonstration of what could go wrong with such a construction (run this in the Guile REPL): (let loop ((attempts-todo 20)) (catch 'oops (lambda () (if (<=3D 0 attempts-todo) (loop (- attempts-todo 1)) (throw 'oops))) (lambda _ (display 'too-bad!) (newline) (backtrace)))) Output: too-bad! Backtrace: In ice-9/boot-9.scm: 1731:15 19 (with-exception-handler # _ # _ =E2=80=A6) [The previous line repeated 17 times] 1731:15 1 (with-exception-handler # _ # _ =E2=80=A6) In unknown file: 0 (backtrace #) With this construction, we were nesting exception handlers within exception= handlers ... So in hindsight it doesn't seem surprising this isn't very performant. (THUNK-THAT-MIGHT-CALL-LOOP-IN-TAIL-POSITION itself is not called in the ta= il-position of the '(let loop ...)' form.) Hope that sheds some light on the matter, Maxime --=-93KPs4P3me86xyvi8Jnh Content-Type: application/pgp-signature; name="signature.asc" Content-Description: This is a digitally signed message part Content-Transfer-Encoding: 7bit -----BEGIN PGP SIGNATURE----- iI0EABYIADUWIQTB8z7iDFKP233XAR9J4+4iGRcl7gUCYFcFmhccbWF4aW1lZGV2 b3NAdGVsZW5ldC5iZQAKCRBJ4+4iGRcl7uELAPwLbkh01qJqniid0xELzUrLgGhz Jqe9NbVxRRcY42SmygEAgz/0DZPmVAoBbxJVUXxGl/J+FVtGrSEmH2atCjL5gQA= =8t1s -----END PGP SIGNATURE----- --=-93KPs4P3me86xyvi8Jnh-- From unknown Sun Jun 22 17:15:27 2025 X-Loop: help-debbugs@gnu.org Subject: [bug#47288] [PATCH] guix: http-client: Tweak http-multiple-get error handling. Resent-From: Ludovic =?UTF-8?Q?Court=C3=A8s?= Original-Sender: "Debbugs-submit" Resent-CC: guix-patches@gnu.org Resent-Date: Wed, 24 Mar 2021 14:56:02 +0000 Resent-Message-ID: Resent-Sender: help-debbugs@gnu.org X-GNU-PR-Message: followup 47288 X-GNU-PR-Package: guix-patches X-GNU-PR-Keywords: patch To: Christopher Baines Cc: 47288@debbugs.gnu.org Received: via spool by 47288-submit@debbugs.gnu.org id=B47288.16165977586239 (code B ref 47288); Wed, 24 Mar 2021 14:56:02 +0000 Received: (at 47288) by debbugs.gnu.org; 24 Mar 2021 14:55:58 +0000 Received: from localhost ([127.0.0.1]:35786 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1lP4vN-0001cZ-M1 for submit@debbugs.gnu.org; Wed, 24 Mar 2021 10:55:57 -0400 Received: from eggs.gnu.org ([209.51.188.92]:37012) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1lP4vM-0001cN-50 for 47288@debbugs.gnu.org; Wed, 24 Mar 2021 10:55:56 -0400 Received: from fencepost.gnu.org ([2001:470:142:3::e]:57201) by eggs.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1lP4vG-0004Wu-Az; Wed, 24 Mar 2021 10:55:50 -0400 Received: from [2a01:e0a:1d:7270:af76:b9b:ca24:c465] (port=57328 helo=ribbon) by fencepost.gnu.org with esmtpsa (TLS1.2:RSA_AES_256_CBC_SHA1:256) (Exim 4.82) (envelope-from ) id 1lP4v6-0002Ho-KK; Wed, 24 Mar 2021 10:55:46 -0400 From: Ludovic =?UTF-8?Q?Court=C3=A8s?= References: <20210321004338.31867-1-mail@cbaines.net> <20210321005600.12551-1-mail@cbaines.net> Date: Wed, 24 Mar 2021 15:55:38 +0100 In-Reply-To: <20210321005600.12551-1-mail@cbaines.net> (Christopher Baines's message of "Sun, 21 Mar 2021 00:56:00 +0000") Message-ID: <87pmzoeh3p.fsf_-_@gnu.org> User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/27.1 (gnu/linux) MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable X-Spam-Score: -0.7 (/) X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.18 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: debbugs-submit-bounces@debbugs.gnu.org Sender: "Debbugs-submit" X-Spam-Score: -1.7 (-) As discussed at , I=E2=80=99m still unsu= re these exceptions need to be caught within =E2=80=98http-multiple-get=E2=80= =99 and at each iteration. Just minor comments: Christopher Baines skribis: > + (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 wheth= er > + ;; that is the case. Note that even upon "Connecti= on: > + ;; 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 conne= ction > + ;; 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 res= ponse) > + ;; later on, once we've sent the request, or a > + ;; ERROR/INVALID-SESSION from GnuTLS. > + (if (or (and (eq? key 'system-error) > + (=3D 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))))))))) This is not new to this patch, but I think the whole exception handling bit should be factorized and written in such a way that =E2=80=98http-multiple-get=E2=80=99 still first on a horizontal screen (eve= n though mine is actually vertical :-)). Otherwise one might easily overlook the core logic of the function. Ludo=E2=80=99. From unknown Sun Jun 22 17:15:27 2025 X-Loop: help-debbugs@gnu.org Subject: [bug#47288] [PATCH] guix: http-client: Tweak http-multiple-get error handling. Resent-From: Ludovic =?UTF-8?Q?Court=C3=A8s?= Original-Sender: "Debbugs-submit" Resent-CC: guix-patches@gnu.org Resent-Date: Wed, 24 Mar 2021 14:57:01 +0000 Resent-Message-ID: Resent-Sender: help-debbugs@gnu.org X-GNU-PR-Message: followup 47288 X-GNU-PR-Package: guix-patches X-GNU-PR-Keywords: patch To: Christopher Baines Cc: 47288@debbugs.gnu.org Received: via spool by 47288-submit@debbugs.gnu.org id=B47288.16165977656285 (code B ref 47288); Wed, 24 Mar 2021 14:57:01 +0000 Received: (at 47288) by debbugs.gnu.org; 24 Mar 2021 14:56:05 +0000 Received: from localhost ([127.0.0.1]:35790 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1lP4vU-0001dH-Vs for submit@debbugs.gnu.org; Wed, 24 Mar 2021 10:56:05 -0400 Received: from eggs.gnu.org ([209.51.188.92]:37060) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1lP4vT-0001cg-7A for 47288@debbugs.gnu.org; Wed, 24 Mar 2021 10:56:03 -0400 Received: from fencepost.gnu.org ([2001:470:142:3::e]:57203) by eggs.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1lP4vM-0004ax-FK; Wed, 24 Mar 2021 10:55:58 -0400 Received: from [2a01:e0a:1d:7270:af76:b9b:ca24:c465] (port=57330 helo=ribbon) by fencepost.gnu.org with esmtpsa (TLS1.2:RSA_AES_256_CBC_SHA1:256) (Exim 4.82) (envelope-from ) id 1lP4vJ-0002NE-3b; Wed, 24 Mar 2021 10:55:54 -0400 From: Ludovic =?UTF-8?Q?Court=C3=A8s?= In-Reply-To: <20210321005600.12551-1-mail@cbaines.net> (Christopher Baines's message of "Sun, 21 Mar 2021 00:56:00 +0000") References: <20210321004338.31867-1-mail@cbaines.net> <20210321005600.12551-1-mail@cbaines.net> User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/27.1 (gnu/linux) Date: Wed, 24 Mar 2021 15:55:51 +0100 Message-ID: <87o8f8eh3c.fsf_-_@gnu.org> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable X-Spam-Score: -0.7 (/) X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.18 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: debbugs-submit-bounces@debbugs.gnu.org Sender: "Debbugs-submit" X-Spam-Score: -1.7 (-) As discussed at , I=E2=80=99m still unsu= re these exceptions need to be caught within =E2=80=98http-multiple-get=E2=80= =99 and at each iteration. Just minor comments: Christopher Baines skribis: > + (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 wheth= er > + ;; that is the case. Note that even upon "Connecti= on: > + ;; 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 conne= ction > + ;; 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 res= ponse) > + ;; later on, once we've sent the request, or a > + ;; ERROR/INVALID-SESSION from GnuTLS. > + (if (or (and (eq? key 'system-error) > + (=3D 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))))))))) This is not new to this patch, but I think the whole exception handling bit should be factorized and written in such a way that =E2=80=98http-multiple-get=E2=80=99 still fits on a horizontal screen (even= though mine is actually vertical :-)). Otherwise one might easily overlook the core logic of the function. Ludo=E2=80=99. From unknown Sun Jun 22 17:15:27 2025 X-Loop: help-debbugs@gnu.org Subject: [bug#47288] [PATCH v3 1/2] guix: http-client: Tweak http-multiple-get error handling. References: <20210321004338.31867-1-mail@cbaines.net> In-Reply-To: <20210321004338.31867-1-mail@cbaines.net> Resent-From: Christopher Baines Original-Sender: "Debbugs-submit" Resent-CC: guix-patches@gnu.org Resent-Date: Thu, 25 Mar 2021 11:04:02 +0000 Resent-Message-ID: Resent-Sender: help-debbugs@gnu.org X-GNU-PR-Message: followup 47288 X-GNU-PR-Package: guix-patches X-GNU-PR-Keywords: patch To: 47288@debbugs.gnu.org Received: via spool by 47288-submit@debbugs.gnu.org id=B47288.16166701995738 (code B ref 47288); Thu, 25 Mar 2021 11:04:02 +0000 Received: (at 47288) by debbugs.gnu.org; 25 Mar 2021 11:03:19 +0000 Received: from localhost ([127.0.0.1]:37326 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1lPNln-0001UT-7o for submit@debbugs.gnu.org; Thu, 25 Mar 2021 07:03:19 -0400 Received: from mira.cbaines.net ([212.71.252.8]:50966) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1lPNll-0001UF-Ox for 47288@debbugs.gnu.org; Thu, 25 Mar 2021 07:03:18 -0400 Received: from localhost (unknown [IPv6:2a02:8010:68c1:0:8ac0:b4c7:f5c8:7caa]) by mira.cbaines.net (Postfix) with ESMTPSA id CA5CD27BC5C for <47288@debbugs.gnu.org>; Thu, 25 Mar 2021 11:03:16 +0000 (GMT) Received: from localhost (localhost [local]) by localhost (OpenSMTPD) with ESMTPA id 8a272564 for <47288@debbugs.gnu.org>; Thu, 25 Mar 2021 11:03:16 +0000 (UTC) From: Christopher Baines Date: Thu, 25 Mar 2021 11:03:15 +0000 Message-Id: <20210325110316.862-1-mail@cbaines.net> X-Mailer: git-send-email 2.30.1 MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Spam-Score: -0.0 (/) X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.18 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: debbugs-submit-bounces@debbugs.gnu.org Sender: "Debbugs-submit" X-Spam-Score: -1.0 (-) 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 From unknown Sun Jun 22 17:15:27 2025 X-Loop: help-debbugs@gnu.org Subject: [bug#47288] [PATCH v3 2/2] guix: http-client: Refactor http-multiple-get. Resent-From: Christopher Baines Original-Sender: "Debbugs-submit" Resent-CC: guix-patches@gnu.org Resent-Date: Thu, 25 Mar 2021 11:04:02 +0000 Resent-Message-ID: Resent-Sender: help-debbugs@gnu.org X-GNU-PR-Message: followup 47288 X-GNU-PR-Package: guix-patches X-GNU-PR-Keywords: patch To: 47288@debbugs.gnu.org Received: via spool by 47288-submit@debbugs.gnu.org id=B47288.16166702005745 (code B ref 47288); Thu, 25 Mar 2021 11:04:02 +0000 Received: (at 47288) by debbugs.gnu.org; 25 Mar 2021 11:03:20 +0000 Received: from localhost ([127.0.0.1]:37328 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1lPNln-0001UV-J9 for submit@debbugs.gnu.org; Thu, 25 Mar 2021 07:03:20 -0400 Received: from mira.cbaines.net ([212.71.252.8]:50968) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1lPNll-0001UG-RJ for 47288@debbugs.gnu.org; Thu, 25 Mar 2021 07:03:18 -0400 Received: from localhost (unknown [IPv6:2a02:8010:68c1:0:8ac0:b4c7:f5c8:7caa]) by mira.cbaines.net (Postfix) with ESMTPSA id D2A8F27BC5D for <47288@debbugs.gnu.org>; Thu, 25 Mar 2021 11:03:16 +0000 (GMT) Received: from localhost (localhost [local]) by localhost (OpenSMTPD) with ESMTPA id 551b1553 for <47288@debbugs.gnu.org>; Thu, 25 Mar 2021 11:03:16 +0000 (UTC) From: Christopher Baines Date: Thu, 25 Mar 2021 11:03:16 +0000 Message-Id: <20210325110316.862-2-mail@cbaines.net> X-Mailer: git-send-email 2.30.1 In-Reply-To: <20210325110316.862-1-mail@cbaines.net> References: <20210325110316.862-1-mail@cbaines.net> MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Spam-Score: -0.0 (/) X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.18 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: debbugs-submit-bounces@debbugs.gnu.org Sender: "Debbugs-submit" X-Spam-Score: -1.0 (-) Split the procedure in to three smaller procedures, rather than using two longer let statements. This might make it easier to read. * guix/http-client.scm (http-multiple-get): Refactor. --- guix/http-client.scm | 195 ++++++++++++++++++++++--------------------- 1 file changed, 101 insertions(+), 94 deletions(-) diff --git a/guix/http-client.scm b/guix/http-client.scm index adbfbc0d6e..b584feba5d 100644 --- a/guix/http-client.scm +++ b/guix/http-client.scm @@ -147,7 +147,7 @@ Raise an '&http-get-error' condition if downloading fails." (uri->string uri) code (response-reason-phrase resp)))))))))))) -(define* (http-multiple-get base-uri proc seed requests +(define* (http-multiple-get base-uri proc seed all-requests #:key port (verify-certificate? #t) (open-connection guix:open-connection-for-uri) (keep-alive? #t) @@ -161,16 +161,90 @@ When PORT is specified, use it as the initial connection on which HTTP requests are sent; otherwise call OPEN-CONNECTION to open a new connection for a URI. When KEEP-ALIVE? is false, close the connection port before returning." - (let connect ((port port) - (requests requests) - (result seed)) + (define (send-batch-of-requests p batch) + ;; Send BATCH in a row. + ;; XXX: Do our own caching to work around inefficiencies when + ;; communicating over TLS: . + (let-values (((buffer get) (open-bytevector-output-port))) + ;; Inherit the HTTP proxying property from P. + (set-http-proxy-port?! buffer (http-proxy-port? p)) + + (for-each (cut write-request <> buffer) + batch) + (put-bytevector p (get)) + (force-output p))) + + (define (process-batch-of-responses p + all-remaining-requests + batch-remaining-requests + processed + result) + (if (null? batch-remaining-requests) + (match (drop all-remaining-requests processed) + (() + (unless keep-alive? + (close-port p)) + (reverse result)) + (remainder + (connect-and-make-requests p remainder result))) + (match + (catch #t + (lambda () + (let* ((request (car batch-remaining-requests)) + (resp (read-response p)) + (body (response-body-port resp)) + (result (proc request 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-and-make-requests + #f + (drop all-remaining-requests (+ 1 processed)) + result)) + (_ + (list 'process-batch-of-responses + p + all-remaining-requests + (cdr batch-remaining-requests) + (+ 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-and-make-requests + #f + (drop all-remaining-requests processed) + result)) + (apply throw key args)))) + + (('connect-and-make-requests . args) + (apply connect-and-make-requests args)) + (('process-batch-of-responses . args) + (apply process-batch-of-responses args))))) + + (define (connect-and-make-requests port remaining-requests result) (define batch - (if (>= batch-size (length requests)) - requests - (take requests batch-size))) + (if (>= batch-size (length remaining-requests)) + remaining-requests + (take remaining-requests batch-size))) - ;; (format (current-error-port) "connecting (~a requests left)..." - ;; (length requests)) (let ((p (or port (open-connection base-uri #:verify-certificate? verify-certificate?)))) @@ -178,92 +252,25 @@ returning." (when (file-port? p) (setvbuf p 'block (expt 2 16))) - ;; Send BATCH in a row. - ;; XXX: Do our own caching to work around inefficiencies when - ;; communicating over TLS: . - (let-values (((buffer get) (open-bytevector-output-port))) - ;; Inherit the HTTP proxying property from P. - (set-http-proxy-port?! buffer (http-proxy-port? p)) - - (catch #t - (lambda () - (for-each (cut write-request <> buffer) - batch) - (put-bytevector p (get)) - (force-output p)) - (lambda (key . args) - ;; If PORT becomes unusable, open a fresh connection and - ;; retry. - (if (or (and (eq? key 'system-error) - (= EPIPE (system-error-errno `(,key ,@args)))) - (and (eq? key 'gnutls-error) - (eq? (first args) error/invalid-session))) - (begin - (close-port p) ; close the broken port - (connect #f - requests - result)) - (apply throw key args))))) + (catch #t + (lambda () + (send-batch-of-requests p batch)) + (lambda (key . args) + ;; If PORT becomes unusable, open a fresh connection and retry. + (if (or (and (eq? key 'system-error) + (= EPIPE (system-error-errno `(,key ,@args)))) + (and (eq? key 'gnutls-error) + (eq? (first args) error/invalid-session))) + (begin + (close-port p) ; close the broken port + (connect-and-make-requests #f + remaining-requests + result)) + (apply throw key args)))) - ;; Now start processing responses. - (let loop ((sent batch) - (processed 0) - (result result)) - (match sent - (() - (match (drop requests processed) - (() - (unless keep-alive? - (close-port p)) - (reverse result)) - (remainder - (connect p remainder result)))) - ((head tail ...) - (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)) - (_ - (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))))))))) + (process-batch-of-responses p remaining-requests batch 0 result))) + + (connect-and-make-requests port all-requests seed)) ;;; -- 2.30.1 From unknown Sun Jun 22 17:15:27 2025 X-Loop: help-debbugs@gnu.org Subject: [bug#47288] [PATCH] guix: http-client: Tweak http-multiple-get error handling. Resent-From: Christopher Baines Original-Sender: "Debbugs-submit" Resent-CC: guix-patches@gnu.org Resent-Date: Thu, 25 Mar 2021 11:10:01 +0000 Resent-Message-ID: Resent-Sender: help-debbugs@gnu.org X-GNU-PR-Message: followup 47288 X-GNU-PR-Package: guix-patches X-GNU-PR-Keywords: patch To: Ludovic =?UTF-8?Q?Court=C3=A8s?= Cc: 47288@debbugs.gnu.org Received: via spool by 47288-submit@debbugs.gnu.org id=B47288.16166705506376 (code B ref 47288); Thu, 25 Mar 2021 11:10:01 +0000 Received: (at 47288) by debbugs.gnu.org; 25 Mar 2021 11:09:10 +0000 Received: from localhost ([127.0.0.1]:37345 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1lPNrR-0001el-WE for submit@debbugs.gnu.org; Thu, 25 Mar 2021 07:09:10 -0400 Received: from mira.cbaines.net ([212.71.252.8]:50984) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1lPNrP-0001ed-Vh for 47288@debbugs.gnu.org; Thu, 25 Mar 2021 07:09:08 -0400 Received: from localhost (unknown [IPv6:2a02:8010:68c1:0:8ac0:b4c7:f5c8:7caa]) by mira.cbaines.net (Postfix) with ESMTPSA id 2854127BC5C; Thu, 25 Mar 2021 11:09:07 +0000 (GMT) Received: from capella (localhost [127.0.0.1]) by localhost (OpenSMTPD) with ESMTP id 445f76e4; Thu, 25 Mar 2021 11:09:06 +0000 (UTC) References: <20210321004338.31867-1-mail@cbaines.net> <20210321005600.12551-1-mail@cbaines.net> <87pmzoeh3p.fsf_-_@gnu.org> User-agent: mu4e 1.4.15; emacs 27.1 From: Christopher Baines In-reply-to: <87pmzoeh3p.fsf_-_@gnu.org> Message-ID: <87zgyra3sg.fsf@cbaines.net> Date: Thu, 25 Mar 2021 11:09:04 +0000 MIME-Version: 1.0 Content-Type: multipart/signed; boundary="==-=-="; micalg=pgp-sha512; protocol="application/pgp-signature" X-Spam-Score: -0.0 (/) X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.18 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: debbugs-submit-bounces@debbugs.gnu.org Sender: "Debbugs-submit" X-Spam-Score: -1.0 (-) --==-=-= Content-Type: multipart/mixed; boundary="=-=-=" --=-=-= Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable Ludovic Court=C3=A8s writes: > As discussed at , I=E2=80=99m still un= sure > these exceptions need to be caught within =E2=80=98http-multiple-get=E2= =80=99 and at > each iteration. > > Just minor comments: > > Christopher Baines skribis: > >> + (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 whet= her >> + ;; that is the case. Note that even upon "Connect= ion: >> + ;; 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 conn= ection >> + ;; in the meantime, we get EPIPE. In that case, ope= n a >> + ;; fresh connection and retry. We might also get >> + ;; 'bad-response or a similar exception from (web re= sponse) >> + ;; later on, once we've sent the request, or a >> + ;; ERROR/INVALID-SESSION from GnuTLS. >> + (if (or (and (eq? key 'system-error) >> + (=3D 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))))))))) > > This is not new to this patch, but I think the whole exception handling > bit should be factorized and written in such a way that > =E2=80=98http-multiple-get=E2=80=99 still first on a horizontal screen (e= ven though mine > is actually vertical :-)). Otherwise one might easily overlook the core > logic of the function. I've sent a v3 now, which makes a few changes to the original patch, and includes a second patch for a potential refactoring. I tested three variants for performance, http-multiple-get with no error handling, the first v3 patch and the first and second v3 patches, and at least with the test I'm using, the performance seems similar. Assuming the performance is lower with error handling, the impact seems to be within the margin of error, at least for test I was using. --=-=-= Content-Type: text/plain Content-Disposition: attachment; filename=http-multiple-get-perf.scm (use-modules (ice-9 binary-ports) (srfi srfi-19) (web uri) (web request) (web response) (guix http-client)) (define (call-with-time-logging requests thunk) (let ((start (current-time time-utc))) (call-with-values thunk (lambda vals (let* ((end (current-time time-utc)) (elapsed (time-difference end start))) (display (format #f "~f seconds (~f microseconds per request)~%" (+ (time-second elapsed) (/ (time-nanosecond elapsed) 1e9)) (* (/ (+ (time-second elapsed) (/ (time-nanosecond elapsed) 1e9)) requests) 1e6))) (apply values vals)))))) (define requests (map (lambda _ (build-request (string->uri "http://localhost/does-not-exist") #:method 'GET)) (iota 200000))) (call-with-time-logging (length requests) (lambda () (http-multiple-get (string->uri "http://localhost/") (lambda (request response port result) (get-bytevector-n port (response-content-length response)) '()) '() requests))) --=-=-=-- --==-=-= Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- iQKlBAEBCgCPFiEEPonu50WOcg2XVOCyXiijOwuE9XcFAmBcb1BfFIAAAAAALgAo aXNzdWVyLWZwckBub3RhdGlvbnMub3BlbnBncC5maWZ0aGhvcnNlbWFuLm5ldDNF ODlFRUU3NDU4RTcyMEQ5NzU0RTBCMjVFMjhBMzNCMEI4NEY1NzcRHG1haWxAY2Jh aW5lcy5uZXQACgkQXiijOwuE9XcMYg//ZHjTZpEIBy3zvkUytf7DqxlMj6/e/fkR G5B8VLkRysYDJUckziRqN/bBykRRrjYR7mCt4qX1XHROZdUAkxZ++K+xkrtUxSPq k9ezSw+akd4+geCpfyw0/R8aiqHPMUVlrvuSheUEsb2jHRD/oikkoS9N7xRrRsQN 3O4+alydMmKd24SKBjAHY375JE+Q0ad69Rry8mXw8rcXPuX4wGGsMfwamI2n38pZ Lghc9sgH82RXP4aZmdGyS2jWt6m/Qmydu0Par159cG9Ru0CCbio0HoMZd4Vpnvy7 VcMzp8UM2pAffD3CfzpOgKgIKKkb4wWJ25EgPD2+sDbCNBInz3XbUX3qCKHhngmi 6SDFzBFe59G5D/VnLGcOACUBg6a2IhFBjKuGcvWSELUDRCuQp2Z91KcdDb5kxPl1 Aznk2IwetBRUjXJUs8MhKsoit7E7KYFihN9zXsvXRlbnRcsyGoatTV48BXZHQOpv pXFngQYroYnp4/UIFgTdx4Dm6Oxs+VWdSZgM6/bSUmVHQmTxw++83Ia+L8zRTgp6 TCtycdC7tcJxlA7fQDfp99VPki5p3QGDZkShDIm3TqHwvlf1VG504HnNTRcr7yY+ 2feCfA3kcA5EJNYl5iDlhHT4dWM4LxGVUUCO51vWxVLKbsxCeRlTfl+GM8ePRzzS 1ErFEpvTEKI= =Wa1I -----END PGP SIGNATURE----- --==-=-=-- From unknown Sun Jun 22 17:15:27 2025 X-Loop: help-debbugs@gnu.org Subject: [bug#47288] [PATCH] guix: http-client: Tweak http-multiple-get error handling. Resent-From: Ludovic =?UTF-8?Q?Court=C3=A8s?= Original-Sender: "Debbugs-submit" Resent-CC: guix-patches@gnu.org Resent-Date: Thu, 25 Mar 2021 22:22:01 +0000 Resent-Message-ID: Resent-Sender: help-debbugs@gnu.org X-GNU-PR-Message: followup 47288 X-GNU-PR-Package: guix-patches X-GNU-PR-Keywords: patch To: Christopher Baines Cc: 47288@debbugs.gnu.org Received: via spool by 47288-submit@debbugs.gnu.org id=B47288.161671086917843 (code B ref 47288); Thu, 25 Mar 2021 22:22:01 +0000 Received: (at 47288) by debbugs.gnu.org; 25 Mar 2021 22:21:09 +0000 Received: from localhost ([127.0.0.1]:39781 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1lPYLl-0004di-2C for submit@debbugs.gnu.org; Thu, 25 Mar 2021 18:21:09 -0400 Received: from eggs.gnu.org ([209.51.188.92]:36868) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1lPYLh-0004cv-Uw for 47288@debbugs.gnu.org; Thu, 25 Mar 2021 18:21:08 -0400 Received: from fencepost.gnu.org ([2001:470:142:3::e]:33252) by eggs.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1lPYLc-0003B1-PS; Thu, 25 Mar 2021 18:21:00 -0400 Received: from [2a01:e0a:1d:7270:af76:b9b:ca24:c465] (port=34964 helo=ribbon) by fencepost.gnu.org with esmtpsa (TLS1.2:RSA_AES_256_CBC_SHA1:256) (Exim 4.82) (envelope-from ) id 1lPYLc-0000X1-85; Thu, 25 Mar 2021 18:21:00 -0400 From: Ludovic =?UTF-8?Q?Court=C3=A8s?= References: <20210321004338.31867-1-mail@cbaines.net> <20210325110316.862-1-mail@cbaines.net> Date: Thu, 25 Mar 2021 23:20:57 +0100 In-Reply-To: <20210325110316.862-1-mail@cbaines.net> (Christopher Baines's message of "Thu, 25 Mar 2021 11:03:15 +0000") Message-ID: <87r1k250za.fsf_-_@gnu.org> User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/27.1 (gnu/linux) MIME-Version: 1.0 Content-Type: multipart/mixed; boundary="=-=-=" X-Spam-Score: -0.7 (/) X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.18 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: debbugs-submit-bounces@debbugs.gnu.org Sender: "Debbugs-submit" X-Spam-Score: -1.7 (-) --=-=-= Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable Hi! Christopher Baines skribis: > 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 ...) wit= hin > the catch bit less performant than avoiding this and calling (loop ...) a= fter > 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. As Maxime wrote, the problem is that we were making non-tail calls, thereby consuming stack space as well as accumulating exception handlers. As discussed earlier, =E2=80=98raise-exception=E2=80=99 exhibits= quadratic behavior in the number of exception handlers, which is okay in normal situations, but not so much when there are thousands of handlers, as is the case when asking for many substitutes. > Anecdotal testing of guix weather suggests this change might work. Don=E2=80=99t leave this last sentence in the actual commit. :-) Please mention . > * 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 ...) [...] > + (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 wheth= er > + ;; that is the case. Note that even upon "Connecti= on: > + ;; 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 conne= ction > + ;; 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 res= ponse) > + ;; later on, once we've sent the request, or a > + ;; ERROR/INVALID-SESSION from GnuTLS. > + (if (or (and (eq? key 'system-error) > + (=3D 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))))))))) OK to write it this way as the first commit, to ease review. What about the approach below: --=-=-= Content-Type: text/x-patch; charset=utf-8 Content-Disposition: inline Content-Transfer-Encoding: quoted-printable diff --git a/guix/http-client.scm b/guix/http-client.scm index 4b4c14ed0b..6351e2d051 100644 --- a/guix/http-client.scm +++ b/guix/http-client.scm @@ -1,5 +1,5 @@ ;;; GNU Guix --- Functional package management for GNU -;;; Copyright =C2=A9 2012, 2013, 2014, 2015, 2016, 2017, 2018, 2020 Ludovi= c Court=C3=A8s +;;; Copyright =C2=A9 2012, 2013, 2014, 2015, 2016, 2017, 2018, 2020, 2021 = Ludovic Court=C3=A8s ;;; Copyright =C2=A9 2015 Mark H Weaver ;;; Copyright =C2=A9 2012, 2015 Free Software Foundation, Inc. ;;; Copyright =C2=A9 2017 Tobias Geerinckx-Rice @@ -147,6 +147,27 @@ Raise an '&http-get-error' condition if downloading fa= ils." (uri->string uri) code (response-reason-phrase resp)))))))))))) =20 +(define-syntax-rule (false-if-networking-error exp) + "Return #f if EXP triggers a network related exception." + ;; FIXME: Duplicated from 'with-cached-connection'. + (catch #t + (lambda () + exp) + (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 fr= om + ;; (web response) later on, once we've sent the request, or a + ;; ERROR/INVALID-SESSION from GnuTLS. + (if (or (and (eq? key 'system-error) + (=3D 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))) + #f + (apply throw key args))))) + (define* (http-multiple-get base-uri proc seed requests #:key port (verify-certificate? #t) (open-connection guix:open-connection-for-uri) @@ -219,42 +240,27 @@ 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) - (=3D 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-componen= t))) - (begin - (close-port p) - (connect #f ; try again - (drop requests (+ 1 processed)) - result)) - (apply throw key args)))))))))) + (match (false-if-networking-error (read-response p)) + ((? response? resp) + (let* ((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))))) + (#f + (close-port p) + (connect #f ; try again + (drop requests (+ 1 processed)) + result))))))))) =20 ;;; --=-=-= Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable I believe it=E2=80=99s a bit more readable because it moves =E2=80=98catch= =E2=80=99 out of sight and avoids the sort of =E2=80=9Cmini DSL=E2=80=9D where we return lists of = arguments. WDYT? Thanks, Ludo=E2=80=99. --=-=-=-- From unknown Sun Jun 22 17:15:27 2025 X-Loop: help-debbugs@gnu.org Subject: [bug#47288] [PATCH] guix: http-client: Tweak http-multiple-get error handling. Resent-From: Christopher Baines Original-Sender: "Debbugs-submit" Resent-CC: guix-patches@gnu.org Resent-Date: Fri, 26 Mar 2021 08:40:02 +0000 Resent-Message-ID: Resent-Sender: help-debbugs@gnu.org X-GNU-PR-Message: followup 47288 X-GNU-PR-Package: guix-patches X-GNU-PR-Keywords: patch To: Ludovic =?UTF-8?Q?Court=C3=A8s?= Cc: 47288@debbugs.gnu.org Received: via spool by 47288-submit@debbugs.gnu.org id=B47288.161674798819287 (code B ref 47288); Fri, 26 Mar 2021 08:40:02 +0000 Received: (at 47288) by debbugs.gnu.org; 26 Mar 2021 08:39:48 +0000 Received: from localhost ([127.0.0.1]:40332 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1lPi0R-000511-S6 for submit@debbugs.gnu.org; Fri, 26 Mar 2021 04:39:48 -0400 Received: from mira.cbaines.net ([212.71.252.8]:36126) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1lPi0P-00050s-HB for 47288@debbugs.gnu.org; Fri, 26 Mar 2021 04:39:46 -0400 Received: from localhost (unknown [IPv6:2a02:8010:68c1:0:8ac0:b4c7:f5c8:7caa]) by mira.cbaines.net (Postfix) with ESMTPSA id 841B027BC5C; Fri, 26 Mar 2021 08:39:44 +0000 (GMT) Received: from capella (localhost [127.0.0.1]) by localhost (OpenSMTPD) with ESMTP id 68faca7c; Fri, 26 Mar 2021 08:39:43 +0000 (UTC) References: <20210321004338.31867-1-mail@cbaines.net> <20210325110316.862-1-mail@cbaines.net> <87r1k250za.fsf_-_@gnu.org> User-agent: mu4e 1.4.15; emacs 27.1 From: Christopher Baines In-reply-to: <87r1k250za.fsf_-_@gnu.org> Date: Fri, 26 Mar 2021 08:39:41 +0000 Message-ID: <87h7ky9ulu.fsf@cbaines.net> MIME-Version: 1.0 Content-Type: multipart/signed; boundary="=-=-="; micalg=pgp-sha512; protocol="application/pgp-signature" X-Spam-Score: -0.0 (/) X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.18 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: debbugs-submit-bounces@debbugs.gnu.org Sender: "Debbugs-submit" X-Spam-Score: -1.0 (-) --=-=-= Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable Ludovic Court=C3=A8s writes: > What about the approach below: > > diff --git a/guix/http-client.scm b/guix/http-client.scm > index 4b4c14ed0b..6351e2d051 100644 > --- a/guix/http-client.scm > +++ b/guix/http-client.scm > @@ -1,5 +1,5 @@ > ;;; GNU Guix --- Functional package management for GNU > -;;; Copyright =C2=A9 2012, 2013, 2014, 2015, 2016, 2017, 2018, 2020 Ludo= vic Court=C3=A8s > +;;; Copyright =C2=A9 2012, 2013, 2014, 2015, 2016, 2017, 2018, 2020, 202= 1 Ludovic Court=C3=A8s > ;;; Copyright =C2=A9 2015 Mark H Weaver > ;;; Copyright =C2=A9 2012, 2015 Free Software Foundation, Inc. > ;;; Copyright =C2=A9 2017 Tobias Geerinckx-Rice > @@ -147,6 +147,27 @@ Raise an '&http-get-error' condition if downloading = fails." > (uri->string uri) code > (response-reason-phrase resp)))))))))))) >=20=20 > +(define-syntax-rule (false-if-networking-error exp) > + "Return #f if EXP triggers a network related exception." > + ;; FIXME: Duplicated from 'with-cached-connection'. > + (catch #t > + (lambda () > + exp) > + (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) > + (=3D 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))) > + #f > + (apply throw key args))))) > + > (define* (http-multiple-get base-uri proc seed requests > #:key port (verify-certificate? #t) > (open-connection guix:open-connection-for-ur= i) > @@ -219,42 +240,27 @@ 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 respons= e) > - ;; later on, once we've sent the request, or a > - ;; ERROR/INVALID-SESSION from GnuTLS. > - (if (or (and (eq? key 'system-error) > - (=3D 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-compon= ent))) > - (begin > - (close-port p) > - (connect #f ; try again > - (drop requests (+ 1 processed)) > - result)) > - (apply throw key args)))))))))) > + (match (false-if-networking-error (read-response p)) > + ((? response? resp) > + (let* ((body (response-body-port resp)) > + (result (proc head resp body result))) Given body is a port, and that port is passed to proc, I'm guessing it's possible for networking things to go wrong inside proc. > + ;; 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))))) > + (#f > + (close-port p) > + (connect #f ; try again > + (drop requests (+ 1 processed)) I realised earlier in this series of patches that this should actually be processed, rather than (+ 1 processed) since proc can't have been run for the current response. > + result))))))))) >=20=20 > > ;;; > > I believe it=E2=80=99s a bit more readable because it moves =E2=80=98catc= h=E2=80=99 out of sight > and avoids the sort of =E2=80=9Cmini DSL=E2=80=9D where we return lists o= f arguments. > > WDYT? It looks OK to me, I think the only thing to figure out for sure is whether it's safe to not include the activity on the body port in the error handling. Thanks, Chris --=-=-= Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- iQKlBAEBCgCPFiEEPonu50WOcg2XVOCyXiijOwuE9XcFAmBdnc1fFIAAAAAALgAo aXNzdWVyLWZwckBub3RhdGlvbnMub3BlbnBncC5maWZ0aGhvcnNlbWFuLm5ldDNF ODlFRUU3NDU4RTcyMEQ5NzU0RTBCMjVFMjhBMzNCMEI4NEY1NzcRHG1haWxAY2Jh aW5lcy5uZXQACgkQXiijOwuE9XcABBAAl7UOhAkQGxDeOVP3ZAUoECJ2tb5FFhwB a1RCXVWCVWlFnOnikiA0h84zI9bM8yVDPuctdpKCEvUZRTUoaGDxSzK0GkC4i0C0 MpCXSo3uX+cFGjgaZ38WWUX8WbRizpdnnuE+VCAByzsVvmge6LyZqcn0w6GeGno8 TGYfXVsworVAxxjQi1VA7ZyEqfiBvCCJ9YkZwnfmGmrb6Qu1qSDT0z4GGrPWgEwW kkGtXxno0H1D1PKkwt9YrvqpiMXx1lHW7W6QGmQnuBK/kuIGhMq7fCgS50COxvAU pKOqOheOuBtiDC6eAYBG7tOlvwUNX4dTW2212wsEENx1qUqGl/imDu5AlLEDE5bt eZDBaxuOEYS9bPw/oUHeGgS5ys2ds5Enp61PSmQef/1nthPGrhEaxBBV6Yt4+zx9 Y8oRZnviA9bDGvcybI4UGidicAoAcP2Z4ri8HLS46aynEVzZlfATMiuZeihqnmix TXoGmY9+cqpJMyvaYIGuJye2AoRhkQ0BB7Ua9UqfUCuVeihSrWSa4KGWVMeqbvSJ M1r3O1xvaThV1ckN3+umTxfOtKb7/H8qPCEYQ7Srfmdg0wDcn/wvTsdM0iIebICR XXK/1mQyFARWUt2MPTrgOFbciDc06hNLnlcvQbHe63HIfw1zF7cBN+36g0IF0ty6 r8AwmsnO/oE= =ihh+ -----END PGP SIGNATURE----- --=-=-=-- From unknown Sun Jun 22 17:15:27 2025 X-Loop: help-debbugs@gnu.org Subject: [bug#47288] [PATCH] guix: http-client: Tweak http-multiple-get error handling. Resent-From: Ludovic =?UTF-8?Q?Court=C3=A8s?= Original-Sender: "Debbugs-submit" Resent-CC: guix-patches@gnu.org Resent-Date: Sat, 27 Mar 2021 17:16:01 +0000 Resent-Message-ID: Resent-Sender: help-debbugs@gnu.org X-GNU-PR-Message: followup 47288 X-GNU-PR-Package: guix-patches X-GNU-PR-Keywords: patch To: Christopher Baines Cc: 47288@debbugs.gnu.org Received: via spool by 47288-submit@debbugs.gnu.org id=B47288.16168653546163 (code B ref 47288); Sat, 27 Mar 2021 17:16:01 +0000 Received: (at 47288) by debbugs.gnu.org; 27 Mar 2021 17:15:54 +0000 Received: from localhost ([127.0.0.1]:44847 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1lQCXS-0001bL-Co for submit@debbugs.gnu.org; Sat, 27 Mar 2021 13:15:54 -0400 Received: from eggs.gnu.org ([209.51.188.92]:40404) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1lQCXP-0001b5-QO for 47288@debbugs.gnu.org; Sat, 27 Mar 2021 13:15:52 -0400 Received: from fencepost.gnu.org ([2001:470:142:3::e]:39410) by eggs.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1lQCXK-00023y-Cu; Sat, 27 Mar 2021 13:15:46 -0400 Received: from [2a01:e0a:1d:7270:af76:b9b:ca24:c465] (port=41816 helo=ribbon) by fencepost.gnu.org with esmtpsa (TLS1.2:RSA_AES_256_CBC_SHA1:256) (Exim 4.82) (envelope-from ) id 1lQCXJ-0003le-GR; Sat, 27 Mar 2021 13:15:45 -0400 From: Ludovic =?UTF-8?Q?Court=C3=A8s?= References: <20210321004338.31867-1-mail@cbaines.net> <20210325110316.862-1-mail@cbaines.net> <87r1k250za.fsf_-_@gnu.org> <87h7ky9ulu.fsf@cbaines.net> Date: Sat, 27 Mar 2021 18:15:42 +0100 In-Reply-To: <87h7ky9ulu.fsf@cbaines.net> (Christopher Baines's message of "Fri, 26 Mar 2021 08:39:41 +0000") Message-ID: <87mtuozfep.fsf_-_@gnu.org> User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/27.1 (gnu/linux) MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable X-Spam-Score: -0.7 (/) X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.18 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: debbugs-submit-bounces@debbugs.gnu.org Sender: "Debbugs-submit" X-Spam-Score: -1.7 (-) Hi, Christopher Baines skribis: > Ludovic Court=C3=A8s writes: [...] >> + (match (false-if-networking-error (read-response p)) >> + ((? response? resp) >> + (let* ((body (response-body-port resp)) >> + (result (proc head resp body result))) > > Given body is a port, and that port is passed to proc, I'm guessing it's > possible for networking things to go wrong inside proc. Yes, but how is this different from a regular read(2) call as made by =E2=80=98get-bytevector-n=E2=80=99 or whatever? We wouldn=E2=80=99t write = every read(2) call in =E2=80=98catch=E2=80=99 because in general any error there is indeed except= ional. I think the only bit that=E2=80=99s =E2=80=9Cless exceptional=E2=80=9D here= is that, because we=E2=80=99re reusing cached connection, we know that the first read(2) or = the first write(2) to that port can trigger one of these errors=E2=80=94which, = we know are not =E2=80=9Cexceptional=E2=80=9D. Errors in subsequent read(2) o= r write(2) calls remain exceptional/unrecoverable and should be treated as such IMO. Does that make sense? (In that sense, I think wrapping every =E2=80=98read-response=E2=80=99 call= rather than just the first one is already too much, but that=E2=80=99s okay.) >> + ;; 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))))) >> + (#f >> + (close-port p) >> + (connect #f ; try again >> + (drop requests (+ 1 processed)) > > I realised earlier in this series of patches that this should actually > be processed, rather than (+ 1 processed) since proc can't have been run > for the current response. Oh, something to fix in a subsequent commit, then. All in all, I propose to go with this patch if that=E2=80=99s fine with you. Thanks! Ludo=E2=80=99. From unknown Sun Jun 22 17:15:27 2025 MIME-Version: 1.0 X-Mailer: MIME-tools 5.505 (Entity 5.505) X-Loop: help-debbugs@gnu.org From: help-debbugs@gnu.org (GNU bug Tracking System) To: Christopher Baines Subject: bug#47288: closed (Re: bug#47288: [PATCH] guix: http-client: Tweak http-multiple-get error handling.) Message-ID: References: <87mtuoxnqo.fsf_-_@gnu.org> <20210321004338.31867-1-mail@cbaines.net> X-Gnu-PR-Message: they-closed 47288 X-Gnu-PR-Package: guix-patches X-Gnu-PR-Keywords: patch Reply-To: 47288@debbugs.gnu.org Date: Sat, 27 Mar 2021 21:59:03 +0000 Content-Type: multipart/mixed; boundary="----------=_1616882343-9661-1" This is a multi-part message in MIME format... ------------=_1616882343-9661-1 Content-Disposition: inline Content-Transfer-Encoding: quoted-printable Content-Type: text/plain; charset="utf-8" Your bug report #47288: [PATCH] guix: http-client: Tweak http-multiple-get error handling. 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 47288@debbugs.gnu.org. --=20 47288: http://debbugs.gnu.org/cgi/bugreport.cgi?bug=3D47288 GNU Bug Tracking System Contact help-debbugs@gnu.org with problems ------------=_1616882343-9661-1 Content-Type: message/rfc822 Content-Disposition: inline Content-Transfer-Encoding: 7bit Received: (at 47288-done) by debbugs.gnu.org; 27 Mar 2021 21:58:50 +0000 Received: from localhost ([127.0.0.1]:45101 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1lQGxG-0002V3-Cl for submit@debbugs.gnu.org; Sat, 27 Mar 2021 17:58:50 -0400 Received: from eggs.gnu.org ([209.51.188.92]:56178) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1lQGxD-0002Uk-DP; Sat, 27 Mar 2021 17:58:48 -0400 Received: from fencepost.gnu.org ([2001:470:142:3::e]:43631) by eggs.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1lQGx7-0007fi-Tc; Sat, 27 Mar 2021 17:58:41 -0400 Received: from [2a01:e0a:1d:7270:af76:b9b:ca24:c465] (port=43670 helo=ribbon) by fencepost.gnu.org with esmtpsa (TLS1.2:RSA_AES_256_CBC_SHA1:256) (Exim 4.82) (envelope-from ) id 1lQGx6-0005Mj-UD; Sat, 27 Mar 2021 17:58:41 -0400 From: =?utf-8?Q?Ludovic_Court=C3=A8s?= To: Christopher Baines Subject: Re: bug#47288: [PATCH] guix: http-client: Tweak http-multiple-get error handling. References: <20210321004338.31867-1-mail@cbaines.net> <20210325110316.862-1-mail@cbaines.net> <87r1k250za.fsf_-_@gnu.org> <87h7ky9ulu.fsf@cbaines.net> <87mtuozfep.fsf_-_@gnu.org> Date: Sat, 27 Mar 2021 22:58:39 +0100 In-Reply-To: <87mtuozfep.fsf_-_@gnu.org> ("Ludovic =?utf-8?Q?Court=C3=A8s?= =?utf-8?Q?=22's?= message of "Sat, 27 Mar 2021 18:15:42 +0100") Message-ID: <87mtuoxnqo.fsf_-_@gnu.org> User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/27.1 (gnu/linux) MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable X-Spam-Score: -0.7 (/) X-Debbugs-Envelope-To: 47288-done Cc: 47283-done@debbugs.gnu.org, 47288-done@debbugs.gnu.org X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.18 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: debbugs-submit-bounces@debbugs.gnu.org Sender: "Debbugs-submit" X-Spam-Score: -1.7 (-) Hi! I went ahead and pushed 45fce38fb0b6c6796906149ade145b8d3594c1c6 along these lines. =E2=80=98guix weather=E2=80=99 runs to completion and things seem to work f= ine. Let me know if you notice anything wrong. Thank you for your work and for your patience on this issue! Ludo=E2=80=99. ------------=_1616882343-9661-1 Content-Type: message/rfc822 Content-Disposition: inline Content-Transfer-Encoding: 7bit Received: (at submit) by debbugs.gnu.org; 21 Mar 2021 00:43:51 +0000 Received: from localhost ([127.0.0.1]:53355 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1lNmC6-0003Fx-IX for submit@debbugs.gnu.org; Sat, 20 Mar 2021 20:43:50 -0400 Received: from lists.gnu.org ([209.51.188.17]:58442) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1lNmC3-0003Fn-EB for submit@debbugs.gnu.org; Sat, 20 Mar 2021 20:43:48 -0400 Received: from eggs.gnu.org ([2001:470:142:3::10]:58066) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1lNmC2-0002SU-JG for guix-patches@gnu.org; Sat, 20 Mar 2021 20:43:46 -0400 Received: from mira.cbaines.net ([2a01:7e00:e000:2f8:fd4d:b5c7:13fb:3d27]:38847) by eggs.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1lNmBy-0002t7-De for guix-patches@gnu.org; Sat, 20 Mar 2021 20:43:46 -0400 Received: from localhost (unknown [IPv6:2a02:8010:68c1:0:8ac0:b4c7:f5c8:7caa]) by mira.cbaines.net (Postfix) with ESMTPSA id 0954327BC57 for ; Sun, 21 Mar 2021 00:43:40 +0000 (GMT) Received: from localhost (localhost [local]) by localhost (OpenSMTPD) with ESMTPA id 886c814a for ; Sun, 21 Mar 2021 00:43:38 +0000 (UTC) From: Christopher Baines To: guix-patches@gnu.org Subject: [PATCH] guix: http-client: Tweak http-multiple-get error handling. Date: Sun, 21 Mar 2021 00:43:38 +0000 Message-Id: <20210321004338.31867-1-mail@cbaines.net> X-Mailer: git-send-email 2.30.1 MIME-Version: 1.0 Content-Transfer-Encoding: 8bit Received-SPF: pass client-ip=2a01:7e00:e000:2f8:fd4d:b5c7:13fb:3d27; envelope-from=mail@cbaines.net; helo=mira.cbaines.net X-Spam_score_int: -18 X-Spam_score: -1.9 X-Spam_bar: - X-Spam_report: (-1.9 / 5.0 requ) BAYES_00=-1.9, SPF_HELO_PASS=-0.001, SPF_PASS=-0.001, UNPARSEABLE_RELAY=0.001 autolearn=ham autolearn_force=no X-Spam_action: no action X-Spam-Score: -1.4 (-) X-Debbugs-Envelope-To: submit X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.18 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: debbugs-submit-bounces@debbugs.gnu.org Sender: "Debbugs-submit" X-Spam-Score: -2.4 (--) 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, 41 insertions(+), 36 deletions(-) diff --git a/guix/http-client.scm b/guix/http-client.scm index 4b4c14ed0b..a189cceecf 100644 --- a/guix/http-client.scm +++ b/guix/http-client.scm @@ -219,42 +219,47 @@ 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 - (drop requests (+ 1 processed)) - result)) - (apply throw key args)))))))))) + (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) + 'try-again-with-new-connection) + (_ + 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) + 'try-again-with-new-connection) + (apply throw key args)))) + ('try-again-with-new-connection + (connect #f + (drop requests (+ 1 processed)) + result)) + (result + (loop tail (+ 1 processed) result))))))))) ;;; -- 2.30.1 ------------=_1616882343-9661-1--