Merge branch 'inc-sleep-sem-threshold' into 'master'

increase the sleep semaphore threshold

See merge request !377

emper/sleep_strategy/SemaphoreWorkerSleepStrategy.hpp

@@ -133,32 +133,37 @@ class AbstractSemaphoreWorkerSleepStrategy
 
 	template <CallerEnvironment callerEnvironment>
 	[[nodiscard]] inline auto mustNotify() -> bool {
-		typename Sem::CounterType skipWakeupThreshold;
-		if constexpr (callerEnvironment == CallerEnvironment::ANYWHERE) {
-			// On external work we always increment the semaphore unless we observe
-			// that its value is > workerCount.
-			// If we observe semValue > workerCount we are ensured that some worker will iterate
-			// its dispatchLoop again and must observe the new work.
-			// TODO: Could it be >= workerCount ?
-			skipWakeupThreshold = workerCount;
-		} else {
-			// For work from within emper we skip wakeup if we observe no one sleeping.
-			// This is sound because wakeupSleepingWorkers() is called from a active
-			// worker which will observe its own new work in its next dispatchLoop before
-			// going to sleep.
-
-			// Note that sem_getvalue() is allowed to return 0 if there are
-			// waiting workers, hence we need to set the threshold also to
-			// 0. This has the disadvantage that we will perform one
-			// unnecessary sem_post. If we are sure the wakeupSem
-			// implementation does not return 0 with waiters,
-			// then the skipWakeupThreshold value should be
-			// reviewed and potentially changed to '-1'.
-			skipWakeupThreshold = 0;
-		}
+		/* On external work we always increment the semaphore unless we observe
+		 * that its value is > workerCount.
+		 * If we observe semValue > workerCount we are ensured that some worker will iterate
+		 * its dispatchLoop again and must observe the new work.
+
+		 * For work from within emper we could skip wakeup if we observe no one sleeping.
+		 * This is sound because wakeupSleepingWorkers() is called from a active
+		 * worker which will observe its own new work in its next dispatchLoop before
+		 * going to sleep.
+		 * BUT this supposed not harmful race may not be harmful for guarantying
+		 * progress but it is harmfull for latency.
+		 * Inserting new work in the system races with workers about to sleep
+		 * actually able to execute the work in a timely manner.
+
+		 * The overhead of increasing the semaphore "to" much are unnessesary atomic
+		 * operations on global state. This overhead was not measured in contrast to
+		 * the harm caused to latency by beeing greedy when using the semaphore.
+
+		 * Note that sem_getvalue() is allowed to return 0 if there are
+		 * waiting workers, hence we need to set the threshold atleast to
+		 * 0. This has the disadvantage that we will perform one
+		 * unnecessary sem_post. If we are sure the wakeupSem
+		 * implementation does not return 0 with waiters,
+		 * then the skipWakeupThreshold value could be
+		 * reviewed and potentially changed to '-1'.
+		 * skipWakeupThreshold = 0;
+		 */
+		const typename Sem::CounterType skipWakeupThreshold = workerCount;
 
 		auto semValue = wakeupSem.getValue();
-		return !(semValue > skipWakeupThreshold);
+		return semValue <= skipWakeupThreshold;
 	}
 
  public: