Skip to content

Commit b2f1b31

Browse files
Address review feedback from cameron314
- Reword traits doc comment per maintainer's suggestion (no formula, no issue ref) - Shorten try_enqueue note and add it consistently to all implicit try-enqueue methods - Rename "Fix" to "Workaround" in test comments (behavior is by design)
1 parent 1a7c45f commit b2f1b31

2 files changed

Lines changed: 17 additions & 13 deletions

File tree

concurrentqueue.h

Lines changed: 15 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -355,12 +355,12 @@ struct ConcurrentQueueDefaultTraits
355355

356356
// How many full blocks can be expected for a single implicit producer? This should
357357
// reflect that number's maximum for optimal performance. Must be a power of 2.
358-
// Note: This controls the maximum number of elements that can be enqueued by a
359-
// single implicit producer when using try_enqueue (which does not allocate).
360-
// The limit is BLOCK_SIZE * IMPLICIT_INITIAL_INDEX_SIZE elements; beyond that,
361-
// the block index needs to grow, which requires allocation, causing try_enqueue
362-
// to fail. Increase this value (or use enqueue(), which can allocate) if you need
363-
// more capacity per implicit producer. See also issue #418.
358+
// Note: This impacts the maximum number of elements that can be enqueued by a
359+
// single implicit producer when using try_enqueue/try_enqeue_bulk exclusively (which
360+
// cannot allocate), since it limits the number of blocks that the producer can hold to
361+
// store elements. When pre-allocating blocks for use with try-enqueueing, configure
362+
// this initial size to the desired maximum number of blocks per implicit producer.
363+
// Alternately, use the regular enqueue methods, which can grow the index as needed.
364364
static const size_t IMPLICIT_INITIAL_INDEX_SIZE = 32;
365365

366366
// The initial size of the hash table mapping thread IDs to implicit producers.
@@ -1069,11 +1069,9 @@ class ConcurrentQueue
10691069
// Does not allocate memory. Fails if not enough room to enqueue (or implicit
10701070
// production is disabled because Traits::INITIAL_IMPLICIT_PRODUCER_HASH_SIZE
10711071
// is 0).
1072-
// Note: For implicit producers (no token), the maximum number of elements that
1073-
// can be held is BLOCK_SIZE * IMPLICIT_INITIAL_INDEX_SIZE before the block
1074-
// index must grow (which requires allocation and causes try_enqueue to fail).
1075-
// Pre-allocating blocks via the constructor does not increase the index size.
1076-
// Increase IMPLICIT_INITIAL_INDEX_SIZE in your traits, or use enqueue() instead.
1072+
// Note: If using only try_enqueue/try_enqueue_bulk with pre-allocated blocks, configure
1073+
// Traits::IMPLICIT_INITIAL_INDEX_SIZE appropriately to ensure the index has sufficient
1074+
// capacity for the number of blocks each producer may need.
10771075
// Thread-safe.
10781076
inline bool try_enqueue(T const& item)
10791077
{
@@ -1085,6 +1083,9 @@ class ConcurrentQueue
10851083
// Does not allocate memory (except for one-time implicit producer).
10861084
// Fails if not enough room to enqueue (or implicit production is
10871085
// disabled because Traits::INITIAL_IMPLICIT_PRODUCER_HASH_SIZE is 0).
1086+
// Note: If using only try_enqueue/try_enqueue_bulk with pre-allocated blocks, configure
1087+
// Traits::IMPLICIT_INITIAL_INDEX_SIZE appropriately to ensure the index has sufficient
1088+
// capacity for the number of blocks each producer may need.
10881089
// Thread-safe.
10891090
inline bool try_enqueue(T&& item)
10901091
{
@@ -1112,6 +1113,9 @@ class ConcurrentQueue
11121113
// Does not allocate memory (except for one-time implicit producer).
11131114
// Fails if not enough room to enqueue (or implicit production is
11141115
// disabled because Traits::INITIAL_IMPLICIT_PRODUCER_HASH_SIZE is 0).
1116+
// Note: If using only try_enqueue/try_enqueue_bulk with pre-allocated blocks, configure
1117+
// Traits::IMPLICIT_INITIAL_INDEX_SIZE appropriately to ensure the index has sufficient
1118+
// capacity for the number of blocks each producer may need.
11151119
// Note: Use std::make_move_iterator if the elements should be moved
11161120
// instead of copied.
11171121
// Thread-safe.

tests/unittests/unittests.cpp

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5070,7 +5070,7 @@ class ConcurrentQueueTests : public TestClass<ConcurrentQueueTests>
50705070
}
50715071

50725072
{
5073-
// Fix #1: enqueue() (which can allocate) works past the limit
5073+
// Workaround #1: enqueue() (which can allocate) can grow the index
50745074
const int limit = (int)(SmallImplicitIndexTraits::BLOCK_SIZE * SmallImplicitIndexTraits::IMPLICIT_INITIAL_INDEX_SIZE);
50755075
ConcurrentQueue<int, SmallImplicitIndexTraits> q(limit + 64);
50765076

@@ -5087,7 +5087,7 @@ class ConcurrentQueueTests : public TestClass<ConcurrentQueueTests>
50875087
}
50885088

50895089
{
5090-
// Fix #2: larger IMPLICIT_INITIAL_INDEX_SIZE allows more try_enqueue calls
5090+
// Workaround #2: larger IMPLICIT_INITIAL_INDEX_SIZE allows more try_enqueue calls
50915091
const int small_limit = (int)(SmallImplicitIndexTraits::BLOCK_SIZE * SmallImplicitIndexTraits::IMPLICIT_INITIAL_INDEX_SIZE); // 16
50925092
const int large_limit = (int)(LargerImplicitIndexTraits::BLOCK_SIZE * LargerImplicitIndexTraits::IMPLICIT_INITIAL_INDEX_SIZE); // 64
50935093
ConcurrentQueue<int, LargerImplicitIndexTraits> q(large_limit + 64);

0 commit comments

Comments
 (0)