Add alternate implementations of synchronization primitives

smol-rs / concurrent-queue

Concurrent multi-producer multi-consumer queue

Apache License 2.0

254 stars 21 forks source link

Add alternate implementations of synchronization primitives #27

Closed notgull closed 1 year ago

notgull commented 1 year ago

This PR adds two alternate implementations of synchronization primitives:

One based on loom. This can be enabled using cfg(loom). Unfortunately there is no real way to test this, since loom tests require some kind of synchronization primitive to make progress.
One based on portable-atomic, enabled using the portable-atomic feature. This allows concurrent-queue to be used on platforms without available atomics.

Obsoletes #5

taiki-e commented 1 year ago

Thanks! Could you add tests for cfg(loom)? When we did something similar before in crossbeam, unbounded queue panicked with odd errors like https://github.com/tokio-rs/loom/issues/283.

notgull commented 1 year ago

I added a basic spsc test for Loom, but I keep bumping my head on this panic:

thread 'spsc' panicked at 'Model exceeded maximum number of branches. This is often caused by an algorithm requiring the processor to make progress, e.g. spin locks.', /home/jtnunley/.cargo/registry/src/github.com-1ecc6299db9ec823/loom-0.5.6/src/rt/path.rs:136:9

I'm not that familiar with loom, but is there a way around this? If I have the receiver thread wait on a Condvar, it complains about the thread being deadlocked and panics.

notgull commented 1 year ago

With some tinkering, I built a channel implementation with loom primitives based on concurrent_queue, specifically to use in this test. It looks like it works well enough to be tested.

notgull commented 1 year ago

Thank you for taking the time to review, @taiki-e! I implemented the suggestions you made.

The only real issue here is that running the CI now takes significantly longer. Even with LOOM_MAX_PREEMPTIONS=2, it looks like it still takes a few minutes to run. It's not really that big of a deal, and we could probably even get away with using LOOM_MAX_PREEMPTIONS=3 if we really wanted to.