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

Inefficient Lwt_io.NumberIO implementations (int32, int64, float read/write). #178

Closed
mfp opened this issue Aug 13, 2015 · 2 comments
Closed
Milestone

Comments

@mfp
Copy link
Collaborator

mfp commented Aug 13, 2015

4.01 introduced primitives for reading/writing such values efficiently.
ocplib-endian builds on them while providing fallback code for older compilers. Lwt_io could easily build on it, getting rid of ~100 LoCs and improving speed.

@mfp mfp changed the title Inefficient Lwt_io.NumberIO (int32, int64, float read/write). Inefficient Lwt_io.NumberIO implementations (int32, int64, float read/write). Aug 13, 2015
@aantron
Copy link
Collaborator

aantron commented May 31, 2017

This issue is about basically throwing away the current NumberIO code and replacing it with an implementation based on the above-mentioned primitives.

Lwt now requires 4.02.0 (soon 4.02.3), so we don't have to worry about adding compatibility fallbacks.

For each reading function, we want to wait until the channel buffer has enough bytes for the kind of number we are trying to read. We then want to apply the fast primitive to extract that number.

For writing, we want to apply a primitive to write a number to the buffer, ideally without copying (not converting it into an intermediate buffer first).

The Lwt_io.NumberIO signature and implementations are found here [1] [2] [3] in the source.

Here's an example, from the compiler's test suite, of how to bind the bytes/number conversion primitives: https://github.com/ocaml/ocaml/blob/70d880a41a82aae1ebd428fd38100e8467f8535a/testsuite/tests/prim-bigstring/bigstring_access.ml#L5-L17

The same, for byte swaps: https://github.com/ocaml/ocaml/blob/70d880a41a82aae1ebd428fd38100e8467f8535a/testsuite/tests/prim-bswap/bswap.ml#L3-L5

And a link to ocplib-endian. I used the source of this library to help find the names of the primitives, etc.

@aantron aantron added the medium label May 31, 2017
@aantron
Copy link
Collaborator

aantron commented May 31, 2017

This is a pretty self-contained and relatively straightforward issue, but I marked it medium because it involves dealing with:

  • endianness,
  • the internals of Lwt_io channels,
  • binding compiler primitives.

Ideally, one would also write a performance comparison (using Unix.times, for example) to show how much faster the new code is, compared to the old one. One would also write tests for the NumberIO API.

@aantron aantron added this to the 4.2.2 milestone Aug 3, 2019
aantron added a commit that referenced this issue Aug 6, 2019
aantron added a commit that referenced this issue Aug 6, 2019
@aantron aantron closed this as completed in e0fa5c7 Aug 6, 2019
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

No branches or pull requests

2 participants