Skip to content

Conversation

@michaelborkowski
Copy link
Owner

@michaelborkowski michaelborkowski commented Mar 24, 2025

It turns out that using Control.Monad.Par to implement (.||.) resolves the race conditions without being obviously slower than the par/pseq/deepseq version.

I've also introduced a 4-ary parallelism operator, but the gap between array size and SEQSIZE has to be huge for that to make a difference.

This would fix #14.

@ulysses4ever
Copy link
Collaborator

Terrific! I didn’t realize that it doesn’t matter for verification what we do to implement the parallel pair operator, so switching to anything that works (like monad-par here) is a no-brainer.

I’m doubtful about the ternary operator. Normally, if green threads are implemented nicely, I’d not expect any difference between this and iterated parallel pair. So, if it only makes any difference under synthetic and exaggerated conditions, I’d probably stick to pairs. I can’t quite judge whether these conditions are abnormal as I haven’t studied benchmarking parallel versions yet. So I leave it for you to decide.

lastly, you mentioned in slack that single-threaded is still faster? This is pretty sad. If this is the case, we need a ticket for it and a volunteer to dig into it. But I’d appreciate at least a ticket with some quick numbers.

CI failed for lists because of missing NFData constraints. It’s good to have CI isn’t it!?.. :-)

@ulysses4ever
Copy link
Collaborator

🎉 🎉 🎉

@michaelborkowski michaelborkowski merged commit 1ef22da into main Apr 1, 2025
5 checks passed
@michaelborkowski michaelborkowski deleted the race-conditions branch April 1, 2025 19:17
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.

Fix incorrectness when running the merge-based sorts with multiple threads

3 participants