From 7355b6fb3edd80ed55ea5531854c7a450fc28287 Mon Sep 17 00:00:00 2001 From: Nicholas Brown Date: Wed, 14 Feb 2018 10:10:03 +0000 Subject: [PATCH 1/3] Remove unused timout code --- src/common/ConcurrentQueue.h | 29 ----------------------------- src/common/TimeoutSemaphore.h | 14 -------------- 2 files changed, 43 deletions(-) diff --git a/src/common/ConcurrentQueue.h b/src/common/ConcurrentQueue.h index c8c6627f..8719abe8 100644 --- a/src/common/ConcurrentQueue.h +++ b/src/common/ConcurrentQueue.h @@ -125,35 +125,6 @@ class ConcurrentQueue return true; } - // like pop above, but with absolute time instead of delta. - // use this instead of the above, makes things easier! - inline bool popAbs(const struct timeval &timeout, T *res) - { - DPRINTF_DEBUG( "(%s) trying to pop element (%d elements in queue)", ownerName.c_str(), count); - - if (popSemaphore.waitAbs(timeout)) { - // popSemaphore.wait() succeeded, now pop the frontmost element - lock.lock(); - *res = queue.front(); - queue.pop(); - poppedCount++; - count--; - lock.unlock(); - - pushSemaphore.post(); - - DPRINTF_DEBUG( "(%s) element popped", ownerName.c_str()); - - return true; - } else { - // timeout occured - DPRINTF_DEBUG( "(%s) timeout or program shutdown", ownerName.c_str()); - *res = 0; - - return false; - } - } - // like pop above, but with absolute time instead of delta. // use this instead of the above, makes things easier! inline bool popAbs(const struct timespec& timeout, T *res) diff --git a/src/common/TimeoutSemaphore.h b/src/common/TimeoutSemaphore.h index cc8b1f97..6e530870 100644 --- a/src/common/TimeoutSemaphore.h +++ b/src/common/TimeoutSemaphore.h @@ -164,20 +164,6 @@ class TimeoutSemaphore return true; } - - /** - * see documentation for waitAbs(struct timespec) - * has same functionality, just different parameter - */ - inline bool waitAbs(const struct timeval& timeout) - { - struct timespec ts; - - TIMEVAL_TO_TIMESPEC(&timeout, &ts); - - return waitAbs(ts); - } - // like wait() but with absolute time instead of delta. makes things easier! // Use this instead of the above function inline bool waitAbs(const struct timespec& ts) From cebc6854fa2715612fadd7d30ff3336a486fdfc7 Mon Sep 17 00:00:00 2001 From: Nicholas Brown Date: Wed, 14 Feb 2018 10:22:56 +0000 Subject: [PATCH 2/3] Remove verbose debug messages from busy poll loops These are extremely noisy as they are called from busy polling loops, and don't really display any internal state, just that the loop is looping. --- src/common/ConcurrentQueue.h | 6 ------ src/common/TimeoutSemaphore.h | 9 +-------- src/core/ConnectionQueue.h | 2 -- src/modules/ipfix/aggregator/BaseAggregator.cpp | 3 --- 4 files changed, 1 insertion(+), 19 deletions(-) diff --git a/src/common/ConcurrentQueue.h b/src/common/ConcurrentQueue.h index 8719abe8..4c58a64c 100644 --- a/src/common/ConcurrentQueue.h +++ b/src/common/ConcurrentQueue.h @@ -79,7 +79,6 @@ class ConcurrentQueue (ownerName.empty() ? "" : ownerName.c_str()), maxEntries-pushSemaphore.getCount()); if (!popSemaphore.wait()) { - DPRINTF_INFO("(%s) failed to pop element, program is being shut down?", ownerName.c_str()); return false; } @@ -102,11 +101,9 @@ class ConcurrentQueue // of the timeout has been reached, res will be set to NULL and false will be returned inline bool pop(long timeout_ms, T *res) { - DPRINTF_DEBUG( "(%s) trying to pop element (%d elements in queue)", ownerName.c_str(), count); // try to get an item from the queue if(!popSemaphore.wait(timeout_ms)) { // timeout occured - DPRINTF_DEBUG( "(%s) timeout", ownerName.c_str()); return false; } @@ -129,8 +126,6 @@ class ConcurrentQueue // use this instead of the above, makes things easier! inline bool popAbs(const struct timespec& timeout, T *res) { - DPRINTF_DEBUG( "(%s) trying to pop element (%d elements in queue)", ownerName.c_str(), count); - if (popSemaphore.waitAbs(timeout)) { // popSemaphore.wait() succeeded, now pop the frontmost element lock.lock(); @@ -148,7 +143,6 @@ class ConcurrentQueue } else { // timeout occured - DPRINTF_DEBUG( "(%s) timeout or program shutdown", ownerName.c_str()); *res = 0; return false; diff --git a/src/common/TimeoutSemaphore.h b/src/common/TimeoutSemaphore.h index 6e530870..e0ebd8f8 100644 --- a/src/common/TimeoutSemaphore.h +++ b/src/common/TimeoutSemaphore.h @@ -106,8 +106,6 @@ class TimeoutSemaphore return false; break; default: - // semaphore could not be aquired because of several reasons, but none are fatal - DPRINTF_DEBUG( "timedwait (<0) returned with %d (%s)", errno, strerror(errno)); return false; } } @@ -120,7 +118,6 @@ class TimeoutSemaphore retval = sem_timedwait(sem, &timeout); #endif if (retval != 0 && errno != ETIMEDOUT) { - DPRINTF_DEBUG( "timedwait (>=0) returned with %d: %s", errno, strerror(errno)); switch (errno) { case EINVAL: /* @@ -141,8 +138,7 @@ class TimeoutSemaphore return false; break; default: - // semaphore could not be aquired because of several reasons, but none are fatal - DPRINTF_DEBUG( "timedwait (>=0) returned with %d", errno); + break; } } if (errno == ETIMEDOUT) { @@ -201,9 +197,6 @@ class TimeoutSemaphore return false; break; default: - // semaphore not acquired for non-fatal reasons - DPRINTF_DEBUG( "timedwait returned with %s", - strerror(errno)); return false; } } diff --git a/src/core/ConnectionQueue.h b/src/core/ConnectionQueue.h index 44a8cfd0..7797feb3 100644 --- a/src/core/ConnectionQueue.h +++ b/src/core/ConnectionQueue.h @@ -239,12 +239,10 @@ class ConnectionQueue : public Adapter, public Timer struct timespec nexttimeout; if (!processTimeouts(nexttimeout)) { if (!queue.pop(&element)) { - DPRINTF_INFO("queue.pop failed - timeout?"); continue; } } else { if (!queue.popAbs(nexttimeout, &element)) { - DPRINTF_INFO("queue.pop failed - timeout?"); continue; } } diff --git a/src/modules/ipfix/aggregator/BaseAggregator.cpp b/src/modules/ipfix/aggregator/BaseAggregator.cpp index d2229342..59550477 100644 --- a/src/modules/ipfix/aggregator/BaseAggregator.cpp +++ b/src/modules/ipfix/aggregator/BaseAggregator.cpp @@ -171,15 +171,12 @@ void BaseAggregator::exporterThread() } gettimeofday(&curtime, 0); - DPRINTF_DEBUG("Aggregator: starting Export"); for (size_t i = 0; i < rules->count; i++) { rules->rule[i]->hashtable->expireFlows(); } struct timeval endtime; gettimeofday(&endtime, 0); timeval_subtract(&difftime, &endtime, &curtime); - - DPRINTF_DEBUG("Aggregator: export took %.03f secs", (float)difftime.tv_usec/1000000+difftime.tv_sec); } if (getShutdownProperly()) { From c0b4e79b11b239859e9537a7713b10cafff6ac4e Mon Sep 17 00:00:00 2001 From: Nicholas Brown Date: Wed, 14 Feb 2018 11:00:02 +0000 Subject: [PATCH 3/3] factor out a common function --- src/common/ConcurrentQueue.h | 67 +++++++++++------------------------- 1 file changed, 21 insertions(+), 46 deletions(-) diff --git a/src/common/ConcurrentQueue.h b/src/common/ConcurrentQueue.h index 4c58a64c..4f724d14 100644 --- a/src/common/ConcurrentQueue.h +++ b/src/common/ConcurrentQueue.h @@ -21,6 +21,24 @@ template class ConcurrentQueue { + private: + + inline bool do_pop(T* res) + { + lock.lock(); + *res = queue.front(); + queue.pop(); + poppedCount++; + count--; + lock.unlock(); + + pushSemaphore.post(); + + DPRINTF_DEBUG( "(%s) element popped", ownerName.c_str()); + + return true; + } + public: /** * default queue size @@ -75,25 +93,10 @@ class ConcurrentQueue inline bool pop(T* res) { - DPRINTF_DEBUG( "(%s) trying to pop element (%d elements in queue)", - (ownerName.empty() ? "" : ownerName.c_str()), - maxEntries-pushSemaphore.getCount()); if (!popSemaphore.wait()) { return false; } - - lock.lock(); - *res = queue.front(); - queue.pop(); - poppedCount++; - count--; - lock.unlock(); - - pushSemaphore.post(); - - DPRINTF_DEBUG( "(%s) element popped", ownerName.c_str()); - - return true; + return do_pop(res); }; // try to pop an entry from the queue before timeout occurs @@ -101,25 +104,10 @@ class ConcurrentQueue // of the timeout has been reached, res will be set to NULL and false will be returned inline bool pop(long timeout_ms, T *res) { - // try to get an item from the queue if(!popSemaphore.wait(timeout_ms)) { - // timeout occured return false; } - - // popSemaphore.wait() succeeded, now pop the frontmost element - lock.lock(); - *res = queue.front(); - queue.pop(); - poppedCount++; - count--; - lock.unlock(); - - pushSemaphore.post(); - - DPRINTF_DEBUG( "(%s) element popped", ownerName.c_str()); - - return true; + return do_pop(res); } // like pop above, but with absolute time instead of delta. @@ -127,24 +115,11 @@ class ConcurrentQueue inline bool popAbs(const struct timespec& timeout, T *res) { if (popSemaphore.waitAbs(timeout)) { - // popSemaphore.wait() succeeded, now pop the frontmost element - lock.lock(); - *res = queue.front(); - queue.pop(); - poppedCount++; - count--; - lock.unlock(); - - pushSemaphore.post(); - - DPRINTF_DEBUG( "(%s) element popped", ownerName.c_str()); - - return true; + return do_pop(res); } else { // timeout occured *res = 0; - return false; } }