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 echo benchmark test #197

Merged
merged 5 commits into from
May 12, 2020
Merged

Add echo benchmark test #197

merged 5 commits into from
May 12, 2020

Conversation

Cjen1
Copy link
Contributor

@Cjen1 Cjen1 commented May 8, 2020

The PR adds a small benchmark for one common use case: accessing a capability over the network (in this case via localhost).

Things to note is that it does not separate the client and server into two separate processes which does impact the performance (since this is a CPU limited benchmark).

Copy link
Contributor

@talex5 talex5 left a comment

Choose a reason for hiding this comment

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

Fails to build in CI, but looks good once that's fixed.

I suggest also adding an empty .mli file, so OCaml can detect unused variables.

Thanks!

test-bin/echo/echo_bench.ml Outdated Show resolved Hide resolved
test-bin/echo/echo_bench.ml Outdated Show resolved Hide resolved
@Cjen1 Cjen1 force-pushed the benchmark branch 2 times, most recently from 428e910 to 3bffb89 Compare May 9, 2020 16:28
@Cjen1
Copy link
Contributor Author

Cjen1 commented May 10, 2020

@talex5 the ocaml-ci issue should have been fixed via #198. If the CI can be restarted? I think the other PR is having the same issue.

Copy link
Contributor

@talex5 talex5 left a comment

Choose a reason for hiding this comment

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

It just needs to be rebased onto master to check it against that.

BTW, I added a couple of suggested changes to make sure the result is correct (looked like it was being ignored before).

I suspect you may get a small speed increase by setting the initial message sizes to sensible values instead of the default, but I don't think we expose that in the API at the moment.

e.g. in capnp-rpc-lwt/request.ml try Message.init_root ~message_size:60 ()

test-bin/echo/echo_bench.ml Outdated Show resolved Hide resolved
test-bin/echo/echo_bench.ml Outdated Show resolved Hide resolved
@talex5 talex5 merged commit baf1bdd into mirage:master May 12, 2020
talex5 added a commit to talex5/opam-repository that referenced this pull request May 14, 2020
…pc and capnp-rpc-unix (0.7.0)

CHANGES:

- Update for x509 0.11.0 API changes (@talex5, mirage/capnp-rpc#196).

- Update to new mirage network API (@Cjen1, mirage/capnp-rpc#198).

- Add echo benchmark test (@Cjen1, mirage/capnp-rpc#197).

- Estimate message sizes to improve performance (@talex5, mirage/capnp-rpc#200).
  By default, capnproto allocates 8k buffers, but most capnp-rpc messages are
  much smaller than this.

Logging:

- Fix application logging, to use library's log (@Cjen1, mirage/capnp-rpc#195).

- Expose the endpoint logger (@Cjen1, mirage/capnp-rpc#195).

- Only enable debug logging for capnp libraries in the calc example.
  TLS now generates a lot of messages at debug level (@talex5, mirage/capnp-rpc#200).
@Cjen1 Cjen1 deleted the benchmark branch May 14, 2020 18:23
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.

2 participants