From 08d62bf69ebf008c7ea2bd5689b210937fa61802 Mon Sep 17 00:00:00 2001 From: Severin Leonhardt Date: Fri, 23 Sep 2022 16:21:26 +0200 Subject: [PATCH 1/2] Remove unreachable code This was noticed by MSVC in a debug build which produced the following warning: warning C4702: unreachable code --- src/execution_profile.cpp | 1 - src/mpmc_queue.hpp | 6 ------ 2 files changed, 7 deletions(-) diff --git a/src/execution_profile.cpp b/src/execution_profile.cpp index e57e6b75e..6292be440 100644 --- a/src/execution_profile.cpp +++ b/src/execution_profile.cpp @@ -108,7 +108,6 @@ CassError cass_execution_profile_set_latency_aware_routing_settings( CassError cass_execution_profile_set_whitelist_filtering(CassExecProfile* profile, const char* hosts) { return cass_execution_profile_set_whitelist_filtering_n(profile, hosts, SAFE_STRLEN(hosts)); - return CASS_OK; } CassError cass_execution_profile_set_whitelist_filtering_n(CassExecProfile* profile, diff --git a/src/mpmc_queue.hpp b/src/mpmc_queue.hpp index f6cd146ca..89aae4d9d 100644 --- a/src/mpmc_queue.hpp +++ b/src/mpmc_queue.hpp @@ -84,9 +84,6 @@ class MPMCQueue : public Allocated { pos = tail_.load(MEMORY_ORDER_RELAXED); } } - - // never taken - return false; } bool dequeue(T& data) { @@ -120,9 +117,6 @@ class MPMCQueue : public Allocated { pos = head_.load(MEMORY_ORDER_RELAXED); } } - - // never taken - return false; } bool is_empty() const { From 7e61eefb642f8dd5e0319ac559a25e95c3db1a33 Mon Sep 17 00:00:00 2001 From: Severin Leonhardt Date: Fri, 14 Oct 2022 09:12:03 +0200 Subject: [PATCH 2/2] Update slot outside of the loop in MPMCQueue As suggested by @absurdfarce this change keeps finding a slot in the loop but moves the use of the slot past the loop. That way there is a return after the loop which should make clear to any code analysis that this function will always return something. --- src/mpmc_queue.hpp | 32 +++++++++++++++++++------------- 1 file changed, 19 insertions(+), 13 deletions(-) diff --git a/src/mpmc_queue.hpp b/src/mpmc_queue.hpp index 89aae4d9d..e85975d83 100644 --- a/src/mpmc_queue.hpp +++ b/src/mpmc_queue.hpp @@ -56,9 +56,10 @@ class MPMCQueue : public Allocated { // buffer must be a size which is a power of 2. this also allows // the sequence to double as a ticket/lock. size_t pos = tail_.load(MEMORY_ORDER_RELAXED); + Node* node; for (;;) { - Node* node = &buffer_[pos & mask_]; + node = &buffer_[pos & mask_]; size_t node_seq = node->seq.load(MEMORY_ORDER_ACQUIRE); intptr_t dif = (intptr_t)node_seq - (intptr_t)pos; @@ -69,11 +70,7 @@ class MPMCQueue : public Allocated { // weak compare is faster, but can return spurious results // which in this instance is OK, because it's in the loop if (tail_.compare_exchange_weak(pos, pos + 1, MEMORY_ORDER_RELAXED)) { - // set the data - node->data = data; - // increment the sequence so that the tail knows it's accessible - node->seq.store(pos + 1, MEMORY_ORDER_RELEASE); - return true; + break; } } else if (dif < 0) { // if seq is less than head seq then it means this slot is @@ -84,13 +81,20 @@ class MPMCQueue : public Allocated { pos = tail_.load(MEMORY_ORDER_RELAXED); } } + + // set the data + node->data = data; + // increment the sequence so that the tail knows it's accessible + node->seq.store(pos + 1, MEMORY_ORDER_RELEASE); + return true; } bool dequeue(T& data) { size_t pos = head_.load(MEMORY_ORDER_RELAXED); + Node* node; for (;;) { - Node* node = &buffer_[pos & mask_]; + node = &buffer_[pos & mask_]; size_t node_seq = node->seq.load(MEMORY_ORDER_ACQUIRE); intptr_t dif = (intptr_t)node_seq - (intptr_t)(pos + 1); @@ -101,12 +105,7 @@ class MPMCQueue : public Allocated { // weak compare is faster, but can return spurious results // which in this instance is OK, because it's in the loop if (head_.compare_exchange_weak(pos, pos + 1, MEMORY_ORDER_RELAXED)) { - // set the output - data = node->data; - // set the sequence to what the head sequence should be next - // time around - node->seq.store(pos + mask_ + 1, MEMORY_ORDER_RELEASE); - return true; + break; } } else if (dif < 0) { // if seq is less than head seq then it means this slot is @@ -117,6 +116,13 @@ class MPMCQueue : public Allocated { pos = head_.load(MEMORY_ORDER_RELAXED); } } + + // set the output + data = node->data; + // set the sequence to what the head sequence should be next + // time around + node->seq.store(pos + mask_ + 1, MEMORY_ORDER_RELEASE); + return true; } bool is_empty() const {