From b38a6e5f2f0baa94006761ac6cc995a8801fcdca Mon Sep 17 00:00:00 2001 From: Christopher Zimmermann Date: Tue, 4 Jan 2022 22:11:47 +0100 Subject: [PATCH 1/8] Remove obsolete second wakeup_paused It was introduced in d5822af9a80b745515e0988541d7a43ed9a6faba. If I understand it correctly this was done to prevent calling select without timeout while there were pending paused promises. Since now we explicitely check for this condition and call select with zero timeout (`should_block_waiting_for_io`). This extra `wakeup_paused` seems to be obsolete. --- CHANGES | 4 ++++ src/unix/lwt_main.ml | 4 +--- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/CHANGES b/CHANGES index 5a80a2d1e7..53f1de0869 100644 --- a/CHANGES +++ b/CHANGES @@ -89,6 +89,10 @@ * Alias Lwt_result.map_err and Lwt_result.bind_lwt_err to Lwt_result.map_error and Lwt_result.bind_lwt_error for consistency with Stdlib. (#927, Antonin Décimo) +====== Misc ====== + + * Resolve paused promises only once in main loop. This lets Lwt.pause behave identical to Lwt_unix.yield.(#917, Christopher Zimmermann) + ===== 5.5.0 ===== ====== Deprecations ====== diff --git a/src/unix/lwt_main.ml b/src/unix/lwt_main.ml index 0cac449b87..17ea3c5a1a 100644 --- a/src/unix/lwt_main.ml +++ b/src/unix/lwt_main.ml @@ -26,8 +26,6 @@ let abandon_yielded_and_paused () = let run p = let rec run_loop () = - (* Fulfill paused promises now. *) - Lwt.wakeup_paused (); match Lwt.poll p with | Some x -> x @@ -40,7 +38,7 @@ let run p = Lwt.paused_count () = 0 && Lwt_sequence.is_empty yielded in Lwt_engine.iter should_block_waiting_for_io; - (* Fulfill paused promises again. *) + (* Fulfill paused promises. *) Lwt.wakeup_paused (); (* Fulfill yield promises. *) From 8c693243f352f0f67b6682ddc0f7554cecb0c191 Mon Sep 17 00:00:00 2001 From: Christopher Zimmermann Date: Thu, 6 Jan 2022 18:07:18 +0100 Subject: [PATCH 2/8] document how to wait for one whole iteration of main loop --- src/core/lwt.mli | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/core/lwt.mli b/src/core/lwt.mli index 4d8ad6dd60..9c9c2d002d 100644 --- a/src/core/lwt.mli +++ b/src/core/lwt.mli @@ -1836,6 +1836,9 @@ val pause : unit -> unit t Putting the rest of your computation into a callback of [Lwt.pause ()] creates a “yield” that gives other callbacks a chance to run first. + To wait for one complete iteration of the main loop you need to wait for + [Lwt.pause () >>= Lwt.pause] + For example, to break up a long-running computation, allowing I/O to be handled between chunks: From 322cc45db73aaf1f77041db91e53d9ab78d773d2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rapha=C3=ABl=20Proust?= Date: Fri, 21 Jan 2022 12:07:18 +0100 Subject: [PATCH 3/8] Deprecated unix's yield functions implemented as calls to Lwt.pause --- src/core/lwt.mli | 3 --- src/unix/lwt_main.ml | 14 ++------------ src/unix/lwt_main.mli | 14 ++++++-------- src/unix/lwt_unix.cppo.ml | 2 +- 4 files changed, 9 insertions(+), 24 deletions(-) diff --git a/src/core/lwt.mli b/src/core/lwt.mli index 9c9c2d002d..4d8ad6dd60 100644 --- a/src/core/lwt.mli +++ b/src/core/lwt.mli @@ -1836,9 +1836,6 @@ val pause : unit -> unit t Putting the rest of your computation into a callback of [Lwt.pause ()] creates a “yield” that gives other callbacks a chance to run first. - To wait for one complete iteration of the main loop you need to wait for - [Lwt.pause () >>= Lwt.pause] - For example, to break up a long-running computation, allowing I/O to be handled between chunks: diff --git a/src/unix/lwt_main.ml b/src/unix/lwt_main.ml index 17ea3c5a1a..ca1f68f19e 100644 --- a/src/unix/lwt_main.ml +++ b/src/unix/lwt_main.ml @@ -16,12 +16,10 @@ open Lwt.Infix let enter_iter_hooks = Lwt_sequence.create () let leave_iter_hooks = Lwt_sequence.create () -let yielded = Lwt_sequence.create () -let yield () = (Lwt.add_task_r [@ocaml.warning "-3"]) yielded +let yield () = Lwt.pause () let abandon_yielded_and_paused () = - Lwt_sequence.clear yielded; Lwt.abandon_paused () let run p = @@ -34,20 +32,12 @@ let run p = Lwt_sequence.iter_l (fun f -> f ()) enter_iter_hooks; (* Do the main loop call. *) - let should_block_waiting_for_io = - Lwt.paused_count () = 0 && Lwt_sequence.is_empty yielded in + let should_block_waiting_for_io = Lwt.paused_count () = 0 in Lwt_engine.iter should_block_waiting_for_io; (* Fulfill paused promises. *) Lwt.wakeup_paused (); - (* Fulfill yield promises. *) - if not (Lwt_sequence.is_empty yielded) then begin - let tmp = Lwt_sequence.create () in - Lwt_sequence.transfer_r yielded tmp; - Lwt_sequence.iter_l (fun resolver -> Lwt.wakeup resolver ()) tmp - end; - (* Call leave hooks. *) Lwt_sequence.iter_l (fun f -> f ()) leave_iter_hooks; diff --git a/src/unix/lwt_main.mli b/src/unix/lwt_main.mli index 0d5c6b3dc7..9e33dd9a36 100644 --- a/src/unix/lwt_main.mli +++ b/src/unix/lwt_main.mli @@ -49,14 +49,9 @@ val yield : unit -> unit Lwt.t [@@deprecated "Use Lwt.pause instead"] @deprecated Since 5.5.0 [yield] is deprecated in favor of the more general {!Lwt.pause} in order to avoid discrepancies in resolution (see below) and - stay compatible with other execution environments such as js_of_ocaml. + stay compatible with other execution environments such as js_of_ocaml. *) - Currently, paused promises are resolved more frequently than yielded promises. - The difference is unintended but existing applications could depend on it. - Unifying the two pools of promises into one in the future would eliminate - possible discrepancies and simplify the code. *) - -val abandon_yielded_and_paused : unit -> unit +val abandon_yielded_and_paused : unit -> unit [@@deprecated "Use Lwt.abandon_paused instead"] (** Causes promises created with {!Lwt.pause} and {!Lwt_main.yield} to remain forever pending. @@ -64,7 +59,10 @@ val abandon_yielded_and_paused : unit -> unit Once [yield] is phased out, this function will be deprecated as well. This is meant for use with {!Lwt_unix.fork}, as a way to “abandon” more - promise chains that are pending in your process. *) + promise chains that are pending in your process. + + @deprecated Since 5.5.1 [abandon_yielded_and_paused] is deprecated in favour + of [Lwt.abandon_paused]. *) diff --git a/src/unix/lwt_unix.cppo.ml b/src/unix/lwt_unix.cppo.ml index eda06efdf2..8d1fa7f313 100644 --- a/src/unix/lwt_unix.cppo.ml +++ b/src/unix/lwt_unix.cppo.ml @@ -123,7 +123,7 @@ let sleep delay = Lwt.on_cancel waiter (fun () -> Lwt_engine.stop_event ev); waiter -let yield = (Lwt_main.yield [@warning "-3"]) +let yield = Lwt.pause let auto_yield timeout = let limit = ref (Unix.gettimeofday () +. timeout) in From d19212007f4f609657bdcc037e110817e6f11ab6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rapha=C3=ABl=20Proust?= Date: Mon, 17 Jul 2023 17:13:17 +0200 Subject: [PATCH 4/8] Update CHANGES after rebase --- CHANGES | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/CHANGES b/CHANGES index 53f1de0869..dbecace81c 100644 --- a/CHANGES +++ b/CHANGES @@ -39,6 +39,10 @@ * Fix marshall header size in Lwt_io.read_value. (Simmo Saan, #995) +====== Misc ====== + + * Resolve paused promises only once in main loop. This lets Lwt.pause behave identical to Lwt_unix.yield. (#917, Christopher Zimmermann) + ===== 5.6.1 ===== @@ -89,10 +93,6 @@ * Alias Lwt_result.map_err and Lwt_result.bind_lwt_err to Lwt_result.map_error and Lwt_result.bind_lwt_error for consistency with Stdlib. (#927, Antonin Décimo) -====== Misc ====== - - * Resolve paused promises only once in main loop. This lets Lwt.pause behave identical to Lwt_unix.yield.(#917, Christopher Zimmermann) - ===== 5.5.0 ===== ====== Deprecations ====== From e4a74cc5ad50bbb468eface71dc703d893b5699b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rapha=C3=ABl=20Proust?= Date: Wed, 26 Jul 2023 09:08:21 +0200 Subject: [PATCH 5/8] Update doc of abandon_yielded_and_paused --- src/unix/lwt_main.mli | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/unix/lwt_main.mli b/src/unix/lwt_main.mli index 9e33dd9a36..1cb3363e50 100644 --- a/src/unix/lwt_main.mli +++ b/src/unix/lwt_main.mli @@ -55,8 +55,7 @@ val abandon_yielded_and_paused : unit -> unit [@@deprecated "Use Lwt.abandon_pau (** Causes promises created with {!Lwt.pause} and {!Lwt_main.yield} to remain forever pending. - [yield] is now deprecated in favor of the more general {!Lwt.pause}. - Once [yield] is phased out, this function will be deprecated as well. + (Note that [yield] is deprecated in favor of the more general {!Lwt.pause}.) This is meant for use with {!Lwt_unix.fork}, as a way to “abandon” more promise chains that are pending in your process. From 0baa154ebb0866cbde1977791871b2d5b32b87ee Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rapha=C3=ABl=20Proust?= Date: Wed, 26 Jul 2023 09:11:48 +0200 Subject: [PATCH 6/8] Update deprecation comment version number --- src/unix/lwt_main.mli | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/unix/lwt_main.mli b/src/unix/lwt_main.mli index 1cb3363e50..3b4f174b74 100644 --- a/src/unix/lwt_main.mli +++ b/src/unix/lwt_main.mli @@ -60,7 +60,7 @@ val abandon_yielded_and_paused : unit -> unit [@@deprecated "Use Lwt.abandon_pau This is meant for use with {!Lwt_unix.fork}, as a way to “abandon” more promise chains that are pending in your process. - @deprecated Since 5.5.1 [abandon_yielded_and_paused] is deprecated in favour + @deprecated Since 5.7 [abandon_yielded_and_paused] is deprecated in favour of [Lwt.abandon_paused]. *) From 0db9599a80794a851d225737ef8728b172a64dcf Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rapha=C3=ABl=20Proust?= Date: Fri, 28 Jul 2023 11:23:51 +0200 Subject: [PATCH 7/8] Apply suggestion --- src/unix/lwt_main.ml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/unix/lwt_main.ml b/src/unix/lwt_main.ml index ca1f68f19e..25d0ccaeec 100644 --- a/src/unix/lwt_main.ml +++ b/src/unix/lwt_main.ml @@ -17,7 +17,7 @@ open Lwt.Infix let enter_iter_hooks = Lwt_sequence.create () let leave_iter_hooks = Lwt_sequence.create () -let yield () = Lwt.pause () +let yield = Lwt.pause let abandon_yielded_and_paused () = Lwt.abandon_paused () From 6419816b2b343dedb163f4e0eeac493f947fd9f1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rapha=C3=ABl=20Proust?= Date: Fri, 28 Jul 2023 11:27:52 +0200 Subject: [PATCH 8/8] mention reviewer in changelog --- CHANGES | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGES b/CHANGES index dbecace81c..2b0328dd65 100644 --- a/CHANGES +++ b/CHANGES @@ -41,7 +41,7 @@ ====== Misc ====== - * Resolve paused promises only once in main loop. This lets Lwt.pause behave identical to Lwt_unix.yield. (#917, Christopher Zimmermann) + * Resolve paused promises only once in main loop. This lets Lwt.pause behave identical to Lwt_unix.yield. (#917, Christopher Zimmermann, Favonia) ===== 5.6.1 =====