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

parallel_find : ... -> (unit -> 'a option) -> 'a option #90

Merged
merged 1 commit into from
May 23, 2023

Conversation

gasche
Copy link
Contributor

@gasche gasche commented Oct 15, 2022

This is proposed as a solution for #89.

@gasche
Copy link
Contributor Author

gasche commented Oct 15, 2022

The CI failures comes from my use of Domain.recommended_domain_count () in my test. The CI uses 5.0~alpha, and I guess this function was not available then? Updating the CI to the recent beta release should fix the issue. (I could also fix the domain number.) In the meantime I think it should be possible to review the code anyway -- the test does pass on my machine.

Copy link
Contributor

@Sudha247 Sudha247 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks! parallel_find is certainly useful to have. The code looks good to me, and the docs capture the expected behaviour well.

You're right about the CI, updating to beta should fix the failure -- I was planning on doing it soon, that shouldn't block this PR.

test/test_parallel_find.ml Outdated Show resolved Hide resolved
Suggested-by: Christophe Raffali <[email protected]>
Reviewed-by: Sudha Parimala <[email protected]>
@Sudha247
Copy link
Contributor

Sudha247 commented Oct 31, 2022

I propose we merge this and #91 before #92 is worked on, if there are no objections.

@ElectreAAS
Copy link

As a user of domainslib (who doesn't know anything about the internals and has no opinion on #91 & #92), I can attest that this is a wanted feature!

@Sudha247
Copy link
Contributor

Thanks for sharing @ElectreAAS. While we may want to add a full fledged cancellation mechanism (for e.g. like the one in Eio) at a later point, this is a useful feature to have at present. The CI failure looks unrelated, and the branch works fine on my local setup. Merging.

@Sudha247 Sudha247 merged commit 3d981ce into ocaml-multicore:master May 23, 2023
Sudha247 added a commit to Sudha247/opam-repository that referenced this pull request Jul 18, 2023
CHANGES:

* Add parallel_find (ocaml-multicore/domainslib#90, @gasche)
* Update CI (ocaml-multicore/domainslib#93, @Sudha247)
* Optimisation to work-stealing (ocaml-multicore/domainslib#96, @art-w)
* Improve docs presentation (ocaml-multicore/domainslib#99, @metanivek)
* Property based tests (ocaml-multicore/domainslib#100, jmid)
* Task: avoid double handler installation (ocaml-multicore/domainslib#101, @gasche & @clef-men)
* Fix a benign data-race in Chan reported by ocaml-tsan (ocaml-multicore/domainslib#103, @art-w)
* Dune, opam, and GitHub Actions fixes (ocaml-multicore/domainslib#105, @MisterDA)
* domain local await support (ocaml-multicore/domainslib#107, @polytypic)
* Windows run on GitHub Actions (ocaml-multicore/domainslib#110, @Sudha247)
* Adjust PBTs based on recommended_domain_count (ocaml-multicore/domainslib#112, @jmid)
* Test condition tweaks (ocaml-multicore/domainslib#113, @jmid)
nberth pushed a commit to nberth/opam-repository that referenced this pull request Jun 18, 2024
CHANGES:

* Add parallel_find (ocaml-multicore/domainslib#90, @gasche)
* Update CI (ocaml-multicore/domainslib#93, @Sudha247)
* Optimisation to work-stealing (ocaml-multicore/domainslib#96, @art-w)
* Improve docs presentation (ocaml-multicore/domainslib#99, @metanivek)
* Property based tests (ocaml-multicore/domainslib#100, jmid)
* Task: avoid double handler installation (ocaml-multicore/domainslib#101, @gasche & @clef-men)
* Fix a benign data-race in Chan reported by ocaml-tsan (ocaml-multicore/domainslib#103, @art-w)
* Dune, opam, and GitHub Actions fixes (ocaml-multicore/domainslib#105, @MisterDA)
* domain local await support (ocaml-multicore/domainslib#107, @polytypic)
* Windows run on GitHub Actions (ocaml-multicore/domainslib#110, @Sudha247)
* Adjust PBTs based on recommended_domain_count (ocaml-multicore/domainslib#112, @jmid)
* Test condition tweaks (ocaml-multicore/domainslib#113, @jmid)
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