smol-rs / concurrent-queue

Concurrent multi-producer multi-consumer queue
Apache License 2.0
254 stars 21 forks source link

Move fetch_and after the prev_value check in ConcurrentQueue::single::force_push #71

Closed notgull closed 4 months ago

notgull commented 4 months ago

cc https://github.com/smol-rs/concurrent-queue/pull/58#discussion_r1574686897

taiki-e commented 4 months ago

Sorry, I meant handling the memory accesses themselves in the branch as well. And this seems to be useful even when optimization is enabled (godbolt)

                // If the value was pushed, swap out the value.
                let prev_value = if prev & PUSHED == 0 {
                    // SAFETY: write is safe because we have locked the state.
                    self.slot.with_mut(|slot| unsafe {
                        slot.write(MaybeUninit::new(value));
                    });
                    None
                } else {
                    // SAFETY: replace is safe because we have locked the state, and
                    // assume_init is safe because we have checked that the value was pushed.
                    let prev_value = unsafe {
                        self.slot
                            .with_mut(move |slot| ptr::replace(slot, MaybeUninit::new(value)).assume_init())
                    };
                    Some(prev_value)
                };
notgull commented 4 months ago

Good point. I've adopted this strategy here.