From dd6b02d555ddfea40249dbca3545641e5531ff78 Mon Sep 17 00:00:00 2001 From: Matthew Kennedy Date: Mon, 5 Oct 2020 05:57:00 -0700 Subject: [PATCH] we don't need two loops (#1855) --- .../controllers/system/timer/event_queue.cpp | 78 ++++++++++--------- .../controllers/system/timer/event_queue.h | 1 + .../system/timer/single_timer_executor.cpp | 19 ++--- .../controllers/trigger/trigger_central.cpp | 2 - 4 files changed, 50 insertions(+), 50 deletions(-) diff --git a/firmware/controllers/system/timer/event_queue.cpp b/firmware/controllers/system/timer/event_queue.cpp index 7275fc8d77..ad59c547c0 100644 --- a/firmware/controllers/system/timer/event_queue.cpp +++ b/firmware/controllers/system/timer/event_queue.cpp @@ -120,57 +120,63 @@ int EventQueue::executeAll(efitime_t now) { assertListIsSorted(); #endif - while (true) { - // Read the head every time - a previously executed event could - // have inserted something new at the head - scheduling_s* current = head; + bool didExecute; + do { + didExecute = executeOne(now); + executionCounter += didExecute ? 1 : 0; + } while (didExecute); - // Queue is empty - bail - if (!current) { - break; - } + return executionCounter; +} - // If the next event is far in the future, we'll reschedule - // and execute it next time. - // We do this when the next event is close enough that the overhead of - // resetting the timer and scheduling an new interrupt is greater than just - // waiting for the time to arrive. On current CPUs, this is reasonable to set - // around 10 microseconds. - if (current->momentX > now + lateDelay) { - break; - } +bool EventQueue::executeOne(efitime_t now) { + // Read the head every time - a previously executed event could + // have inserted something new at the head + scheduling_s* current = head; - // near future - spin wait for the event to happen and avoid the - // overhead of rescheduling the timer. - // yes, that's a busy wait but that's what we need here - while (current->momentX > getTimeNowNt()) { - UNIT_TEST_BUSY_WAIT_CALLBACK(); - } + // Queue is empty - bail + if (!current) { + return false; + } - executionCounter++; + // If the next event is far in the future, we'll reschedule + // and execute it next time. + // We do this when the next event is close enough that the overhead of + // resetting the timer and scheduling an new interrupt is greater than just + // waiting for the time to arrive. On current CPUs, this is reasonable to set + // around 10 microseconds. + if (current->momentX > now + lateDelay) { + return false; + } - // step the head forward, unlink this element, clear scheduled flag - head = current->nextScheduling_s; - current->nextScheduling_s = nullptr; - current->isScheduled = false; + // near future - spin wait for the event to happen and avoid the + // overhead of rescheduling the timer. + // yes, that's a busy wait but that's what we need here + while (current->momentX > getTimeNowNt()) { + UNIT_TEST_BUSY_WAIT_CALLBACK(); + } + + // step the head forward, unlink this element, clear scheduled flag + head = current->nextScheduling_s; + current->nextScheduling_s = nullptr; + current->isScheduled = false; #if EFI_UNIT_TEST - printf("QUEUE: execute current=%d param=%d\r\n", (long)current, (long)current->action.getArgument()); + printf("QUEUE: execute current=%d param=%d\r\n", (long)current, (long)current->action.getArgument()); #endif - // Execute the current element - { - ScopePerf perf2(PE::EventQueueExecuteCallback); - current->action.execute(); - } + // Execute the current element + { + ScopePerf perf2(PE::EventQueueExecuteCallback); + current->action.execute(); + } #if EFI_UNIT_TEST // (tests only) Ensure we didn't break anything assertListIsSorted(); #endif - } - return executionCounter; + return true; } int EventQueue::size(void) const { diff --git a/firmware/controllers/system/timer/event_queue.h b/firmware/controllers/system/timer/event_queue.h index f16939e7e9..88c1c7657e 100644 --- a/firmware/controllers/system/timer/event_queue.h +++ b/firmware/controllers/system/timer/event_queue.h @@ -56,6 +56,7 @@ public: bool insertTask(scheduling_s *scheduling, efitime_t timeX, action_s action); int executeAll(efitime_t now); + bool executeOne(efitime_t now); efitime_t getNextEventTime(efitime_t nowUs) const; void clear(void); diff --git a/firmware/controllers/system/timer/single_timer_executor.cpp b/firmware/controllers/system/timer/single_timer_executor.cpp index 89a99dfbc3..e4fd913d9e 100644 --- a/firmware/controllers/system/timer/single_timer_executor.cpp +++ b/firmware/controllers/system/timer/single_timer_executor.cpp @@ -41,7 +41,6 @@ EXTERN_ENGINE; static efitime_t nextEventTimeNt = 0; uint32_t hwSetTimerDuration; -uint32_t lastExecutionCount; void globalTimerCallback() { efiAssertVoid(CUSTOM_ERR_6624, getCurrentRemainingStack() > EXPECTED_REMAINING_STACK, "lowstck#2y"); @@ -131,16 +130,13 @@ void SingleTimerExecutor::executeAllPendingActions() { * next listeners. * TODO: add a counter & figure out a limit of iterations? */ - int totalExecuted = 0; - while (shouldExecute > 0) { - /** - * It's worth noting that that the actions might be adding new actions into the queue - */ + + bool didExecute; + do { efitick_t nowNt = getTimeNowNt(); - shouldExecute = queue.executeAll(nowNt); - totalExecuted += shouldExecute; - } - lastExecutionCount = totalExecuted; + didExecute = queue.executeOne(nowNt); + } while (didExecute); + if (!isLocked()) { firmwareError(CUSTOM_ERR_LOCK_ISSUE, "Someone has stolen my lock"); return; @@ -162,10 +158,9 @@ void SingleTimerExecutor::scheduleTimerCallback() { efiAssertVoid(CUSTOM_ERR_6625, nextEventTimeNt > nowNt, "setTimer constraint"); if (nextEventTimeNt == EMPTY_QUEUE) return; // no pending events in the queue + int32_t hwAlarmTime = NT2US((int32_t)nextEventTimeNt - (int32_t)nowNt); - uint32_t beforeHwSetTimer = getTimeNowLowerNt(); setHardwareUsTimer(hwAlarmTime == 0 ? 1 : hwAlarmTime); - hwSetTimerDuration = getTimeNowLowerNt() - beforeHwSetTimer; } void initSingleTimerExecutorHardware(void) { diff --git a/firmware/controllers/trigger/trigger_central.cpp b/firmware/controllers/trigger/trigger_central.cpp index 0184b704a2..c9b9cd5416 100644 --- a/firmware/controllers/trigger/trigger_central.cpp +++ b/firmware/controllers/trigger/trigger_central.cpp @@ -603,7 +603,6 @@ extern PwmConfig triggerSignal; #endif /* #if EFI_PROD_CODE */ extern uint32_t hipLastExecutionCount; -extern uint32_t hwSetTimerDuration; extern uint32_t maxLockedDuration; extern uint32_t maxEventCallbackDuration; @@ -737,7 +736,6 @@ void triggerInfo(void) { #if EFI_HIP_9011 scheduleMsg(logger, "hipLastExecutionCount=%d", hipLastExecutionCount); #endif /* EFI_HIP_9011 */ - scheduleMsg(logger, "hwSetTimerDuration=%d", hwSetTimerDuration); scheduleMsg(logger, "totalTriggerHandlerMaxTime=%d", triggerMaxDuration); scheduleMsg(logger, "maxPrecisionCallbackDuration=%d", maxPrecisionCallbackDuration);