Conduit Free Servers

[rgrinberg]

Conduit on the server side of cohttp is a performance bottleneck and a source of complexity (dependencies, larger api surface). On the other hand, it is also rarely useful. The standard way to deploy web servers is behind a proxy that handles SSL termination. Nginx, HAProxy, Envoy are common examples of such proxies. Therefore conduit is mostly useful in environments where self contained servers that do their own SSL termination are necessary. Mirage is the obvious example here.

My proposal is therefore to provide a version of cohttp-async and cohttp-lwt-unix that is free of conduit. In particular, I propose the following two new packages:

• cohttp-server-lwt-unix
• cohttp-server-async

My reasoning is that this option is minimally disruptive to end users. In particular, users that use the existing cohttp-async and cohttp-lwt-unix packages for the http clients will not be disturbed at all. As conduit remains very useful for http clients, there's no justification to break their existing code.

[samoht]

I think that's a good idea. For Mirage, we are moving away from conduit from the server-side, where, as you said, using something this flexible is rarely useful anyway.

We'd like also to move away from the current design for the client-side, but we are not sure on which API yet (as the revert of the conduit 3.0 API showed). Clearly, we (at least I :p) are not super happy with the current design of that library, but it does its job well and it's difficult to replace without exposing a horrible or convoluted API to the user.

[mseri]

For future reference, in #819 a test was made dropping conduit-lwt-unix and using directly Lwt_unix.file_descr and it resulted in a server that was able to handle 100_000. RPS with average latencies of 2ms with tail latencies of 7.6ms, basically on par with httpaf. Using conduit-lwt-unix in the same test, the average latencies degraded to 1.2 seconds with tail latencies of 2 seconds.

You can test this using the following patch:

From 8876adb574e973ce661b41296d167e2c556fcbfe Mon Sep 17 00:00:00 2001
From: Anurag Soni <[email protected]>
Date: Thu, 6 Jan 2022 22:24:57 -0500
Subject: [PATCH] try lwt fd instead of Lwt_io for input channel

---
 cohttp-lwt-unix/src/bytebuffer.ml    |  2 +-
 cohttp-lwt-unix/src/input_channel.ml |  2 +-
 cohttp-lwt-unix/src/io.ml            |  2 +-
 cohttp-lwt-unix/src/io.mli           |  2 +-
 cohttp-lwt-unix/src/server.ml        | 17 +++++++++++++----
 5 files changed, 17 insertions(+), 8 deletions(-)

diff --git a/cohttp-lwt-unix/src/bytebuffer.ml b/cohttp-lwt-unix/src/bytebuffer.ml
index 46513aca3..9eff58717 100644
--- a/cohttp-lwt-unix/src/bytebuffer.ml
+++ b/cohttp-lwt-unix/src/bytebuffer.ml
@@ -47,7 +47,7 @@ let drop t len =
 
 let refill t reader =
   compact t;
-  Lwt_io.read_into reader t.buf t.pos_fill (Bytes.length t.buf - t.pos_fill)
+  Lwt_unix.read reader t.buf t.pos_fill (Bytes.length t.buf - t.pos_fill)
   >|= fun count ->
   if count > 0 then (
     t.pos_fill <- t.pos_fill + count;
diff --git a/cohttp-lwt-unix/src/input_channel.ml b/cohttp-lwt-unix/src/input_channel.ml
index 0701f8681..82a43cdd8 100644
--- a/cohttp-lwt-unix/src/input_channel.ml
+++ b/cohttp-lwt-unix/src/input_channel.ml
@@ -1,6 +1,6 @@
 open Lwt.Infix
 
-type t = { buf : Bytebuffer.t; chan : Lwt_io.input_channel }
+type t = { buf : Bytebuffer.t; chan : Lwt_unix.file_descr }
 
 let create ?(buf_len = 0x4000) chan = { buf = Bytebuffer.create buf_len; chan }
 
diff --git a/cohttp-lwt-unix/src/io.ml b/cohttp-lwt-unix/src/io.ml
index be5cf85f1..6a45be201 100644
--- a/cohttp-lwt-unix/src/io.ml
+++ b/cohttp-lwt-unix/src/io.ml
@@ -29,7 +29,7 @@ let return = Lwt.return
 
 type ic = Input_channel.t
 type oc = Lwt_io.output_channel
-type conn = Conduit_lwt_unix.flow
+type conn = unit
 
 let src = Logs.Src.create "cohttp.lwt.io" ~doc:"Cohttp Lwt IO module"
 
diff --git a/cohttp-lwt-unix/src/io.mli b/cohttp-lwt-unix/src/io.mli
index 2638ffecd..3b0056a1e 100644
--- a/cohttp-lwt-unix/src/io.mli
+++ b/cohttp-lwt-unix/src/io.mli
@@ -23,5 +23,5 @@ include
   Cohttp_lwt.S.IO
     with type ic = Input_channel.t
      and type oc = Lwt_io.output_channel
-     and type conn = Conduit_lwt_unix.flow
+     and type conn = unit
      and type error = exn
diff --git a/cohttp-lwt-unix/src/server.ml b/cohttp-lwt-unix/src/server.ml
index b700d2e58..cd3d81009 100644
--- a/cohttp-lwt-unix/src/server.ml
+++ b/cohttp-lwt-unix/src/server.ml
@@ -66,7 +66,16 @@ let log_on_exn = function
 
 let create ?timeout ?backlog ?stop ?(on_exn = log_on_exn)
     ?(ctx = Net.default_ctx) ?(mode = `TCP (`Port 8080)) spec =
-  Conduit_lwt_unix.serve ?backlog ?timeout ?stop ~on_exn ~ctx:ctx.Net.ctx ~mode
-    (fun flow ic oc ->
-      let ic = Input_channel.create ic in
-      callback spec flow ic oc)
+  match mode with
+  | `TCP (`Port port) ->
+      let listen_address = Unix.(ADDR_INET (inet_addr_loopback, port)) in
+      Lwt.async (fun () ->
+          Lwt_io.establish_server_with_client_socket ~backlog:11_000
+            listen_address (fun _addr socket ->
+              let ic = Input_channel.create socket in
+              let oc = Lwt_io.of_fd ~mode:Lwt_io.output socket in
+              callback spec () ic oc)
+          >>= fun _server -> Lwt.return_unit);
+      let forever, _ = Lwt.wait () in
+      forever
+  | _ -> assert false

[smorimoto]

I also would like to support this good idea. This is exactly what I have been thinking for the past few months.

[dinosaure]

Just be clear, even if I want to delete ocaml-conduit (and this is what we mostly did on the MirageOS side), I should notice you that ocaml-conduit keeps the MirageOS compatibility. That mostly means that if you want to go further on this side, you should keep in your mind that we must keep a certain abstraction to ensure, again, the compatibility with MirageOS (which does not have a socket a priori).

[rgrinberg]

Re-opening because this needs to be done for Async as well.

[anuragsoni]

@rgrinberg as part of https://github.com/mirage/ocaml-cohttp/pull/839 we added an api for async that allows using cohttp-async servers with reader/writer without going through conduit. This should cover the conduit-free interface for cohttp-async.

[rgrinberg]

@anuragsoni i'm looking at cohttp_async/server.mli and i don't see an api for creating a server without conduit. Am I missing something?

[anuragsoni]

@rgrinberg Its the Expert module within cohttp-async https://github.com/mirage/ocaml-cohttp/blob/master/cohttp-async/src/server.mli#L109-L133 My intention behind it was for the user to create a tcp server using one of functions within Tcp.Server in async (https://ocaml.org/p/async_unix/v0.15.0/doc/Async_unix/Tcp/Server/index.html#val-create_sock) and then forward the reader/writer to cohttp-async. I liked this as it give the user full control over all the various Tcp options that async lets you configure without cohttp having to duplicate them in its public api.

[rgrinberg]

Hmm then I would suggest that we factor out the package somehow to reflect this. There should be a way to use this Expert module without downloading and installing conduit.