[568902] reworked timer handler to reduce required mutex locks

With this change the timer handler only requires to acquire a mutex in
its main loop when a new timed FB is added or removed. This reduces the
computation time of the timerhandler and should bring less jitter to
timed fbs.

Bug: https://bugs.eclipse.org/bugs/show_bug.cgi?id=568902
Change-Id: If28d4a08a2a7f3ebafb6c6e8ad337a0e02e610cd
Signed-off-by: Alois Zoitl <alois.zoitl@gmx.at>
diff --git a/src/arch/timerha.cpp b/src/arch/timerha.cpp
index a267050..51b2035 100644
--- a/src/arch/timerha.cpp
+++ b/src/arch/timerha.cpp
@@ -18,6 +18,8 @@
 #include "../core/devexec.h"
 #include "../core/esfb.h"
 #include "../core/utils/criticalregion.h"
+#include <algorithm>
+#include <functional>
 
 DEFINE_HANDLER(CTimerHandler)
 
@@ -38,8 +40,9 @@
   // set the first next activation time right here to reduce jitter, see Bug #568902 for details
   paTimerListEntry->mTimeOut = mForteTime + paTimerListEntry->mInterval;
   {
-    CCriticalRegion criticalRegion(mSync);
-    addTimedFBEntry(paTimerListEntry);
+    CCriticalRegion criticalRegion(mAddListSync);
+    paTimerListEntry->mNext = mAddFBList;
+    mAddFBList = paTimerListEntry;
   }
 }
 
@@ -67,7 +70,11 @@
 }
 
 void CTimerHandler::unregisterTimedFB(CEventSourceFB *paTimedFB) {
-  CCriticalRegion criticalRegion(mSync);
+  CCriticalRegion criticalRegion(mRemoveListSync);
+  mRemoveFBList.push_back(paTimedFB);
+}
+
+void CTimerHandler::removeTimedFB(CEventSourceFB *paTimedFB) {
   if (0 != mTimedFBList) {
     STimedFBListEntry *buffer = 0;
     if (mTimedFBList->mTimedFB == paTimedFB) {
@@ -94,30 +101,65 @@
 void CTimerHandler::nextTick(void) {
   ++mForteTime;
   mDeviceExecution.notifyTime(mForteTime); //notify the device execution that one tick passed by.
-  if(0 != mTimedFBList){
-    //only check the list if there are entries in the list
-    CCriticalRegion criticalRegion(mSync);
-    while (0 != mTimedFBList) {
-      if (mTimedFBList->mTimeOut > mForteTime) {
-        break;
-      }
-      mDeviceExecution.startNewEventChain(mTimedFBList->mTimedFB);
-      STimedFBListEntry *buffer = mTimedFBList;
-      mTimedFBList = mTimedFBList->mNext;
 
-      switch (buffer->mType) {
-        case e_Periodic:
-          buffer->mTimeOut = mForteTime + buffer->mInterval;  // the next activation time of this FB
-          addTimedFBEntry(buffer); //re-register the timed FB
-          break;
-        case e_SingleShot:
-          // nothing special is to do up to now
-        default:
-          buffer->mNext = 0;
-          buffer->mTimeOut = 0;
-          break;
-      }
+  if(!mRemoveFBList.empty()){
+    processRemoveList();
+  }
+
+  processTimedFBList();
+
+  if(0 != mAddFBList){
+    processAddList();
+  }
+}
+
+void  CTimerHandler::processTimedFBList(){
+  while (0 != mTimedFBList) {
+    if (mTimedFBList->mTimeOut > mForteTime) {
+      break;
+    }
+    STimedFBListEntry *buffer = mTimedFBList;
+    mTimedFBList = buffer->mNext;  //remove buffer from the list
+    triggerTimedFB(buffer);
+  }
+}
+
+void CTimerHandler::triggerTimedFB(STimedFBListEntry *paTimerListEntry){
+  mDeviceExecution.startNewEventChain(paTimerListEntry->mTimedFB);
+
+  switch (paTimerListEntry->mType) {
+    case e_Periodic:
+      paTimerListEntry->mTimeOut = mForteTime + paTimerListEntry->mInterval;  // the next activation time of this FB
+      addTimedFBEntry(paTimerListEntry); //re-register the timed FB
+      break;
+    case e_SingleShot:
+      // nothing special is to do up to now, therefore go to default
+    default:
+      paTimerListEntry->mNext = 0;
+      paTimerListEntry->mTimeOut = 0;
+      break;
+  }
+}
+
+void CTimerHandler::processAddList(){
+  CCriticalRegion criticalRegion(mAddListSync);
+  while(0 != mAddFBList){
+    STimedFBListEntry *buffer = mAddFBList;
+    mAddFBList = buffer->mNext; //remove buffer from the list
+    if(buffer->mTimeOut < mForteTime){
+      // the time already passed trigger the fb
+      triggerTimedFB(buffer);
+    }
+    else{
+      addTimedFBEntry(buffer);
     }
   }
 }
 
+void CTimerHandler::processRemoveList(){
+  CCriticalRegion criticalRegion(mRemoveListSync);
+  std::for_each(mRemoveFBList.begin(), mRemoveFBList.end(), std::bind1st(std::mem_fun(&CTimerHandler::removeTimedFB), this));
+  mRemoveFBList.clear();
+}
+
+
diff --git a/src/arch/timerha.h b/src/arch/timerha.h
index 2f471d5..6b380c1 100644
--- a/src/arch/timerha.h
+++ b/src/arch/timerha.h
@@ -1,14 +1,18 @@
 /*******************************************************************************

  * Copyright (c) 2005 - 2015 ACIN, Profactor GmbH, fortiss GmbH

- * This program and the accompanying materials are made available under the
- * terms of the Eclipse Public License 2.0 which is available at
- * http://www.eclipse.org/legal/epl-2.0.
- *
+ *               2020 Johannes Kepler University Linz

+ *

+ * This program and the accompanying materials are made available under the

+ * terms of the Eclipse Public License 2.0 which is available at

+ * http://www.eclipse.org/legal/epl-2.0.

+ *

  * SPDX-License-Identifier: EPL-2.0

  *

  * Contributors:

  *   Alois Zoitl, Thomas Strasser, Rene Smodic, Monika Wenger, Ingo Hegny

  *    - initial API and implementation and/or initial documentation

+ *   Alois Zoitl - worked on reducing the jitter and overhead of timer handler

+ *                 Bug #568902

  *******************************************************************************/

 #ifndef _TIMERHA_H_

 #define _TIMERHA_H_

@@ -16,6 +20,7 @@
 #include <forte_config.h>

 #include "../core/extevhan.h"

 #include <forte_sync.h>

+#include <vector>

 

 class CEventSourceFB;

 class CIEC_TIME;

@@ -37,7 +42,8 @@
  *  \ingroup EXTEVHAND

  */

 class CTimerHandler : public CExternalEventHandler{

-  DECLARE_HANDLER(CTimerHandler);

+  DECLARE_HANDLER(CTimerHandler)

+    ;

   public:

 

     /*!\brief create the timer handler and set the parameter pointer with the the new timer handler.

@@ -45,7 +51,7 @@
      * This function is not implemented in the standardtimerhandler and has to be implemented in the specific implementation.

      * implementations should check that not two timerhanlders can be created.

      */

-    static CTimerHandler* createTimerHandler(CDeviceExecution& paDeviceExecution);

+    static CTimerHandler* createTimerHandler(CDeviceExecution &paDeviceExecution);

 

     /*!\brief Sets the priority of the event source

      *

@@ -87,18 +93,34 @@
       return mForteTime;

     }

 

-  protected:

-    CSyncObject mSync;

-

   private:

     //!Add an entry to the timed list.

     void addTimedFBEntry(STimedFBListEntry *paTimerListEntry);

 

+    void processTimedFBList();

+    void processAddList();

+    void processRemoveList();

+

+    //!Remove an entry from the timed list.

+    void removeTimedFB(CEventSourceFB *paTimedFB);

+

+    //! process one timed FB entry, trigger the external event and if needed readd into the list.

+    void triggerTimedFB(STimedFBListEntry *paTimerListEntry);

+

     //!The runtime time in ticks till the start of FORTE.

     uint_fast64_t mForteTime;

 

     //! List of function blocks currently registered to the timer handler

     STimedFBListEntry *mTimedFBList;

+

+    //! List of function blocks to be added to the timer handler

+    STimedFBListEntry *mAddFBList;

+    CSyncObject mAddListSync;

+

+    //! List of function blocks to be removed from the timer handler

+    std::vector<CEventSourceFB *> mRemoveFBList;

+    CSyncObject mRemoveListSync;

+

 };

 

 #endif /*TIMERHA_H_*/