Allow scheduling_s to come from a pool (#4841)

* injection events use scheduling pool

* knock and prime

* bench test

* dropped this: *

* extract action before execute

* comment

* init allocated timers

* metrics

* dropped this: ,

* guard

* injection events use scheduling pool

* knock and prime

* bench test

* dropped this: *

* extract action before execute

* comment

* init allocated timers

* metrics

* dropped this: ,

* guard

Co-authored-by: Matthew Kennedy <makenne@microsoft.com>

firmware/console/binary/output_channels.txt

@@ -365,5 +365,7 @@ float mapFast
 	uint16_t autoscale rawMaf2;;"V",{1/@@PACK_MULT_VOLTAGE@@}, 0, 0, 5, 3
 	uint16_t autoscale mafMeasured2;@@GAUGE_NAME_AIR_FLOW_MEASURED_2@@;"kg/h",{1/@@PACK_MULT_MASS_FLOW@@}, 0, 0, 0, 1
 
-	uint8_t[138 iterate] unusedAtTheEnd;;"",1, 0, 0, 0, 0
+	uint16_t schedulingUsedCount;;"",1,0,0,0,0
+
+	uint8_t[136 iterate] unusedAtTheEnd;;"",1, 0, 0, 0, 0
 end_struct

firmware/controllers/bench_test.cpp

@@ -57,9 +57,6 @@ bool isRunningBenchTest(void) {
 	return isRunningBench;
 }
 
-static scheduling_s benchSchedStart;
-static scheduling_s benchSchedEnd;
-
 static void benchOn(OutputPin* output) {
 	output->setValue(true);
 }
@@ -105,8 +102,8 @@ static void runBench(brain_pin_e brainPin, OutputPin *output, float startDelayMs
 		efitick_t endTime = startTime + US2NT(onTimeUs);
 
 		// Schedule both events
-		engine->executor.scheduleByTimestampNt("bstart", &benchSchedStart, startTime, {benchOn, output});
+		engine->executor.scheduleByTimestampNt("bstart", nullptr, startTime, {benchOn, output});
-		engine->executor.scheduleByTimestampNt("bend", &benchSchedEnd, endTime, {benchOff, output});
+		engine->executor.scheduleByTimestampNt("bend", nullptr, endTime, {benchOff, output});
 
 		// Wait one full cycle time for the event + delay to happen
 		chThdSleepMicroseconds(onTimeUs + offTimeUs);

firmware/controllers/engine_cycle/fuel_schedule.h

@@ -38,9 +38,6 @@ public:
 
 	float injectionStartAngle = 0;
 
-	scheduling_s signalTimerUp;
-	scheduling_s endOfInjectionEvent;
-
 	/**
 	 * we need atomic flag so that we do not schedule a new pair of up/down before previous down was executed.
 	 *

firmware/controllers/engine_cycle/knock_controller.cpp

@@ -155,13 +155,11 @@ static void startKnockSampling(Engine* engine) {
 	onStartKnockSampling(cylinderNumberCopy, samplingSeconds, channel);
 }
 
-static scheduling_s startSampling;
-
 void Engine::onSparkFireKnockSense(uint8_t cylinderNumber, efitick_t nowNt) {
 	cylinderNumberCopy = cylinderNumber;
 
 #if EFI_HIP_9011 || EFI_SOFTWARE_KNOCK
-	scheduleByAngle(&startSampling, nowNt,
+	scheduleByAngle(nullptr, nowNt,
 			/*angle*/engineConfiguration->knockDetectionWindowStart, { startKnockSampling, engine });
 #else
 	UNUSED(nowNt);

firmware/controllers/engine_cycle/main_trigger_callback.cpp

@@ -155,7 +155,7 @@ void InjectionEvent::onTriggerTooth(int rpm, efitick_t nowNt, float currentPhase
 	}
 #endif /*EFI_PRINTF_FUEL_DETAILS */
 
-	if (isScheduled) {
+if (isScheduled) {
 #if EFI_PRINTF_FUEL_DETAILS
 		if (printFuelDebug) {
 			InjectorOutputPin *output = outputs[0];
@@ -183,9 +183,9 @@ void InjectionEvent::onTriggerTooth(int rpm, efitick_t nowNt, float currentPhase
 		angleFromNow += getEngineState()->engineCycle;
 	}
 
-	efitick_t startTime = scheduleByAngle(&signalTimerUp, nowNt, angleFromNow, startAction);
+	efitick_t startTime = scheduleByAngle(nullptr, nowNt, angleFromNow, startAction);
 	efitick_t turnOffTime = startTime + US2NT((int)durationUs);
-	getExecutorInterface()->scheduleByTimestampNt("inj", &endOfInjectionEvent, turnOffTime, endAction);
+	getExecutorInterface()->scheduleByTimestampNt("inj", nullptr, turnOffTime, endAction);
 
 #if EFI_UNIT_TEST
 		printf("scheduling injection angle=%.2f/delay=%.2f injectionDuration=%.2f\r\n", angleFromNow, NT2US(startTime - nowNt), injectionDuration);

firmware/controllers/engine_cycle/prime_injection.cpp

@@ -60,7 +60,7 @@ void PrimeController::onIgnitionStateChanged(bool ignitionOn) {
 		auto primeDelayMs = engineConfiguration->primingDelay * 1000;
 
 		auto startTime = getTimeNowNt() + MS2NT(primeDelayMs);
-		getExecutorInterface()->scheduleByTimestampNt("prime", &m_start, startTime, { PrimeController::onPrimeStartAdapter, this });
+		getExecutorInterface()->scheduleByTimestampNt("prime", nullptr, startTime, { PrimeController::onPrimeStartAdapter, this });
 	} else {
 		efiPrintf("Skipped priming pulse since ignSwitchCounter = %d", ignSwitchCounter);
 	}
@@ -101,7 +101,7 @@ void PrimeController::onPrimeStart() {
 	// Open all injectors, schedule closing later
 	m_isPriming = true;
 	startSimultaneousInjection();
-	getExecutorInterface()->scheduleByTimestampNt("prime", &m_end, endTime, { onPrimeEndAdapter, this });
+	getExecutorInterface()->scheduleByTimestampNt("prime", nullptr, endTime, { onPrimeEndAdapter, this });
 }
 
 void PrimeController::onPrimeEnd() {

firmware/controllers/engine_cycle/prime_injection.h

@@ -24,9 +24,6 @@ public:
 	}
 
 private:
-	scheduling_s m_start;
-	scheduling_s m_end;
-
 	bool m_isPriming = false;
 
 	static void onPrimeStartAdapter(PrimeController* instance) {

firmware/controllers/system/timer/event_queue.cpp

@@ -21,6 +21,40 @@ extern int timeNowUs;
 extern bool verboseMode;
 #endif /* EFI_UNIT_TEST */
 
+EventQueue::EventQueue(efitick_t lateDelay)
+	: lateDelay(lateDelay)
+{
+	for (size_t i = 0; i < efi::size(m_pool); i++) {
+		tryReturnScheduling(&m_pool[i]);
+	}
+}
+
+scheduling_s* EventQueue::getFreeScheduling() {
+	auto retVal = m_freelist;
+
+	if (retVal) {
+		m_freelist = retVal->nextScheduling_s;
+		retVal->nextScheduling_s = nullptr;
+
+#if EFI_PROD_CODE
+		getTunerStudioOutputChannels()->schedulingUsedCount++;
+#endif
+	}
+
+	return retVal;
+}
+
+void EventQueue::tryReturnScheduling(scheduling_s* sched) {
+	// Only return this scheduling to the free list if it's from the correct pool
+	if (sched >= &m_pool[0] && sched <= &m_pool[efi::size(m_pool) - 1]) {
+		sched->nextScheduling_s = m_freelist;
+		m_freelist = sched;
+
+#if EFI_PROD_CODE
+		getTunerStudioOutputChannels()->schedulingUsedCount--;
+#endif
+	}
+}
 
 /**
  * @return true if inserted into the head of the list
@@ -28,6 +62,17 @@ extern bool verboseMode;
 bool EventQueue::insertTask(scheduling_s *scheduling, efitick_t timeX, action_s action) {
 	ScopePerf perf(PE::EventQueueInsertTask);
 
+	if (!scheduling) {
+		scheduling = getFreeScheduling();
+
+		// If still null, the free list is empty and all schedulings in the pool have been expended.
+		if (!scheduling) {
+			// TODO: should we warn or error here?
+
+			return false;
+		}
+	}
+
 #if EFI_UNIT_TEST
 	assertListIsSorted();
 #endif /* EFI_UNIT_TEST */
@@ -210,6 +255,9 @@ bool EventQueue::executeOne(efitick_t now) {
 	auto action = current->action;
 	current->action = {};
 
+	tryReturnScheduling(current);
+	current = nullptr;
+
 #if EFI_UNIT_TEST
 	printf("QUEUE: execute current=%d param=%d\r\n", (uintptr_t)current, (uintptr_t)action.getArgument());
 #endif

firmware/controllers/system/timer/event_queue.h

@@ -44,7 +44,7 @@ public:
 	// See comment in EventQueue::executeAll for info about lateDelay - it sets the 
 	// time gap between events for which we will wait instead of rescheduling the next
 	// event in a group of events near one another.
-	EventQueue(efitick_t lateDelay = 0) : lateDelay(lateDelay) {}
+	explicit EventQueue(efitick_t lateDelay = 0);
 
 	/**
 	 * O(size) - linear search in sorted linked list
@@ -61,11 +61,17 @@ public:
 	scheduling_s *getElementAtIndexForUnitText(int index);
 	scheduling_s * getHead();
 	void assertListIsSorted() const;
+
+	scheduling_s* getFreeScheduling();
+	void tryReturnScheduling(scheduling_s* sched);
 private:
 	/**
 	 * this list is sorted
 	 */
 	scheduling_s *head = nullptr;
 	const efitick_t lateDelay;
+
+	scheduling_s* m_freelist = nullptr; 
+	scheduling_s m_pool[64];
 };
 

firmware/controllers/system/timer/scheduler.h

@@ -26,6 +26,7 @@ public:
 	schfunc_t getCallback() const;
 	void * getArgument() const;
 
+	// Actions with a callback set are truthy, all others are falsy
 	operator bool() const {
 		return callback != nullptr;
 	}

firmware/controllers/system/timer/signal_executor_sleep.cpp

@@ -33,6 +33,12 @@ bool printSchedulerDebug = true;
 
 #if EFI_SIGNAL_EXECUTOR_SLEEP
 
+struct CallbackContext
+{
+	scheduling_s* scheduling = nullptr;
+	bool shouldFree = false;
+};
+
 void SleepExecutor::scheduleByTimestamp(const char *msg, scheduling_s *scheduling, efitimeus_t timeUs, action_s action) {
 	scheduleForLater(msg, scheduling, timeUs - getTimeNowUs(), action);
 }
@@ -41,18 +47,30 @@ void SleepExecutor::scheduleByTimestampNt(const char *msg, scheduling_s* schedul
 	scheduleByTimestamp(msg, scheduling, NT2US(timeNt), action);
 }
 
-static void timerCallback(scheduling_s *scheduling) {
+static void timerCallback(CallbackContext* ctx) {
 #if EFI_PRINTF_FUEL_DETAILS
 	if (printSchedulerDebug) {
-		if (scheduling->action.getCallback() == (schfunc_t)&turnInjectionPinLow) {
+		if (ctx->scheduling->action.getCallback() == (schfunc_t)&turnInjectionPinLow) {
-			printf("executing cb=turnInjectionPinLow p=%d sch=%d now=%d\r\n", (int)scheduling->action.getArgument(), (int)scheduling,
+			printf("executing cb=turnInjectionPinLow p=%d sch=%d now=%d\r\n", (int)ctx->scheduling->action.getArgument(), (int)scheduling,
 				(int)getTimeNowUs());
 		} else {
 //		printf("exec cb=%d p=%d\r\n", (int)scheduling->callback, (int)scheduling->param);
 		}
 	}
 #endif // EFI_PRINTF_FUEL_DETAILS
-	scheduling->action.execute();
+
+	// Grab the action but clear it in the event so we can reschedule from the action's execution
+	action_s action = ctx->scheduling->action;
+	ctx->scheduling->action = {};
+
+	// Clean up any memory we allocated
+	if (ctx->shouldFree) {
+		delete ctx->scheduling;
+	}
+	delete ctx;
+
+	// Lastly, actually execute the action
+	action.execute();
 }
 
 static void doScheduleForLater(scheduling_s *scheduling, int delayUs, action_s action) {
@@ -67,6 +85,14 @@ static void doScheduleForLater(scheduling_s *scheduling, int delayUs, action_s a
 
 	chibios_rt::CriticalSectionLocker csl;
 
+	auto ctx = new CallbackContext;
+	if (!scheduling) {
+		scheduling = new scheduling_s;
+		chVTObjectInit(&scheduling->timer);
+		ctx->shouldFree = true;
+	}
+	ctx->scheduling = scheduling;
+
 	scheduling->action = action;
 	int isArmed = chVTIsArmedI(&scheduling->timer);
 	if (isArmed) {
@@ -84,7 +110,7 @@ static void doScheduleForLater(scheduling_s *scheduling, int delayUs, action_s a
 	}
 #endif /* EFI_SIMULATOR */
 
-	chVTSetI(&scheduling->timer, delaySt, (vtfunc_t)timerCallback, scheduling);
+	chVTSetI(&scheduling->timer, delaySt, (vtfunc_t)timerCallback, ctx);
 }
 
 void SleepExecutor::scheduleForLater(const char *msg, scheduling_s *scheduling, int delayUs, action_s action) {

unit_tests/tests/trigger/test_injection_scheduling.cpp

@@ -33,9 +33,9 @@ TEST(injectionScheduling, InjectionIsScheduled) {
 		// rising edge 5 degrees from now
 		float nt5deg = USF2NT(engine->rpmCalculator.oneDegreeUs * 5);
 		efitick_t startTime = nowNt + nt5deg;
-		EXPECT_CALL(mockExec, scheduleByTimestampNt(testing::NotNull(), &event.signalTimerUp, startTime, _));
+		EXPECT_CALL(mockExec, scheduleByTimestampNt(testing::NotNull(), _, startTime, _));
 		// falling edge 20ms later
-		EXPECT_CALL(mockExec, scheduleByTimestampNt(testing::NotNull(), &event.endOfInjectionEvent, startTime + MS2NT(20), _));
+		EXPECT_CALL(mockExec, scheduleByTimestampNt(testing::NotNull(), _, startTime + MS2NT(20), _));
 	}
 
 	
@@ -73,9 +73,9 @@ TEST(injectionScheduling, InjectionIsScheduledBeforeWraparound) {
 		// rising edge 5 degrees from now
 		float nt5deg = USF2NT(engine->rpmCalculator.oneDegreeUs * 5);
 		efitick_t startTime = nowNt + nt5deg;
-		EXPECT_CALL(mockExec, scheduleByTimestampNt(testing::NotNull(), &event.signalTimerUp, startTime, _));
+		EXPECT_CALL(mockExec, scheduleByTimestampNt(testing::NotNull(), _, startTime, _));
 		// falling edge 20ms later
-		EXPECT_CALL(mockExec, scheduleByTimestampNt(testing::NotNull(), &event.endOfInjectionEvent, startTime + MS2NT(20), _));
+		EXPECT_CALL(mockExec, scheduleByTimestampNt(testing::NotNull(), _, startTime + MS2NT(20), _));
 	}
 
 	// Event scheduled at 715 degrees
@@ -112,9 +112,9 @@ TEST(injectionScheduling, InjectionIsScheduledAfterWraparound) {
 		// rising edge 15 degrees from now
 		float nt5deg = USF2NT(engine->rpmCalculator.oneDegreeUs * 15);
 		efitick_t startTime = nowNt + nt5deg;
-		EXPECT_CALL(mockExec, scheduleByTimestampNt(testing::NotNull(), &event.signalTimerUp, startTime, _));
+		EXPECT_CALL(mockExec, scheduleByTimestampNt(testing::NotNull(), _, startTime, _));
 		// falling edge 20ms later
-		EXPECT_CALL(mockExec, scheduleByTimestampNt(testing::NotNull(), &event.endOfInjectionEvent, startTime + MS2NT(20), _));
+		EXPECT_CALL(mockExec, scheduleByTimestampNt(testing::NotNull(), _, startTime + MS2NT(20), _));
 	}
 
 	// Event scheduled at 5 degrees