increase the sleep semaphore threshold

We currently use the semaphore of our sleep strategy very greedy. This is due to skipping the semaphore's V() operation if we are sure that it does not kill our progress guaranty.

When scheduling new work from within the runtime we skip the wakeup if we observe nobody sleeping. This is fine in terms of progress and limits the amount of atomic operations on global state to a minimum.

Using a threshold of 0 (observe nobody sleeping) however introduces a race between inserting new work and going to sleep which harm the latency when the worker goes to sleep and is not notified about the new work.

This race is common and can be observed in the pulse micro benchmark. A emper with a threshold of 0 shows high latency compared to using an io-based sleep strategy or when increasing the threshold.

$ build-release/eval/pulse | python -c "<calc mean>"
Starting pulse evaluation with pulse=1, iterations=30 and utilization=80
mean: 1721970116.425

$ build-increased-sem-threshold/eval/pulse | python -c "<calc mean"
Starting pulse evaluation with pulse=1, iterations=30 and utilization=80
mean: 1000023942.15

$ build-pipe-release/eval/pulse | python -c "<calc mean>
Starting pulse evaluation with pulse=1, iterations=30 and utilization=80
mean: 1000030557.0861111

$ build-pipe-no-completer/eval/pulse | python -c "<calc mean>"
Starting pulse evaluation with pulse=1, iterations=30 and utilization=80
mean: 1000021514.1805556

I could not measure any significant overhead due to the more atomics on my 16 core machine using the fs-eval on a SSD.

$ ./eval.py -r 50 -i emper-vanilla emper-inc-sem-threshold emper-pipe emper-pipe-no-completer
...
$ ./summarize.py results/1599f44-dirty-pasture/<date>/ -f '{target}-{median} (+- {std})'
duration_time:u:
emper-vanilla-0.202106189 (+- 0.016075981812486713)
emper-inc-sem-threshold-0.2049344115 (+- 0.015348506939891596)
emper-pipe-0.21689131 (+- 0.015198438371285145)
emper-pipe-no-completer-0.1372724185 (+- 0.005865720218833998)

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: