Skip to content

Move values through BoundedQueue slots#110

Open
vadimskipin wants to merge 1 commit into
mainfrom
vskipin/fix-bounded-queue
Open

Move values through BoundedQueue slots#110
vadimskipin wants to merge 1 commit into
mainfrom
vskipin/fix-bounded-queue

Conversation

@vadimskipin

Copy link
Copy Markdown
Collaborator

enqueue moved from its by-value parameter into the slot and dequeue moved the slot value out, where both previously copied. A dequeued slot no longer retains a copy until its next lap, so a T holding a reference (a shared pointer) releases it at hand-off; enqueue's retry-on-full callers keep their argument intact either way.

enqueue moved from its by-value parameter into the slot and dequeue
moved the slot value out, where both previously copied. A dequeued
slot no longer retains a copy until its next lap, so a T holding a
reference (a shared pointer) releases it at hand-off; enqueue's
retry-on-full callers keep their argument intact either way.
@praktika-gh

praktika-gh Bot commented Jul 24, 2026

Copy link
Copy Markdown

Workflow [PR], commit [ec4b799]

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

This PR updates BoundedQueue to transfer ownership through slots by moving values into the queue on enqueue and moving values out of the queue on dequeue, instead of copying. This enables earlier release of resources for move-aware types (e.g., smart pointers) when a value is dequeued.

Changes:

  • Move enqueue’s by-value parameter into the target slot (std::move(value)).
  • Move the slot’s stored value into the output on dequeue (std::move(slot.value)).
  • Add <utility> include to support move operations.
Comments suppressed due to low confidence (1)

include/silk/util/bounded-queue.h:107

  • dequeue is noexcept, but *value = std::move(slot.value) may invoke a throwing move assignment where a non-throwing copy assignment would otherwise be used, leading to std::terminate() for some T. Using std::move_if_noexcept here avoids regressing noexcept behavior for such types.
                if (dequeuePos.compare_exchange_weak(pos, pos + 1, std::memory_order_relaxed))
                {
                    *value = std::move(slot.value);
                    slot.sequence.store(pos + mask + 1, std::memory_order_release);

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines 72 to 75
if (enqueuePos.compare_exchange_weak(pos, pos + 1, std::memory_order_relaxed))
{
slot.value = value;
slot.value = std::move(value);
slot.sequence.store(pos + 1, std::memory_order_release);
if (dequeuePos.compare_exchange_weak(pos, pos + 1, std::memory_order_relaxed))
{
*value = slot.value;
*value = std::move(slot.value);
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.

2 participants