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

Factor out (the deprecated) Lwt_pqueue #360

Closed
aantron opened this issue May 18, 2017 · 5 comments
Closed

Factor out (the deprecated) Lwt_pqueue #360

aantron opened this issue May 18, 2017 · 5 comments

Comments

@aantron
Copy link
Collaborator

aantron commented May 18, 2017

Lwt_pqueue (.mli, .ml) is a pretty ordinary priority queue, implemented as min-heap, that Lwt uses internally.

Since Lwt is not a basic data structures library, Lwt would much rather rely on a priority queue implementation in the standard library, or some data structures library that we choose. Unfortunately, priority queues are not included in the standard library.

This issue is for tracking Lwt_pqueue, lobbying for priority queues in the standard library, and any other related activity or discussion.

Despite the fact that Lwt_pqueue is an implementation detail of Lwt, it is accessible when linking with Lwt. There is some code that takes advantage of its existence, which is why Lwt deprecated Lwt_pqueue in c86cba3. We don't plan to actually remove it in the near future, to avoid breaking code that still uses it. We just want to switch to a common implementation.

The technical side of this issue is easy, but it is organizationally difficult to get this arranged.

@c-cube
Copy link
Collaborator

c-cube commented May 18, 2017

The module is pretty simple, though. How much of a technical debt is it? Lobbying for heaps in the stdlib would take a while anyway, and only work on the next version of OCaml…

@aantron
Copy link
Collaborator Author

aantron commented May 18, 2017

How much of a technical debt is it?

It's not a big technical debt. Still, I think it's a good idea to permanently note that priority queues belong elsewhere (ideally in stdlib), and Lwt was apparently forced to implement its own version of them.

Other projects that currently refer to Lwt_pqueue also would have benefitted from a priority queue in the stdib. These projects might have an easier time deciding to use an extra data structures library, however.

This is an issue for the long term. If there are related discussions elsewhere about this, this issue provides people with something to link to. Maybe it will stimulate or remind people that this ought to eventually be done.

Lobbying for heaps in the stdlib would take a while anyway, and only work on the next version of OCaml…

True, but eventually the minimum version of Lwt might catch up to that version of OCaml, and we can switch. There may be shims available. Lwt currently uses the result shim, admittedly a simple one, which is only necessary while Lwt continues to support 4.02 (which I think we intend to do for a while).

IMO the sooner stdlib gets a more thorough set of basic data structures, the better.

@aantron
Copy link
Collaborator Author

aantron commented May 18, 2017

Also, #135 ("Lwt_pqueue not documented") is the previous issue on Lwt_pqueue.

@c-cube
Copy link
Collaborator

c-cube commented May 18, 2017

I agree on adding more of these useful structures to the stdlib, but I'm tired of trying and a bit pessimistic. Nice to hear that you want to continue supporting 4.02 :-)

@aantron aantron added this to the 3.2.0 milestone Nov 7, 2017
@aantron aantron removed this from the 3.2.0 milestone Dec 20, 2017
aantron added a commit that referenced this issue Aug 3, 2019
@aantron
Copy link
Collaborator Author

aantron commented Aug 3, 2019

Closing this, as we've made it an implementation detail, deprecated using it, and can factor it out later as needed.

Also found psq.

@aantron aantron closed this as completed Aug 3, 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