Skip to content

Commit

Permalink
Reuse Buf_write formatter
Browse files Browse the repository at this point in the history
  • Loading branch information
SGrondin committed Jan 6, 2024
1 parent 74e506f commit 211c259
Show file tree
Hide file tree
Showing 3 changed files with 25 additions and 14 deletions.
33 changes: 22 additions & 11 deletions lib_eio/buf_write.ml
Original file line number Diff line number Diff line change
Expand Up @@ -163,6 +163,7 @@ type t =
; mutable bytes_written : int (* Total written bytes. Wraps. *)
; mutable state : state
; mutable wake_writer : unit -> unit
; mutable printf : (Format.formatter * bool ref) option
}
(* Invariant: [write_pos >= scheduled_pos] *)

Expand Down Expand Up @@ -377,6 +378,7 @@ let of_buffer ?sw buffer =
; bytes_written = 0
; state = Active
; wake_writer = ignore
; printf = None
}
in
begin match sw with
Expand Down Expand Up @@ -422,19 +424,28 @@ let make_formatter t =
(fun () -> flush t)

let printf t =
let is_formatting = ref true in
let ppf =
Format.make_formatter
(fun buf off len -> write_gen t buf ~off ~len ~blit:Bigstringaf.blit_from_string)
(fun () ->
(* As per the Format module manual, an explicit flush writes to the
output channel and ensures that "all pending text is displayed"
and "these explicit flush calls [...] could dramatically impact efficiency".
Therefore it is clear that we need to call `flush t` instead of `flush_buffer t`. *)
if !is_formatting then flush t)
let ppf, is_formatting =
match t.printf with
| Some (_, is_formatting as x) ->
is_formatting := true;
x
| None ->
let is_formatting = ref true in
let ppf =
Format.make_formatter
(fun buf off len -> write_gen t buf ~off ~len ~blit:Bigstringaf.blit_from_string)
(fun () ->
(* As per the Format module manual, an explicit flush writes to the
output channel and ensures that "all pending text is displayed"
and "these explicit flush calls [...] could dramatically impact efficiency".
Therefore it is clear that we need to call `flush t` instead of `flush_buffer t`. *)
if !is_formatting then flush t)
in
t.printf <- Some (ppf, is_formatting);
ppf, is_formatting
in
Format.kfprintf (fun ppf ->
assert !is_formatting;
if not !is_formatting then raise (Sys_error "Buf_write.printf: invalid concurrent access");
(* Ensure that [ppf]'s internal buffer is flushed to [t], but without flushing [t] itself: *)
is_formatting := false;
Format.pp_print_flush ppf ()
Expand Down
3 changes: 1 addition & 2 deletions lib_eio/buf_write.mli
Original file line number Diff line number Diff line change
Expand Up @@ -126,8 +126,7 @@ val cstruct : t -> Cstruct.t -> unit
val printf : t -> ('a, Format.formatter, unit) format -> 'a
(** [printf t fmt x y z] formats the arguments according to the format string [fmt].
It supports all formatting and pretty-printing features of the Format module.
Accordingly, explicit flushes using [@.] or [%!] must perform a full (blocking) flush
so consider using [Fiber.fork] in such cases. *)
Accordingly, explicit flushes using [@.] or [%!] perform a full (blocking) flush. *)

val make_formatter : t -> Format.formatter
(** [make_formatter t] creates a new formatter that writes to [t].
Expand Down
3 changes: 2 additions & 1 deletion tests/buf_write.md
Original file line number Diff line number Diff line change
Expand Up @@ -170,14 +170,15 @@ With pausing
Fmt.pf f "Space@ @[<v 6>%s@ %i@.%a@]@ "
"This is a test" 123
Eio.Net.Sockaddr.pp (`Tcp (Eio.Net.Ipaddr.V6.loopback, 8080));
Write.printf t "This is a %s call to printf" "second";
Fmt.pf f "@.Flushed. %s@." "Goodbye"
+flow: wrote "Write.printf can force a full flush\n"
+flow: wrote "It also\n"
+ " flushes to [t] automatically at the end, but without flushing [t] itself.\n"
+ "Space\n"
+ "This is a test\n"
+ " 123\n"
+flow: wrote "tcp:[::1]:8080\n"
+flow: wrote "tcp:[::1]:8080This is a second call to printf\n"
+ "\n"
+flow: wrote "Flushed. Goodbye\n"
- : unit = ()
Expand Down

0 comments on commit 211c259

Please sign in to comment.