Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add buffering of outgoing messages #287

Merged
merged 1 commit into from
Nov 17, 2024
Merged

Conversation

talex5
Copy link
Contributor

@talex5 talex5 commented Nov 16, 2024

Sending each message in its own TCP packet isn't very efficient, and also interacts very badly with Nagle's algorithm. See https://roscidus.com/blog/blog/2024/07/22/performance/ for details.

Note: The Prometheus metrics messages_outbound_sent_total and messages_outbound_dropped_total have gone. They weren't very useful and we no longer know the number of messages by the time the connection is dropped (could report dropped bytes if needed though).

dune exec -- ./test-bin/echo/echo_bench.exe
echo_bench.exe: [INFO] rate = 522.989613	# Before
echo_bench.exe: [INFO] rate = 62439.433376  # After

@talex5 talex5 added this to the v2.0 (Eio) milestone Nov 16, 2024
@talex5 talex5 force-pushed the buf-write branch 2 times, most recently from 84d58a3 to 73c98db Compare November 17, 2024 12:01
Sending each message in its own TCP packet isn't very efficient,
and also interacts very badly with Nagle's algorithm. See
https://roscidus.com/blog/blog/2024/07/22/performance/ for details.

Note: The Prometheus metrics `messages_outbound_sent_total` and
`messages_outbound_dropped_total` have gone. They weren't very useful
and we no longer know the number of messages by the time the connection
is dropped (could report dropped bytes if needed though).

    dune exec -- ./test-bin/echo/echo_bench.exe
    echo_bench.exe: [INFO] rate = 522.989613	# Before
    echo_bench.exe: [INFO] rate = 62439.433376  # After

This commit also improves the log messages a bit.
@@ -67,8 +99,23 @@ let rec recv t =
let disconnect t =
try
Eio.Flow.shutdown t.flow `All
with
| Invalid_argument _
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We now don't close the socket until both send and receive fibers are done, so Invalid_argument shouldn't happen now.

@@ -187,8 +106,6 @@ module Make (Network : S.NETWORK) = struct
);
send_abort t (Capnp_rpc.Exception.v ~ty:`Failed (Printexc.to_string ex))
end;
Log.info (fun f -> f ~tags "Connection closed");
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Endpont already reports when the connection is closed, and with a better message.

@talex5 talex5 merged commit c95f619 into mirage:master Nov 17, 2024
1 of 3 checks passed
@talex5 talex5 deleted the buf-write branch November 17, 2024 12:31
@talex5 talex5 mentioned this pull request Nov 17, 2024
talex5 added a commit to talex5/opam-repository that referenced this pull request Nov 30, 2024
CHANGES:

capnp-rpc 2.0 switches from using Lwt to Eio.
The `capnp-rpc-lwt` package is now just Lwt wrappers around the Eio functions in `capnp-rpc`.
This allows libraries using the old Lwt API to be used with applications and libraries using the new Eio one,
in the same binary.
Because Lwt libraries (using `capnp-rpc-lwt`) can be used with Eio applications (using `capnp-rpc-unix`),
you should first upgrade your application and then upgrade the libraries afterwards.

It is recommended to upgrade in stages, checking your application still works after each step:

1. Upgrade to the latest Lwt version (`opam install capnp-rpc-unix.1.2.4`).
   This uses the new version of mirage-crypto, tls, etc,
   which may be incompatible with other libraries you are using.
   Get that sorted out before switching to capnp-rpc 2.0.

2. Use [lwt_eio][] to allow using Eio and Lwt together in your application.
   This means replacing your call to `Lwt_main.run` with code to run Eio and Lwt together,
   as explained at the start of the `lwt_eio` README.

3. Upgrade your main application to capnp-rpc-unix 2.0.
   Calls to functions in the `Capnp_rpc_unix` and `Capnp_rpc_net` modules will need minor updates.
   They were previously called from Lwt context, so you'll need to wrap them with `Lwt_eio.run_eio`
   (or remove a `Lwt_eio.run_lwt` if you already updated the surrounding code).
   You should also use `mirage-crypto-rng-eio` to ensure randomness is available
   (`capnp-rpc-unix` no longer does this, although some other library might).

4. Upgrade code and libraries using `Capnp_rpc_lwt`:

   1. Replace `open Capnp_rpc_lwt` with `open Capnp_rpc.Std`.
   2. Replace all other uses of `Capnp_rpc_lwt` with just `Capnp_rpc`.
   3. In `dune` and `opam` files, replace `capnp-rpc-lwt` with `capnp-rpc`.
   4. Some modules are in `Capnp_rpc` but not the `Capnp_rpc.Std` subset.
      Those should now be fully qualified (e.g. replace `Persistence` with
      `Capnp_rpc.Persistence`).
   5. Replace `Service.return_lwt` with `Lwt_eio.run_lwt`.
      Replace `Lwt.return (Ok x)` with `Service.return x`.

Once all Lwt code is gone, you can remove the dependencies on `lwt` and `lwt_eio`.

New features:

- Allow numeric IPv6 listen addresses (@talex5 mirage/capnp-rpc#296, requested by @BChabanne).

API changes:

- Eio port (@talex5 mirage/capnp-rpc#280 mirage/capnp-rpc#284 mirage/capnp-rpc#298 mirage/capnp-rpc#292 mirage/capnp-rpc#297 mirage/capnp-rpc#300 mirage/capnp-rpc#304).

  This switches capnp-rpc from Lwt to Eio.
  One particularly nice side effect of this is that `Service.return_lwt` has gone,
  as there is no distinction now between concurrent and non-concurrent service methods.

  Uses of `Capnp_rpc_lwt` should be replaced by uses of `Capnp_rpc`
  (and the old `Capnp_rpc` is now an internal module, `Capnp_rpc_proto`).
  The new `Capnp_rpc` now provides `Error`, `Exception` and `Debug` aliases to the same modules in `Capnp_rpc_proto`,
  so that `Capnp_rpc_proto` doesn't need to be used directly.

  `Capnp_rpc_lwt` now provides (deprecated) compatibility wrappers, using `lwt_eio`.

  This also adds `Capnp_rpc.Std` with some common module aliases,
  to reduce the need to `open Capnp_rpc` (which is rather large).

- Deprecate `Debug.failf` (@talex5 mirage/capnp-rpc#291).

Performance and bug fixes:

- Add buffering of outgoing messages (@talex5 mirage/capnp-rpc#287 mirage/capnp-rpc#303).
  Sending each message in its own TCP packet isn't very efficient, and also interacts very badly with Nagle's algorithm.
  See <https://roscidus.com/blog/blog/2024/07/22/performance/> for details.

- Only disconnect socket when sending is done (@talex5 mirage/capnp-rpc#295).
  Allows sending the reason for the disconnection in some cases.

- Fix type of `Capability.call` (@talex5 mirage/capnp-rpc#293).
  It was previously unusable.

- Work around FreeBSD `close` problem (@talex5 mirage/capnp-rpc#302).

Build:

- Update to capnp 3.6.0 (@talex5 mirage/capnp-rpc#285).

- Update to dune 3.16 and fix warnings (@talex5 mirage/capnp-rpc#282).

- Use `String.starts_with` instead of `Astring` (@talex5 mirage/capnp-rpc#290).

- Remove unused `tls-mirage` dependency (@talex5 mirage/capnp-rpc#289).

- Remove `asetmap` dependency (@talex5 mirage/capnp-rpc#288).

[lwt_eio]: https://github.com/ocaml-multicore/lwt_eio
talex5 added a commit to talex5/opam-repository that referenced this pull request Nov 30, 2024
CHANGES:

capnp-rpc 2.0 switches from using Lwt to Eio.
The `capnp-rpc-lwt` package is now just Lwt wrappers around the Eio functions in `capnp-rpc`.
This allows libraries using the old Lwt API to be used with applications and libraries using the new Eio one,
in the same binary.
Because Lwt libraries (using `capnp-rpc-lwt`) can be used with Eio applications (using `capnp-rpc-unix`),
you should first upgrade your application and then upgrade the libraries afterwards.

It is recommended to upgrade in stages, checking your application still works after each step:

1. Upgrade to the latest Lwt version (`opam install capnp-rpc-unix.1.2.4`).
   This uses the new version of mirage-crypto, tls, etc,
   which may be incompatible with other libraries you are using.
   Get that sorted out before switching to capnp-rpc 2.0.

2. Use [lwt_eio][] to allow using Eio and Lwt together in your application.
   This means replacing your call to `Lwt_main.run` with code to run Eio and Lwt together,
   as explained at the start of the `lwt_eio` README.

3. Upgrade your main application to capnp-rpc-unix 2.0.
   Calls to functions in the `Capnp_rpc_unix` and `Capnp_rpc_net` modules will need minor updates.
   They were previously called from Lwt context, so you'll need to wrap them with `Lwt_eio.run_eio`
   (or remove a `Lwt_eio.run_lwt` if you already updated the surrounding code).
   You should also use `mirage-crypto-rng-eio` to ensure randomness is available
   (`capnp-rpc-unix` no longer does this, although some other library might).

4. Upgrade code and libraries using `Capnp_rpc_lwt`:

   1. Replace `open Capnp_rpc_lwt` with `open Capnp_rpc.Std`.
   2. Replace all other uses of `Capnp_rpc_lwt` with just `Capnp_rpc`.
   3. In `dune` and `opam` files, replace `capnp-rpc-lwt` with `capnp-rpc`.
   4. Some modules are in `Capnp_rpc` but not the `Capnp_rpc.Std` subset.
      Those should now be fully qualified (e.g. replace `Persistence` with
      `Capnp_rpc.Persistence`).
   5. Replace `Service.return_lwt` with `Lwt_eio.run_lwt`.
      Replace `Lwt.return (Ok x)` with `Service.return x`.

Once all Lwt code is gone, you can remove the dependencies on `lwt` and `lwt_eio`.

New features:

- Allow numeric IPv6 listen addresses (@talex5 mirage/capnp-rpc#296, requested by @BChabanne).

API changes:

- Eio port (@talex5 mirage/capnp-rpc#280 mirage/capnp-rpc#284 mirage/capnp-rpc#298 mirage/capnp-rpc#292 mirage/capnp-rpc#297 mirage/capnp-rpc#300 mirage/capnp-rpc#304).

  This switches capnp-rpc from Lwt to Eio.
  One particularly nice side effect of this is that `Service.return_lwt` has gone,
  as there is no distinction now between concurrent and non-concurrent service methods.

  Uses of `Capnp_rpc_lwt` should be replaced by uses of `Capnp_rpc`
  (and the old `Capnp_rpc` is now an internal module, `Capnp_rpc_proto`).
  The new `Capnp_rpc` now provides `Error`, `Exception` and `Debug` aliases to the same modules in `Capnp_rpc_proto`,
  so that `Capnp_rpc_proto` doesn't need to be used directly.

  `Capnp_rpc_lwt` now provides (deprecated) compatibility wrappers, using `lwt_eio`.

  This also adds `Capnp_rpc.Std` with some common module aliases,
  to reduce the need to `open Capnp_rpc` (which is rather large).

- Deprecate `Debug.failf` (@talex5 mirage/capnp-rpc#291).

Performance and bug fixes:

- Add buffering of outgoing messages (@talex5 mirage/capnp-rpc#287 mirage/capnp-rpc#303).
  Sending each message in its own TCP packet isn't very efficient, and also interacts very badly with Nagle's algorithm.
  See <https://roscidus.com/blog/blog/2024/07/22/performance/> for details.

- Only disconnect socket when sending is done (@talex5 mirage/capnp-rpc#295).
  Allows sending the reason for the disconnection in some cases.

- Fix type of `Capability.call` (@talex5 mirage/capnp-rpc#293).
  It was previously unusable.

- Work around FreeBSD `close` problem (@talex5 mirage/capnp-rpc#302).

Build:

- Update to capnp 3.6.0 (@talex5 mirage/capnp-rpc#285).

- Update to dune 3.16 and fix warnings (@talex5 mirage/capnp-rpc#282).

- Use `String.starts_with` instead of `Astring` (@talex5 mirage/capnp-rpc#290).

- Remove unused `tls-mirage` dependency (@talex5 mirage/capnp-rpc#289).

- Remove `asetmap` dependency (@talex5 mirage/capnp-rpc#288).

[lwt_eio]: https://github.com/ocaml-multicore/lwt_eio
talex5 added a commit to talex5/opam-repository that referenced this pull request Nov 30, 2024
CHANGES:

capnp-rpc 2.0 switches from using Lwt to Eio.
The `capnp-rpc-lwt` package is now just Lwt wrappers around the Eio functions in `capnp-rpc`.
This allows libraries using the old Lwt API to be used with applications and libraries using the new Eio one,
in the same binary.
Because Lwt libraries (using `capnp-rpc-lwt`) can be used with Eio applications (using `capnp-rpc-unix`),
you should first upgrade your application and then upgrade the libraries afterwards.

It is recommended to upgrade in stages, checking your application still works after each step:

1. Upgrade to the latest Lwt version (`opam install capnp-rpc-unix.1.2.4`).
   This uses the new version of mirage-crypto, tls, etc,
   which may be incompatible with other libraries you are using.
   Get that sorted out before switching to capnp-rpc 2.0.

2. Use [lwt_eio][] to allow using Eio and Lwt together in your application.
   This means replacing your call to `Lwt_main.run` with code to run Eio and Lwt together,
   as explained at the start of the `lwt_eio` README.

3. Upgrade your main application to capnp-rpc-unix 2.0.
   Calls to functions in the `Capnp_rpc_unix` and `Capnp_rpc_net` modules will need minor updates.
   They were previously called from Lwt context, so you'll need to wrap them with `Lwt_eio.run_eio`
   (or remove a `Lwt_eio.run_lwt` if you already updated the surrounding code).
   You should also use `mirage-crypto-rng-eio` to ensure randomness is available
   (`capnp-rpc-unix` no longer does this, although some other library might).

4. Upgrade code and libraries using `Capnp_rpc_lwt`:

   1. Replace `open Capnp_rpc_lwt` with `open Capnp_rpc.Std`.
   2. Replace all other uses of `Capnp_rpc_lwt` with just `Capnp_rpc`.
   3. In `dune` and `opam` files, replace `capnp-rpc-lwt` with `capnp-rpc`.
   4. Some modules are in `Capnp_rpc` but not the `Capnp_rpc.Std` subset.
      Those should now be fully qualified (e.g. replace `Persistence` with
      `Capnp_rpc.Persistence`).
   5. Replace `Service.return_lwt` with `Lwt_eio.run_lwt`.
      Replace `Lwt.return (Ok x)` with `Service.return x`.

Once all Lwt code is gone, you can remove the dependencies on `lwt` and `lwt_eio`.

New features:

- Allow numeric IPv6 listen addresses (@talex5 mirage/capnp-rpc#296, requested by @BChabanne).

API changes:

- Eio port (@talex5 mirage/capnp-rpc#280 mirage/capnp-rpc#284 mirage/capnp-rpc#298 mirage/capnp-rpc#292 mirage/capnp-rpc#297 mirage/capnp-rpc#300 mirage/capnp-rpc#304).

  This switches capnp-rpc from Lwt to Eio.
  One particularly nice side effect of this is that `Service.return_lwt` has gone,
  as there is no distinction now between concurrent and non-concurrent service methods.

  Uses of `Capnp_rpc_lwt` should be replaced by uses of `Capnp_rpc`
  (and the old `Capnp_rpc` is now an internal module, `Capnp_rpc_proto`).
  The new `Capnp_rpc` now provides `Error`, `Exception` and `Debug` aliases to the same modules in `Capnp_rpc_proto`,
  so that `Capnp_rpc_proto` doesn't need to be used directly.

  `Capnp_rpc_lwt` now provides (deprecated) compatibility wrappers, using `lwt_eio`.

  This also adds `Capnp_rpc.Std` with some common module aliases,
  to reduce the need to `open Capnp_rpc` (which is rather large).

- Deprecate `Debug.failf` (@talex5 mirage/capnp-rpc#291).

Performance and bug fixes:

- Add buffering of outgoing messages (@talex5 mirage/capnp-rpc#287 mirage/capnp-rpc#303).
  Sending each message in its own TCP packet isn't very efficient, and also interacts very badly with Nagle's algorithm.
  See <https://roscidus.com/blog/blog/2024/07/22/performance/> for details.

- Only disconnect socket when sending is done (@talex5 mirage/capnp-rpc#295).
  Allows sending the reason for the disconnection in some cases.

- Fix type of `Capability.call` (@talex5 mirage/capnp-rpc#293).
  It was previously unusable.

- Work around FreeBSD `close` problem (@talex5 mirage/capnp-rpc#302).

Build:

- Update to capnp 3.6.0 (@talex5 mirage/capnp-rpc#285).

- Update to dune 3.16 and fix warnings (@talex5 mirage/capnp-rpc#282).

- Use `String.starts_with` instead of `Astring` (@talex5 mirage/capnp-rpc#290).

- Remove unused `tls-mirage` dependency (@talex5 mirage/capnp-rpc#289).

- Remove `asetmap` dependency (@talex5 mirage/capnp-rpc#288).

[lwt_eio]: https://github.com/ocaml-multicore/lwt_eio
talex5 added a commit to talex5/opam-repository that referenced this pull request Dec 1, 2024
CHANGES:

capnp-rpc 2.0 switches from using Lwt to Eio.
The `capnp-rpc-lwt` package is now just Lwt wrappers around the Eio functions in `capnp-rpc`.
This allows libraries using the old Lwt API to be used with applications and libraries using the new Eio one,
in the same binary.
Because Lwt libraries (using `capnp-rpc-lwt`) can be used with Eio applications (using `capnp-rpc-unix`),
you should first upgrade your application and then upgrade the libraries afterwards.

It is recommended to upgrade in stages, checking your application still works after each step:

1. Upgrade to the latest Lwt version (`opam install capnp-rpc-unix.1.2.4`).
   This uses the new version of mirage-crypto, tls, etc,
   which may be incompatible with other libraries you are using.
   Get that sorted out before switching to capnp-rpc 2.0.

2. Use [lwt_eio][] to allow using Eio and Lwt together in your application.
   This means replacing your call to `Lwt_main.run` with code to run Eio and Lwt together,
   as explained at the start of the `lwt_eio` README.

3. Upgrade your main application to capnp-rpc-unix 2.0.
   Calls to functions in the `Capnp_rpc_unix` and `Capnp_rpc_net` modules will need minor updates.
   They were previously called from Lwt context, so you'll need to wrap them with `Lwt_eio.run_eio`
   (or remove a `Lwt_eio.run_lwt` if you already updated the surrounding code).
   You should also use `mirage-crypto-rng-eio` to ensure randomness is available
   (`capnp-rpc-unix` no longer does this, although some other library might).

4. Upgrade code and libraries using `Capnp_rpc_lwt`:

   1. Replace `open Capnp_rpc_lwt` with `open Capnp_rpc.Std`.
   2. Replace all other uses of `Capnp_rpc_lwt` with just `Capnp_rpc`.
   3. In `dune` and `opam` files, replace `capnp-rpc-lwt` with `capnp-rpc`.
   4. Some modules are in `Capnp_rpc` but not the `Capnp_rpc.Std` subset.
      Those should now be fully qualified (e.g. replace `Persistence` with
      `Capnp_rpc.Persistence`).
   5. Replace `Service.return_lwt` with `Lwt_eio.run_lwt`.
      Replace `Lwt.return (Ok x)` with `Service.return x`.

Once all Lwt code is gone, you can remove the dependencies on `lwt` and `lwt_eio`.

New features:

- Allow numeric IPv6 listen addresses (@talex5 mirage/capnp-rpc#296, requested by @BChabanne).

API changes:

- Eio port (@talex5 mirage/capnp-rpc#280 mirage/capnp-rpc#284 mirage/capnp-rpc#298 mirage/capnp-rpc#292 mirage/capnp-rpc#297 mirage/capnp-rpc#300 mirage/capnp-rpc#304).

  This switches capnp-rpc from Lwt to Eio.
  One particularly nice side effect of this is that `Service.return_lwt` has gone,
  as there is no distinction now between concurrent and non-concurrent service methods.

  Uses of `Capnp_rpc_lwt` should be replaced by uses of `Capnp_rpc`
  (and the old `Capnp_rpc` is now an internal module, `Capnp_rpc_proto`).
  The new `Capnp_rpc` now provides `Error`, `Exception` and `Debug` aliases to the same modules in `Capnp_rpc_proto`,
  so that `Capnp_rpc_proto` doesn't need to be used directly.

  `Capnp_rpc_lwt` now provides (deprecated) compatibility wrappers, using `lwt_eio`.

  This also adds `Capnp_rpc.Std` with some common module aliases,
  to reduce the need to `open Capnp_rpc` (which is rather large).

- Deprecate `Debug.failf` (@talex5 mirage/capnp-rpc#291).

Performance and bug fixes:

- Add buffering of outgoing messages (@talex5 mirage/capnp-rpc#287 mirage/capnp-rpc#303).
  Sending each message in its own TCP packet isn't very efficient, and also interacts very badly with Nagle's algorithm.
  See <https://roscidus.com/blog/blog/2024/07/22/performance/> for details.

- Only disconnect socket when sending is done (@talex5 mirage/capnp-rpc#295).
  Allows sending the reason for the disconnection in some cases.

- Fix type of `Capability.call` (@talex5 mirage/capnp-rpc#293).
  It was previously unusable.

- Work around FreeBSD `close` problem (@talex5 mirage/capnp-rpc#302).

Build:

- Update to capnp 3.6.0 (@talex5 mirage/capnp-rpc#285).

- Update to dune 3.16 and fix warnings (@talex5 mirage/capnp-rpc#282).

- Use `String.starts_with` instead of `Astring` (@talex5 mirage/capnp-rpc#290).

- Remove unused `tls-mirage` dependency (@talex5 mirage/capnp-rpc#289).

- Remove `asetmap` dependency (@talex5 mirage/capnp-rpc#288).

[lwt_eio]: https://github.com/ocaml-multicore/lwt_eio
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant