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

Switch to a proper scheduler #550

Closed
4 tasks done
rgrinberg opened this issue Nov 21, 2021 · 3 comments
Closed
4 tasks done

Switch to a proper scheduler #550

rgrinberg opened this issue Nov 21, 2021 · 3 comments
Labels
enhancement New feature or request
Milestone

Comments

@rgrinberg
Copy link
Member

rgrinberg commented Nov 21, 2021

The current situation suffers from poor performance and consumes too much resources. I'm planning to get rid of it with libev bindings I've developed: https://github.com/rgrinberg/lev.

This work has to be done in steps:

  • Update to the latest Fiber api in dune.
  • Implement a general fiber based scheduler for Lev.
  • Replace the current home made scheduler with it.
  • Test on windows

Once this is done, we can even consider removing the thread used for merlin. I'm not 100% convinced it buys us something at this point.

@ulugbekna
Copy link
Collaborator

Couldn't we switch to lwt? It would lower the entry barrier by far imho. We've had several scheduler-related problems, and the current scheduler is a dreadful piece of code with signals, threads, etc. again imho.

@rgrinberg
Copy link
Member Author

rgrinberg commented Nov 22, 2021

How would it lower the barrier? Fiber is far simpler than Lwt (and faster, and with much better error handling).

We've had plenty of scheduler related problems because we've build the scheduler wrong. Fortunately, this final approach should fix the issue for good by building on top of reliable primitives. Also, dune itself suffers from the same problems so if the libev based scheduler performs well, we'll be able to use it in dune too. For dune, Lwt is definitely out of question so I wouldn't save myself any time by avoiding this work here in lsp.

@rgrinberg
Copy link
Member Author

Once this is done, we can even consider removing the thread used for merlin. I'm not 100% convinced it buys us something at this point.

This is unfortunately impossible as merlin spawns ppx processes which can be quite slow. It will have to continue running in a separate thread.

But otherwise, I've completed the ported and it seems to be working quite well.

@rgrinberg rgrinberg added this to the 1.10.0 milestone Feb 1, 2022
@rgrinberg rgrinberg added the enhancement New feature or request label Feb 1, 2022
rgrinberg added a commit to rgrinberg/opam-repository that referenced this issue Feb 10, 2022
CHANGES:

## Features

- Add better support for code folding: more folds and more precise folds

## Fixes

- Fix infer interface code action crash when implementation source does not
  exist (ocaml/ocaml-lsp#597)

- Improve error message when the reason plugin for merlin is absent (ocaml/ocaml-lsp#608)

- Fix `chdir` races when running ppx (ocaml/ocaml-lsp#550)

- More accurate completion kinds.
  New completion kinds for variants and fields. Removed inaccurate completion
  kinds for constructors and types. (ocaml/ocaml-lsp#510)

- Fix handling request cancellation (ocaml/ocaml-lsp#616)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants