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

Reimplementation of the library #56

Draft
wants to merge 18 commits into
base: main
Choose a base branch
from
Draft

Reimplementation of the library #56

wants to merge 18 commits into from

Conversation

wokalski
Copy link
Contributor

This PR contains a reimplementation of the library. Only the eio variant works for now. The goal of the reimplementation is to make it more:

  1. modular
  2. performant
  3. ergonomic

Building an opinionated workflow is not a part of this library, I will implement it in a separate library.

TODO

  • Refactor the lwt variant
  • Refactor the async variant
  • Depart from exposing ('a -> string) and (string -> 'a) (de)serializers and expose Bigstringaf directly to avoid copying. string variants can be put on top for those who need them simply in the userspace. The string variants impede the performance due to excessive copying.
  • Slight API modifications to reduce redundancy (headers probably don't need to contain the extra part for example)
  • Introduce 'ctx to server request handlers. Not sure if it's needed - maybe can be done in the user space, maybe has to be a part of the library - will experiment with it.

quernd and others added 8 commits April 5, 2024 11:15
Using lazy sequences by default is too opinionated. They incur a lot of overhead and are tricky to manage.
Rather, we can expose a mechanism similar to `ocaml-h2` that uses a `schedule_read` function expecting two callbacks.
When multiple messages are batched and read into the buffer, only the
first message was extracted. Remaining messages would not be extracted
until the next invocation of `on_read`.
@tmcgilchrist
Copy link
Collaborator

tmcgilchrist commented Apr 16, 2024

@wokalski if you use the OCaml 5.3 beta OCaml 5.3 trunk (opam switch create 5.3.0+trunk) with janestreet/memtrace#22 then you can do memory profiling with Eio.

Edit, got confused with my compiler versions.

@wokalski
Copy link
Contributor Author

statmemprof ftw!!!

@wokalski
Copy link
Contributor Author

@tmcgilchrist will statmemprof be available in 5.2 or only in 5.3?

@tmcgilchrist
Copy link
Collaborator

Only in 5.3. Based on this libraries dependencies it should be possible to build against that version.
The problematic pieces are usually PPX and merlin/lsp.

@wokalski
Copy link
Contributor Author

wokalski commented Oct 2, 2024

just a little update - we are using this on production and we will probably push the final commits to this soon.

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.

3 participants