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

Semaphores and condition variables. #10503

Merged

Conversation

Apprentice-Alchemist
Copy link
Contributor

@Apprentice-Alchemist Apprentice-Alchemist commented Nov 25, 2021

Since neko is deprecated I have not implemented these for neko.

@RealyUniqueName
Copy link
Member

Nice!
Pls, fix building so we can check CI.

opam Outdated Show resolved Hide resolved
@Apprentice-Alchemist
Copy link
Contributor Author

Please don't do this, there are some implications related to packaging that we would like to avoid.

Changes to the ocaml version have been reverted.
Using luv to implement semaphores and condition variables seems to work.

@RealyUniqueName
Copy link
Member

I think luv's synchronization primitives require explicit calls to destroy methods for cleanups.

@RealyUniqueName RealyUniqueName added this to the Release 4.3 milestone Nov 29, 2021
@ncannasse
Copy link
Member

I'm curious about the efficiency of these VS implementing them on top of Lock/Mutex :)

@Apprentice-Alchemist
Copy link
Contributor Author

Lock is already implemented using a semaphore on many targets.
So to implement a semaphore, you'd end up using a semaphore + an extra counter + a mutex to protect that counter.
That sounds less efficient that using a semaphore directly.

@skial skial mentioned this pull request Dec 1, 2021
1 task
@Simn
Copy link
Member

Simn commented Jan 8, 2022

Sorry for the delay! I've merged the hxcpp PR, could you update this one so CI runs?

Edit: Uhm, wrong PR, sorry!

@Simn
Copy link
Member

Simn commented Feb 14, 2022

Now my last comment is accurate, so could you push again? I still don't know why I can't restart CI on this one.

Apprentice-Alchemist added a commit to Apprentice-Alchemist/hxcpp that referenced this pull request Feb 14, 2022
Can still be enabled with `HXCPP_WINXP_COMPAT`.
Required for HaxeFoundation/haxe#10503.
@Apprentice-Alchemist
Copy link
Contributor Author

windows64-test (cpp) failure: Turns out hxcpp already sets the HXCPP_WINXP_COMPAT by default.
Simplest fix (in my opinion) is to turn that off: HaxeFoundation/hxcpp#984
windows-build failure: no idea, it's not showing any logs.

Simn pushed a commit to HaxeFoundation/hxcpp that referenced this pull request Feb 14, 2022
Can still be enabled with `HXCPP_WINXP_COMPAT`.
Required for HaxeFoundation/haxe#10503.
@Simn
Copy link
Member

Simn commented Feb 14, 2022

Had to restart CI again because CI / linux-arm64 (pull_request) was hanging. Let's see if this finally turns green now...

@Simn
Copy link
Member

Simn commented Feb 14, 2022

Whatever, I'll just merge this!

@Simn Simn merged commit bc6fb5c into HaxeFoundation:development Feb 14, 2022
@skial skial mentioned this pull request Feb 16, 2022
1 task
@Simn Simn mentioned this pull request Apr 4, 2022
@Apprentice-Alchemist Apprentice-Alchemist deleted the feature/semaphore-cond-var branch October 21, 2022 19:48
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.

5 participants