Avoid unnecessary wake-ups for timers.

base/application.cpp

@@ -101,7 +101,10 @@ void Application::RunEventLoop(void)
 
 		Object::ClearHeldObjects();
 
-		Timer::CallExpiredTimers();
+		long sleep = Timer::ProcessTimers();
+
+		if (m_ShuttingDown)
+			break;
 
 		FD_ZERO(&readfds);
 		FD_ZERO(&writefds);
@@ -134,15 +137,8 @@ void Application::RunEventLoop(void)
 				nfds = fd;
 		}
 
-		time_t now = time(NULL);
-		time_t next = Timer::GetNextCall();
-		time_t sleep = (next < now) ? 0 : (next - now);
-
-		if (m_ShuttingDown)
-			break;
-
 		timeval tv;
-		tv.tv_sec = (sleep < 0) ? 0 : (long)sleep;
+		tv.tv_sec = sleep;
 		tv.tv_usec = 0;
 
 		int ready;

base/timer.cpp

@@ -21,7 +21,6 @@
 
 using namespace icinga;
 
-time_t Timer::NextCall;
 Timer::CollectionType Timer::Timers;
 
 /**
@@ -33,45 +32,13 @@ Timer::Timer(void)
 }
 
 /**
- * Retrieves when the next timer is due.
+ * Calls expired timers and returned when the next wake-up should happen.
  *
  * @returns Time when the next timer is due.
  */
-time_t Timer::GetNextCall(void)
+long Timer::ProcessTimers(void)
 {
-	if (NextCall < time(NULL))
+	long wakeup = 30;
-		Timer::RescheduleTimers();
-
-	return NextCall;
-}
-
-/**
- * Reschedules all timers, thereby updating the NextCall
- * timestamp used by the GetNextCall() function.
- */
-void Timer::RescheduleTimers(void)
-{
-	/* Make sure we wake up at least once every 30 seconds */
-	NextCall = time(NULL) + 30;
-
-	for (Timer::CollectionType::iterator i = Timers.begin(); i != Timers.end(); i++) {
-		Timer::Ptr timer = i->lock();
-
-		if (timer == NULL)
-			continue;
-
-		if (timer->m_Next < NextCall)
-			NextCall = timer->m_Next;
-	}
-}
-
-/**
- * Calls all expired timers and reschedules them.
- */
-void Timer::CallExpiredTimers(void)
-{
-	time_t now;
-	time(&now);
 
 	Timer::CollectionType::iterator prev, i;
 	for (i = Timers.begin(); i != Timers.end(); ) {
@@ -85,11 +52,28 @@ void Timer::CallExpiredTimers(void)
 			continue;
 		}
 
+		time_t now;
+		time(&now);
+
 		if (timer->m_Next <= now) {
 			timer->Call();
-			timer->Reschedule(time(NULL) + timer->GetInterval());
+
+			/* time may have changed depending on how long the
+			 * timer call took - we need to fetch the current time */
+			time(&now);
+
+			timer->Reschedule(now + timer->GetInterval());
 		}
+
+		assert(timer->m_Next > now);
+
+		if (timer->m_Next - now < wakeup)
+			wakeup = timer->m_Next - now;
 	}
+
+	assert(wakeup > 0);
+
+	return wakeup;
 }
 
 /**
@@ -119,8 +103,11 @@ void Timer::Call(void)
  *
  * @param interval The new interval.
  */
-void Timer::SetInterval(time_t interval)
+void Timer::SetInterval(long interval)
 {
+	if (interval <= 0)
+		throw invalid_argument("interval");
+
 	m_Interval = interval;
 }
 
@@ -129,7 +116,7 @@ void Timer::SetInterval(time_t interval)
  *
  * @returns The interval.
  */
-time_t Timer::GetInterval(void) const
+long Timer::GetInterval(void) const
 {
 	return m_Interval;
 }
@@ -162,7 +149,4 @@ void Timer::Stop(void)
 void Timer::Reschedule(time_t next)
 {
 	m_Next = next;
-
-	if (next < NextCall)
-		NextCall = next;
 }

base/timer.h

@@ -41,11 +41,10 @@ public:
 
 	Timer(void);
 
-	void SetInterval(time_t interval);
+	void SetInterval(long interval);
-	time_t GetInterval(void) const;
+	long GetInterval(void) const;
 
-	static time_t GetNextCall(void);
+	static long ProcessTimers(void);
-	static void CallExpiredTimers(void);
 
 	void Start(void);
 	void Stop(void);
@@ -55,13 +54,9 @@ public:
 	boost::signal<void(const Timer::Ptr&)> OnTimerExpired;
 
 private:
-	time_t m_Interval; /**< The interval of the timer. */
+	long m_Interval; /**< The interval of the timer. */
 	time_t m_Next; /**< When the next event should happen. */
 
-	static time_t NextCall; /**< When the next event should happen (for all timers). */
-
-	static void RescheduleTimers(void);
-
 	void Call(void);
 };