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

Remove use of notty for formatting benchmark results #71

Merged
merged 1 commit into from
Aug 25, 2022

Conversation

talex5
Copy link
Collaborator

@talex5 talex5 commented Aug 24, 2022

The dependency on notty prevents uring from being tested on OCaml 5.

Old output:

╭────────────────┬───────────────────────────┬───────────────────────────┬───────────────────────────╮
│name            │  major-allocated          │  minor-allocated          │  monotonic-clock          │
├────────────────┼───────────────────────────┼───────────────────────────┼───────────────────────────┤
│  noop      10  │             0.0000 mjw/run│           160.2533 mnw/run│           1003.7415 ns/run│
│  noop      30  │             0.0000 mjw/run│           480.3344 mnw/run│           2687.7522 ns/run│
│  noop     100  │             0.0930 mjw/run│          1600.4478 mnw/run│           9072.9584 ns/run│
│  noop     300  │            12.2791 mjw/run│          4800.6122 mnw/run│          27520.1296 ns/run│
│  noop    1000  │           114.9145 mjw/run│         16000.9554 mnw/run│          98416.3581 ns/run│
│  noop    3000  │          1030.7835 mjw/run│         48001.5152 mnw/run│         298576.3733 ns/run│
│  noop   10000  │         13413.3058 mjw/run│        160002.6316 mnw/run│        1078551.5092 ns/run│
╰────────────────┴───────────────────────────┴───────────────────────────┴───────────────────────────╯

New output:

          name,   major-allocated,   minor-allocated,   monotonic-clock
  noop      10,          0.003163,         89.035794,       1122.076257
  noop      30,          0.010071,        272.687218,       2796.151085
  noop     100,          0.914579,        978.710714,       9079.485427
  noop     300,         11.478316,       3511.237206,      28500.701765
  noop    1000,        115.853532,      13936.387169,     104048.680888
  noop    3000,       1033.692225,      44261.436753,     308647.065684
  noop   10000,      12420.745528,     153296.414960,    1216105.351957

The units have gone, but on the plus side you can now graph it as CSV data.

The results do seem to be slightly different, though, which is odd. Hmm. Restoring the List.iter (fun v -> Bechamel_notty.Unit.add v (Measure.unit v)) metrics; line affects things:

          name,   major-allocated,   minor-allocated,   monotonic-clock
  noop      10,          0.000000,        160.284696,       1124.221385
  noop      30,          0.000000,        480.360509,       2997.389849
  noop     100,          0.000000,       1600.467290,       9834.411904
  noop     300,         12.125976,       4800.649351,      30976.050309
  noop    1000,        112.719320,      16001.034483,     115284.222115
  noop    3000,       1022.773418,      48001.612903,     339099.821312
  noop   10000,      13348.487663,     160002.830189,    1201494.350105

@dinosaure: is this the right way to do this?

@talex5 talex5 force-pushed the nonotty branch 2 times, most recently from 8654596 to 06dcedb Compare August 24, 2022 14:52
@talex5
Copy link
Collaborator Author

talex5 commented Aug 24, 2022

A couple of the new test platforms fail with:

 | /tmp/build_200d78_dune/ocaml-configuratorf12775/c-test-0/test.c:5:10: fatal error: linux/time_types.h: No such file or directory
 |  #include <linux/time_types.h>
 |           ^~~~~~~~~~~~~~~~~~~~
 | compilation terminated.
Error: failed to compile program

I think we can remove this once #70 is merged, since liburing.h pulls it in correctly (cc @bikallem).

Also, it appears that #37 is fixed now!

@bikallem
Copy link
Contributor

bikallem commented Aug 24, 2022

I think we can remove this once #70 is merged, since liburing.h pulls it in correctly

Yes, "liburing.h" defines __kernel_timespec if it is not found. https://github.com/axboe/liburing/blob/master/configure#L446-L453

I originally intended to just use "liburing.h" with dune configurator but wasn't successful. So #70 should make "<linux/time_types.h>" redundant and can be removed.

EDIT: I have added some comments on the #70 itself.

@talex5
Copy link
Collaborator Author

talex5 commented Aug 25, 2022

OK, I merged #70 and rebased on that, which has fixed the errors.

It prevents uring from being tested on OCaml 5.
@talex5 talex5 merged commit c302549 into ocaml-multicore:main Aug 25, 2022
@talex5 talex5 deleted the nonotty branch August 25, 2022 10:02
talex5 added a commit to talex5/opam-repository that referenced this pull request Aug 26, 2022
CHANGES:

New features:

- Add `Uring.timeout` (@bikallem ocaml-multicore/ocaml-uring#59).

- Add `Uring.read` and `Uring.write` (@haesbaert ocaml-multicore/ocaml-uring#62).
  These are simple wrappers for read(2) and write(2).

- Add `Uring.unlink` (@talex5 ocaml-multicore/ocaml-uring#65).
  This uses unlinkat(2), and so can also be used to remove directories.

- Add support for uring probes (@talex5 ocaml-multicore/ocaml-uring#70).
  Allows checking whether a feature is supported by the kernel at runtime.

- Rename `peek` to `get_cqe_nonblocking` (@talex5 ocaml-multicore/ocaml-uring#67).
  The old name was confusing because it does remove the item from the ring.

- Update to liburing 2.2 (@talex5 ocaml-multicore/ocaml-uring#56).

- Add `Uring.active_ops` (@talex5 ocaml-multicore/ocaml-uring#68).
  Avoids needing to track the value returned by `submit`, which is important as it is sometimes called automatically.

- Add `Uring.iov_max` constant (@talex5 ocaml-multicore/ocaml-uring#76).

- Add `Uring.get_debug_stats` (@talex5 ocaml-multicore/ocaml-uring#64).
  This should make it easier to check that the uring is behaving as expected.

Performance:

- Introduce a Sketch buffer per Uring (@haesbaert ocaml-multicore/ocaml-uring#63).
  The main motivation of this change is to avoid having one malloc per packet in readv(2), writev(2) and friends.

- Use `submit_and_wait` where appropriate (@haesbaert ocaml-multicore/ocaml-uring#69).

- Add a `readv` benchmark (@talex5 ocaml-multicore/ocaml-uring#64).

- Avoid unnecessary use of `CAMLparam` in the C stubs (@haesbaert ocaml-multicore/ocaml-uring#61).

Bug fixes:

- Prevent ring from being used after exit (@talex5 ocaml-multicore/ocaml-uring#78).

Build changes:

- Remove use of notty for formatting benchmark results (@talex5 ocaml-multicore/ocaml-uring#71).
  It prevented uring from being tested on OCaml 5.

- Use MDX for README (@talex5 ocaml-multicore/ocaml-uring#57).

- Convert tests to MDX (@talex5 ocaml-multicore/ocaml-uring#58 ocaml-multicore/ocaml-uring#73).

- Use opam-repository syntax for license (@kit-ty-kate ocaml-multicore/ocaml-uring#72).

- Remove internal `is_dirty` flag (@talex5 ocaml-multicore/ocaml-uring#77).
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