This is an automated email from the ASF dual-hosted git repository.

pnoltes pushed a commit to branch feature/scheduled_event_on_event_thread
in repository https://gitbox.apache.org/repos/asf/celix.git

commit fe902d063fc23a76246b9519415511b03f48773e
Author: Pepijn Noltes <[email protected]>
AuthorDate: Mon Jul 3 19:55:24 2023 +0200

    Refactor scheduled event to use nextDeadline instead of 
lastScheduledEventTime
---
 libs/framework/src/celix_scheduled_event.c | 24 +++++++-----------------
 libs/framework/src/celix_scheduled_event.h |  5 ++---
 libs/framework/src/framework.c             | 30 +++++++++++++-----------------
 libs/utils/gtest/src/TimeUtilsTestSuite.cc | 24 ++++++++++++++++++++++++
 libs/utils/include/celix_utils.h           |  8 ++++++++
 libs/utils/src/utils.c                     | 11 +++++++++++
 6 files changed, 65 insertions(+), 37 deletions(-)

diff --git a/libs/framework/src/celix_scheduled_event.c 
b/libs/framework/src/celix_scheduled_event.c
index ec1d2584..69b651dc 100644
--- a/libs/framework/src/celix_scheduled_event.c
+++ b/libs/framework/src/celix_scheduled_event.c
@@ -72,7 +72,7 @@ struct celix_scheduled_event {
     size_t callCount;           /**< The call count of the scheduled event. */
     bool isMarkedForRemoval;    /**< Whether the scheduled event is marked for 
removal. */
     bool isRemoved;             /**< Whether the scheduled event is removed. */
-    struct timespec lastScheduledEventTime; /**< The last scheduled event time 
of the scheduled event. */
+    struct timespec nextDeadline; /**< The next deadline of the scheduled 
event. */
     bool processForWakeup; /**< Whether the scheduled event should be 
processed directly due to a wakeupScheduledEvent
                               call. */
 };
@@ -119,7 +119,7 @@ celix_scheduled_event_t* 
celix_scheduledEvent_create(celix_framework_t* fw,
     event->useCount = 1;
     event->callCount = 0;
     event->isRemoved = false;
-    event->lastScheduledEventTime = celixThreadCondition_getTime();
+    event->nextDeadline = 
celixThreadCondition_getDelayedTime(event->initialDelayInSeconds);
     event->processForWakeup = false;
 
     celixThreadMutex_create(&event->mutex, NULL);
@@ -177,25 +177,15 @@ long celix_scheduledEvent_getBundleId(const 
celix_scheduled_event_t* event) { re
 
 bool celix_scheduledEvent_deadlineReached(celix_scheduled_event_t* event,
                                           const struct timespec* currentTime,
-                                          double* nextProcessTimeInSeconds) {
+                                          struct timespec* nextDeadline) {
     celixThreadMutex_lock(&event->mutex);
-    double elapsed = celix_difftime(&event->lastScheduledEventTime, 
currentTime);
-    double deadline = event->callCount == 0 ? event->initialDelayInSeconds : 
event->intervalInSeconds;
-    deadline -= CELIX_SCHEDULED_EVENT_INTERVAL_ALLOW_ERROR_IN_SECONDS;
-    bool deadlineReached = elapsed >= deadline;
+    double timeLeft = celix_difftime(currentTime, &event->nextDeadline);
+    bool deadlineReached = timeLeft - 
CELIX_SCHEDULED_EVENT_INTERVAL_ALLOW_ERROR_IN_SECONDS <= 0;
     if (event->processForWakeup) {
         deadlineReached = true;
     }
-
-    if (deadlineReached && nextProcessTimeInSeconds) {
-        *nextProcessTimeInSeconds =
-            event->intervalInSeconds == 0 /*one shot*/ ? 
CELIX_FRAMEWORK_DEFAULT_MAX_TIMEDWAIT_EVENT_HANDLER_IN_SECONDS
-                                                       : 
event->intervalInSeconds;
-    } else if (nextProcessTimeInSeconds) {
-        *nextProcessTimeInSeconds = event->callCount == 0 ? 
event->initialDelayInSeconds : event->intervalInSeconds;
-    }
     celixThreadMutex_unlock(&event->mutex);
-    return deadlineReached;
+    return deadlineReached || event->processForWakeup;
 }
 
 void celix_scheduledEvent_process(celix_scheduled_event_t* event, const struct 
timespec* currentTime) {
@@ -211,7 +201,7 @@ void celix_scheduledEvent_process(celix_scheduled_event_t* 
event, const struct t
     struct timespec end = celix_gettime(CLOCK_MONOTONIC);
 
     celixThreadMutex_lock(&event->mutex);
-    event->lastScheduledEventTime = *currentTime;
+    event->nextDeadline = celix_delayedTimespec(currentTime, 
event->intervalInSeconds);
     event->callCount += 1;
     event->processForWakeup = false;
     celixThreadCondition_broadcast(&event->cond); // for changed callCount
diff --git a/libs/framework/src/celix_scheduled_event.h 
b/libs/framework/src/celix_scheduled_event.h
index 0dca61b7..77ff37e6 100644
--- a/libs/framework/src/celix_scheduled_event.h
+++ b/libs/framework/src/celix_scheduled_event.h
@@ -114,13 +114,12 @@ long celix_scheduledEvent_getBundleId(const 
celix_scheduled_event_t* event);
  * @brief Returns whether the event deadline is reached and the event should 
be processed.
  * @param[in] event The event to check.
  * @param[in] currentTime The current time.
- * @param[out] nextProcessTimeInSeconds The time in seconds until the next 
event should be processed.
- *                                      if the deadline is reached, this is 
the next interval.
+ * @param[out] nextDeadline The next deadline. Can be NULL.
  * @return true if the event deadline is reached and the event should be 
processed.
  */
 bool celix_scheduledEvent_deadlineReached(celix_scheduled_event_t* event,
                                           const struct timespec* currentTime,
-                                          double* nextProcessTimeInSeconds);
+                                          struct timespec* nextDeadline);
 
 /**
  * @brief Process the event by calling the event callback.
diff --git a/libs/framework/src/framework.c b/libs/framework/src/framework.c
index 4c5a4fdb..63f2740d 100644
--- a/libs/framework/src/framework.c
+++ b/libs/framework/src/framework.c
@@ -1407,17 +1407,17 @@ static inline void fw_handleEvents(celix_framework_t* 
framework) {
 /**
  * @brief Process all scheduled events.
  */
-static double celix_framework_processScheduledEvents(celix_framework_t* fw) {
+static struct timespec 
celix_framework_processScheduledEvents(celix_framework_t* fw) {
     struct timespec ts = celixThreadCondition_getTime();
 
-    double nextClosestScheduledEvent;
+    struct timespec closestDeadline;
     celix_scheduled_event_t* callEvent;
     celix_scheduled_event_t* removeEvent;
     do {
-        nextClosestScheduledEvent = -1; //negative means no event next event
+        closestDeadline.tv_sec = 0;
+        closestDeadline.tv_nsec = 0;
         callEvent = NULL;
         removeEvent = NULL;
-        double nextEvent;
         celixThreadMutex_lock(&fw->dispatcher.mutex);
         CELIX_LONG_HASH_MAP_ITERATE(fw->dispatcher.scheduledEvents, entry) {
             celix_scheduled_event_t* visit = entry.value.ptrValue;
@@ -1427,9 +1427,10 @@ static double 
celix_framework_processScheduledEvents(celix_framework_t* fw) {
                 break;
             }
 
-            bool call = celix_scheduledEvent_deadlineReached(visit, &ts, 
&nextEvent);
-            if (nextClosestScheduledEvent < 0 || nextEvent < 
nextClosestScheduledEvent) {
-                nextClosestScheduledEvent = nextEvent;
+            struct timespec nextDeadline;
+            bool call = celix_scheduledEvent_deadlineReached(visit, &ts, 
&nextDeadline);
+            if (celix_compareTime(&nextDeadline, &closestDeadline) < 0) {
+                closestDeadline = nextDeadline;
             }
             if (call) {
                 callEvent = visit;
@@ -1458,7 +1459,7 @@ static double 
celix_framework_processScheduledEvents(celix_framework_t* fw) {
         }
     } while (callEvent || removeEvent);
 
-    return nextClosestScheduledEvent;
+    return closestDeadline;
 }
 
 void celix_framework_cleanupScheduledEvents(celix_framework_t* fw, long bndId) 
{
@@ -1513,15 +1514,10 @@ static bool 
requiresScheduledEventsProcessing(celix_framework_t* framework) {
     return eventProcessingRequired;
 }
 
-static void celix_framework_waitForNextEvent(celix_framework_t* fw, double 
nextScheduledEvent) {
-    if (nextScheduledEvent < 0 || nextScheduledEvent > 
CELIX_FRAMEWORK_DEFAULT_MAX_TIMEDWAIT_EVENT_HANDLER_IN_SECONDS) {
-        nextScheduledEvent = 
CELIX_FRAMEWORK_DEFAULT_MAX_TIMEDWAIT_EVENT_HANDLER_IN_SECONDS;
-    }
-    struct timespec absTimeout = 
celixThreadCondition_getDelayedTime(nextScheduledEvent);
-
+static void celix_framework_waitForNextEvent(celix_framework_t* fw, struct 
timespec nextDeadline) {
     celixThreadMutex_lock(&fw->dispatcher.mutex);
     if (celix_framework_eventQueueSize(fw) == 0 && 
!requiresScheduledEventsProcessing(fw) && fw->dispatcher.active) {
-        celixThreadCondition_waitUntil(&fw->dispatcher.cond, 
&fw->dispatcher.mutex, &absTimeout);
+        celixThreadCondition_waitUntil(&fw->dispatcher.cond, 
&fw->dispatcher.mutex, &nextDeadline);
         // note failing through to fw_eventDispatcher even if timeout is not 
reached, the fw_eventDispatcher
         // will call this again after processing the events and scheduled 
events.
     }
@@ -1537,8 +1533,8 @@ static void *fw_eventDispatcher(void *fw) {
 
     while (active) {
         fw_handleEvents(framework);
-        double nextScheduledEvent = 
celix_framework_processScheduledEvents(framework);
-        celix_framework_waitForNextEvent(framework, nextScheduledEvent);
+        struct timespec nextDeadline = 
celix_framework_processScheduledEvents(framework);
+        celix_framework_waitForNextEvent(framework, nextDeadline);
 
         celixThreadMutex_lock(&framework->dispatcher.mutex);
         active = framework->dispatcher.active;
diff --git a/libs/utils/gtest/src/TimeUtilsTestSuite.cc 
b/libs/utils/gtest/src/TimeUtilsTestSuite.cc
index 2aff69a4..d175d7d6 100644
--- a/libs/utils/gtest/src/TimeUtilsTestSuite.cc
+++ b/libs/utils/gtest/src/TimeUtilsTestSuite.cc
@@ -110,3 +110,27 @@ TEST_F(TimeUtilsTestSuite, DelayedTimespecTest) {
     ASSERT_EQ(delayedTime.tv_sec, expectedTime.tv_sec);
     ASSERT_EQ(delayedTime.tv_nsec, expectedTime.tv_nsec);
 }
+
+TEST_F(TimeUtilsTestSuite, CompareTimeTest) {
+    struct timespec time1 = {0, 500000000};
+    struct timespec time2 = {0, 500000000};
+    ASSERT_EQ(celix_compareTime(&time1, &time2), 0);
+
+    time1 = {0, 500000000};
+    time2 = {0, 600000000};
+    ASSERT_EQ(celix_compareTime(&time1, &time2), -1); //time1 is before time2
+
+    time1 = {0, 600000000};
+    time2 = {0, 500000000};
+    ASSERT_EQ(celix_compareTime(&time1, &time2), 1); //time1 is after time2
+
+    time1 = {1, 500000000};
+    time2 = {0, 500000000};
+    ASSERT_EQ(celix_compareTime(&time1, &time2), 1); //time1 is after time2
+
+    time1 = {0, 500000000};
+    time2 = {1, 500000000};
+    ASSERT_EQ(celix_compareTime(&time1, &time2), -1); //time1 is before time2
+}
+
+
diff --git a/libs/utils/include/celix_utils.h b/libs/utils/include/celix_utils.h
index eab3484a..707637eb 100644
--- a/libs/utils/include/celix_utils.h
+++ b/libs/utils/include/celix_utils.h
@@ -172,6 +172,14 @@ CELIX_UTILS_EXPORT struct timespec 
celix_delayedTimespec(const struct timespec*
  */
 CELIX_UTILS_EXPORT double celix_elapsedtime(clockid_t clockId, struct timespec 
startTime);
 
+/**
+ * @brief Compare two time arguments.
+ * @param[in] a The first timespec.
+ * @param[in] b The second timespec.
+ * @return 0 if equal, -1 if a is before b and 1 if a is after b.
+ */
+CELIX_UTILS_EXPORT int celix_compareTime(const struct timespec* a, const 
struct timespec* b);
+
 /**
  * @brief Creates a hash from a string
  */
diff --git a/libs/utils/src/utils.c b/libs/utils/src/utils.c
index 24371d43..f80b3bd1 100644
--- a/libs/utils/src/utils.c
+++ b/libs/utils/src/utils.c
@@ -295,6 +295,17 @@ double celix_elapsedtime(clockid_t clockId, struct 
timespec startTime) {
     return celix_difftime(&startTime, &now);
 }
 
+int celix_compareTime(const struct timespec *a, const struct timespec *b) {
+    if (a->tv_sec == b->tv_sec && a->tv_nsec == b->tv_nsec) {
+        return 0;
+    }
+    double diff = celix_difftime(a, b);
+    if (diff < 0) {
+        return 1;
+    }
+    return -1;
+}
+
 char* celix_utils_strdup(const char *str) {
     if (str != NULL) {
         return strndup(str, CELIX_UTILS_MAX_STRLEN);

Reply via email to