remove scheduled functions complexity overhead, change recurrent api (#6214)

* remove scheduled functions complexity overhead, change recurrent functions api

cores/esp8266/Schedule.cpp

@@ -6,16 +6,11 @@
 #include "interrupts.h"
 #include "coredecls.h"
 
-typedef std::function<bool(void)> mFuncT;
-
+typedef std::function<void(void)> mSchedFuncT;
 struct scheduled_fn_t
 {
     scheduled_fn_t* mNext = nullptr;
-    mFuncT mFunc;
-    esp8266::polledTimeout::periodicFastUs callNow;
-    schedule_e policy;
-
-    scheduled_fn_t() : callNow(esp8266::polledTimeout::periodicFastUs::alwaysExpired) { }
+    mSchedFuncT mFunc;
 };
 
 static scheduled_fn_t* sFirst = nullptr;
@@ -23,8 +18,22 @@ static scheduled_fn_t* sLast = nullptr;
 static scheduled_fn_t* sUnused = nullptr;
 static int sCount = 0;
 
+typedef std::function<bool(void)> mRecFuncT;
+struct recurrent_fn_t
+{
+    recurrent_fn_t* mNext = nullptr;
+    mRecFuncT mFunc;
+    esp8266::polledTimeout::periodicFastUs callNow;
+    recurrent_fn_t (esp8266::polledTimeout::periodicFastUs interval): callNow(interval) { }
+};
+
+static recurrent_fn_t* rFirst = nullptr; // fifo not needed
+
+// Returns a pointer to an unused sched_fn_t,
+// or if none are available allocates a new one,
+// or nullptr if limit is reached
 IRAM_ATTR // called from ISR
-static scheduled_fn_t* get_fn_unsafe()
+static scheduled_fn_t* get_fn_unsafe ()
 {
     scheduled_fn_t* result = nullptr;
     // try to get an item from unused items list
@@ -33,18 +42,18 @@ static scheduled_fn_t* get_fn_unsafe()
         result = sUnused;
         sUnused = sUnused->mNext;
         result->mNext = nullptr;
-        result->callNow.reset(esp8266::polledTimeout::periodicFastUs::alwaysExpired);
     }
     // if no unused items, and count not too high, allocate a new one
     else if (sCount < SCHEDULED_FN_MAX_COUNT)
     {
-        result = new scheduled_fn_t;
-        ++sCount;
+        result = (scheduled_fn_t*)malloc(sizeof(scheduled_fn_t));
+        if (result)
+            ++sCount;
     }
     return result;
 }
 
-static void recycle_fn_unsafe(scheduled_fn_t* fn)
+static void recycle_fn_unsafe (scheduled_fn_t* fn)
 {
     fn->mFunc = nullptr; // special overload in c++ std lib
     fn->mNext = sUnused;
@@ -52,19 +61,14 @@ static void recycle_fn_unsafe(scheduled_fn_t* fn)
 }
 
 IRAM_ATTR // (not only) called from ISR
-bool schedule_function_us(std::function<bool(void)>&& fn, uint32_t repeat_us, schedule_e policy)
+bool schedule_function (const std::function<void(void)>& fn)
 {
-    assert(repeat_us < decltype(scheduled_fn_t::callNow)::neverExpires); //~26800000us (26.8s)
-
     esp8266::InterruptLock lockAllInterruptsInThisScope;
 
     scheduled_fn_t* item = get_fn_unsafe();
     if (!item)
         return false;
 
-    if (repeat_us)
-        item->callNow.reset(repeat_us);
-    item->policy = policy;
     item->mFunc = fn;
 
     if (sFirst)
@@ -76,25 +80,58 @@ bool schedule_function_us(std::function<bool(void)>&& fn, uint32_t repeat_us, sc
     return true;
 }
 
-IRAM_ATTR // (not only) called from ISR
-bool schedule_function_us(const std::function<bool(void)>& fn, uint32_t repeat_us, schedule_e policy)
+bool schedule_recurrent_function_us (const std::function<bool(void)>& fn, uint32_t repeat_us)
 {
-    return schedule_function_us(std::function<bool(void)>(fn), repeat_us, policy);
+    assert(repeat_us < decltype(recurrent_fn_t::callNow)::neverExpires); //~26800000us (26.8s)
+
+    esp8266::InterruptLock lockAllInterruptsInThisScope;
+
+    recurrent_fn_t* item = new recurrent_fn_t(repeat_us);
+    if (!item)
+        return false;
+
+    item->mFunc = fn;
+
+    if (rFirst)
+    {
+        item->mNext = rFirst;
+        rFirst = item;
+    }
+    else
+        rFirst = item;
+
+    return true;
 }
 
-IRAM_ATTR // called from ISR
-bool schedule_function(std::function<void(void)>&& fn, schedule_e policy)
+void run_scheduled_functions ()
 {
-    return schedule_function_us([fn]() { fn(); return false; }, 0, policy);
+    esp8266::polledTimeout::periodicFastMs yieldNow(100); // yield every 100ms
+
+    while (sFirst)
+    {
+        sFirst->mFunc();
+
+        {
+            esp8266::InterruptLock lockAllInterruptsInThisScope;
+
+            auto to_recycle = sFirst;
+            sFirst = sFirst->mNext;
+            if (!sFirst)
+                sLast = nullptr;
+            recycle_fn_unsafe(to_recycle);
+        }
+
+        if (yieldNow)
+        {
+            // because scheduled function are allowed to last:
+            // this is yield() in cont stack:
+            esp_schedule();
+            cont_yield(g_pcont);
+        }
+    }
 }
 
-IRAM_ATTR // called from ISR
-bool schedule_function(const std::function<void(void)>& fn, schedule_e policy)
-{
-    return schedule_function(std::function<void(void)>(fn), policy);
-}
-
-void run_scheduled_functions(schedule_e policy)
+void run_scheduled_recurrent_functions ()
 {
     // Note to the reader:
     // There is no exposed API to remove a scheduled function:
@@ -102,62 +139,51 @@ void run_scheduled_functions(schedule_e policy)
     // its purpose is that it is never called from an interrupt
     // (always on cont stack).
 
+    if (!rFirst)
+        return;
+
     static bool fence = false;
     {
-        esp8266::InterruptLock lockAllInterruptsInThisScope;
+        // fence is like a mutex but as we are never called from ISR,
+        // locking is useless here. Leaving comment for reference.
+        //esp8266::InterruptLock lockAllInterruptsInThisScope;
+
         if (fence)
             // prevent recursive calls from yield()
+            // (even if they are not allowed)
             return;
         fence = true;
     }
 
-    esp8266::polledTimeout::periodicFastMs yieldNow(100); // yield every 100ms
-    scheduled_fn_t* lastRecurring = nullptr;
-    scheduled_fn_t* nextCall = sFirst;
-    while (nextCall)
-    {
-        scheduled_fn_t* toCall = nextCall;
-        nextCall = nextCall->mNext;
+    recurrent_fn_t* prev = nullptr;
+    recurrent_fn_t* current = rFirst;
 
-        // run scheduled function:
-        // - when its schedule policy allows it anytime
-        // - or if we are called at loop() time
-        // and
-        // - its time policy allows it
-        if (   (   toCall->policy == SCHEDULED_FUNCTION_WITHOUT_YIELDELAYCALLS
-                || policy == SCHEDULED_FUNCTION_ONCE_PER_LOOP)
-            && toCall->callNow)
+    while (current)
+    {
+        if (current->callNow && !current->mFunc())
         {
-            if (toCall->mFunc())
+            // remove function from stack
+            esp8266::InterruptLock lockAllInterruptsInThisScope;
+
+            auto to_ditch = current;
+
+            if (prev)
             {
-                // function stays in list
-                lastRecurring = toCall;
+                current = current->mNext;
+                prev->mNext = current;
             }
             else
             {
-                // function removed from list
-                esp8266::InterruptLock lockAllInterruptsInThisScope;
-
-                if (sFirst == toCall)
-                    sFirst = sFirst->mNext;
-                else if (lastRecurring)
-                    lastRecurring->mNext = toCall->mNext;
-
-                if (sLast == toCall)
-                    sLast = lastRecurring;
-
-                recycle_fn_unsafe(toCall);
+                rFirst = rFirst->mNext;
+                current = rFirst;
             }
+
+            delete(to_ditch);
         }
         else
-            // function stays in list
-            lastRecurring = toCall;
-
-        if (policy == SCHEDULED_FUNCTION_ONCE_PER_LOOP && yieldNow)
         {
-            // this is yield() in cont stack:
-            esp_schedule();
-            cont_yield(g_pcont);
+            prev = current;
+            current = current->mNext;
         }
     }
 

cores/esp8266/Schedule.h

@@ -3,56 +3,47 @@
 
 #include <functional>
 
-// This API is stabilizing
-// Function signatures may change, internal queue will remain FIFO.
-//
-// * Add the given lambda to a fifo list of lambdas, which is run when
-//   - `loop` function returns,
-//   - or `yield` is called,
-//   - or `run_scheduled_functions` is called.
-//
-// * Use lambdas to pass arguments to a function, or call a class/static
-//   member function.
-//
-// * Please ensure variables or instances used from inside lambda will exist
-//   when lambda is later called
-//
-// * There is no mechanism for cancelling scheduled functions.
-//
-// * `yield` can be called from inside lambdas
-//
-// * Returns false if the number of scheduled functions exceeds
-//   SCHEDULED_FN_MAX_COUNT.
-
 #define SCHEDULED_FN_MAX_COUNT 32
 
-enum schedule_e
-{
-    SCHEDULED_FUNCTION_ONCE_PER_LOOP,
-    SCHEDULED_FUNCTION_WITHOUT_YIELDELAYCALLS
-};
+// scheduled functions called once:
+//
+// * internal queue is FIFO.
+// * Add the given lambda to a fifo list of lambdas, which is run when
+//   `loop` function returns.
+// * Use lambdas to pass arguments to a function, or call a class/static
+//   member function.
+// * Please ensure variables or instances used from inside lambda will exist
+//   when lambda is later called.
+// * There is no mechanism for cancelling scheduled functions.
+// * `yield` can be called from inside lambdas.
+// * Returns false if the number of scheduled functions exceeds
+//   SCHEDULED_FN_MAX_COUNT.
+// * Run the lambda only once next time.
 
-// * Run the lambda only once next time
-bool schedule_function(std::function<void(void)>&& fn,
-                       schedule_e policy = SCHEDULED_FUNCTION_ONCE_PER_LOOP);
-bool schedule_function(const std::function<void(void)>& fn,
-                       schedule_e policy = SCHEDULED_FUNCTION_ONCE_PER_LOOP);
+bool schedule_function (const std::function<void(void)>& fn);
 
+// Run all scheduled functions.
+// Use this function if your are not using `loop`, or `loop` does not return
+// on a regular basis.
+
+void run_scheduled_functions();
+
+// recurrent scheduled function:
+//
+// * internal queue if not FIFO.
 // * Run the lambda periodically about every <repeat_us> microseconds until
 //   it returns false.
 // * Note that it may be more than <repeat_us> microseconds between calls if
 //   `yield` is not called frequently, and therefore should not be used for
 //   timing critical operations.
-bool schedule_function_us(std::function<bool(void)>&& fn,
-                          uint32_t repeat_us,
-                          schedule_e policy = SCHEDULED_FUNCTION_ONCE_PER_LOOP);
-bool schedule_function_us(const std::function<bool(void)>& fn,
-                          uint32_t repeat_us,
-                          schedule_e policy = SCHEDULED_FUNCTION_ONCE_PER_LOOP);
+// * There is no mechanism for cancelling recurrent scheduled functions.
+// * long running operations or yield() or delay() are not wise in the lambda.
 
-// Run all scheduled functions.
-// Use this function if your are not using `loop`, or `loop` does not return
-// on a regular basis.
-void run_scheduled_functions(schedule_e policy = SCHEDULED_FUNCTION_ONCE_PER_LOOP);
+bool schedule_recurrent_function_us (const std::function<bool(void)>& fn, uint32_t repeat_us);
 
-#endif //ESP_SCHEDULE_H
+// Test recurrence and run recurrent scheduled functions.
+// (internally called at every `yield()` and `loop()`)
+
+void run_scheduled_recurrent_functions ();
+
+#endif // ESP_SCHEDULE_H

cores/esp8266/core_esp8266_main.cpp

@@ -87,7 +87,7 @@ void preloop_update_frequency() {
 static inline void esp_yield_within_cont() __attribute__((always_inline));
 static void esp_yield_within_cont() {
         cont_yield(g_pcont);
-        run_scheduled_functions(SCHEDULED_FUNCTION_WITHOUT_YIELDELAYCALLS);
+        run_scheduled_recurrent_functions();
 }
 
 extern "C" void esp_yield() {
@@ -129,7 +129,8 @@ static void loop_wrapper() {
         setup_done = true;
     }
     loop();
-    run_scheduled_functions(SCHEDULED_FUNCTION_ONCE_PER_LOOP);
+    run_scheduled_functions();
+    run_scheduled_recurrent_functions();
     esp_schedule();
 }