From 66d447881007d9eb5a66f83084df7a0cad4e145b Mon Sep 17 00:00:00 2001 From: Lukas Zeller Date: Thu, 17 Aug 2023 23:20:26 +0200 Subject: [PATCH] [PR pending] SystemLayerImplSelect: libev: eliminate I/O wakeup thread libev based regular builds do not need a separate I/O wakeup thread, so the wakeup thread can be eliminated in CHIP_SYSTEM_CONFIG_USE_LIBEV case. In normal operation, libev based builds also never call Signal(), so if it is still called it now emits a error log message, to indicate something might be wrong in the setup. However we keep Signal() and related LayerSocketsLoop methods, following the Darwin dispatch implementation, as fallback to select-based event handling seems to be needed for some I/O tests. As noted in a comment to the original libev PR [1], eventually, the select() based mainloop and external mainloop based solutions like Darwin Dispatch and libev should be detangled and extracted into separate classes, adapting all the tests that somehow rely on the select() fallback. As a single self-funded developer however, I cannot possibly be expected to solve this for Apple ;-) [1] https://github.com/project-chip/connectedhomeip/pull/24232#pullrequestreview-1236220931 --- src/system/SystemLayerImplSelect.cpp | 11 +++++++++++ src/system/SystemLayerImplSelect.h | 2 ++ src/system/WakeEvent.cpp | 4 ++-- src/system/WakeEvent.h | 4 ++-- 4 files changed, 17 insertions(+), 4 deletions(-) diff --git a/src/system/SystemLayerImplSelect.cpp b/src/system/SystemLayerImplSelect.cpp index 9b1ce3bc265dba..a4e23e4aff99fa 100644 --- a/src/system/SystemLayerImplSelect.cpp +++ b/src/system/SystemLayerImplSelect.cpp @@ -66,8 +66,10 @@ CHIP_ERROR LayerImplSelect::Init() mHandleSelectThread = PTHREAD_NULL; #endif // CHIP_SYSTEM_CONFIG_POSIX_LOCKING +#if !CHIP_SYSTEM_CONFIG_USE_LIBEV // Create an event to allow an arbitrary thread to wake the thread in the select loop. ReturnErrorOnFailure(mWakeEvent.Open(*this)); +#endif // !CHIP_SYSTEM_CONFIG_USE_LIBEV VerifyOrReturnError(mLayerState.SetInitialized(), CHIP_ERROR_INCORRECT_STATE); return CHIP_NO_ERROR; @@ -113,13 +115,18 @@ void LayerImplSelect::Shutdown() mTimerPool.ReleaseAll(); #endif // CHIP_SYSTEM_CONFIG_USE_DISPATCH/LIBEV +#if !CHIP_SYSTEM_CONFIG_USE_LIBEV mWakeEvent.Close(*this); +#endif // !CHIP_SYSTEM_CONFIG_USE_LIBEV mLayerState.ResetFromShuttingDown(); // Return to uninitialized state to permit re-initialization. } void LayerImplSelect::Signal() { +#if CHIP_SYSTEM_CONFIG_USE_LIBEV + ChipLogError(DeviceLayer, "Signal() should not be called in CHIP_SYSTEM_CONFIG_USE_LIBEV builds (might be ok in tests)"); +#else /* * Wake up the I/O thread by writing a single byte to the wake pipe. * @@ -143,6 +150,7 @@ void LayerImplSelect::Signal() ChipLogError(chipSystemLayer, "System wake event notify failed: %" CHIP_ERROR_FORMAT, status.Format()); } +#endif // !CHIP_SYSTEM_CONFIG_USE_LIBEV } CHIP_ERROR LayerImplSelect::StartTimer(Clock::Timeout delay, TimerCompleteCallback onComplete, void * appState) @@ -274,7 +282,10 @@ void LayerImplSelect::CancelTimer(TimerCompleteCallback onComplete, void * appSt #endif // CHIP_SYSTEM_CONFIG_USE_DISPATCH/LIBEV mTimerPool.Release(timer); +#if !CHIP_SYSTEM_CONFIG_USE_LIBEV + // LIBEV has no I/O wakeup thread, so must not call Signal() Signal(); +#endif } CHIP_ERROR LayerImplSelect::ScheduleWork(TimerCompleteCallback onComplete, void * appState) diff --git a/src/system/SystemLayerImplSelect.h b/src/system/SystemLayerImplSelect.h index c835b8a76720c5..361feef991460f 100644 --- a/src/system/SystemLayerImplSelect.h +++ b/src/system/SystemLayerImplSelect.h @@ -146,7 +146,9 @@ class LayerImplSelect : public LayerSocketsLoop int mSelectResult; ObjectLifeCycle mLayerState; +#if !CHIP_SYSTEM_CONFIG_USE_LIBEV WakeEvent mWakeEvent; +#endif #if CHIP_SYSTEM_CONFIG_POSIX_LOCKING std::atomic mHandleSelectThread; diff --git a/src/system/WakeEvent.cpp b/src/system/WakeEvent.cpp index 4b5d7c2a9ad00f..12ed9231e4a2de 100644 --- a/src/system/WakeEvent.cpp +++ b/src/system/WakeEvent.cpp @@ -23,7 +23,7 @@ #include -#if CHIP_SYSTEM_CONFIG_USE_SOCKETS +#if CHIP_SYSTEM_CONFIG_USE_SOCKETS && !CHIP_SYSTEM_CONFIG_USE_LIBEV #include @@ -207,4 +207,4 @@ CHIP_ERROR WakeEvent::Notify() const } // namespace System } // namespace chip -#endif // CHIP_SYSTEM_CONFIG_USE_SOCKETS +#endif // CHIP_SYSTEM_CONFIG_USE_SOCKETS && !CHIP_SYSTEM_CONFIG_USE_LIBEV diff --git a/src/system/WakeEvent.h b/src/system/WakeEvent.h index b979c67ab17752..d0c603952c21b7 100644 --- a/src/system/WakeEvent.h +++ b/src/system/WakeEvent.h @@ -25,7 +25,7 @@ // Include configuration headers #include -#if CHIP_SYSTEM_CONFIG_USE_SOCKETS +#if CHIP_SYSTEM_CONFIG_USE_SOCKETS && !CHIP_SYSTEM_CONFIG_USE_LIBEV #include #include @@ -67,4 +67,4 @@ class WakeEvent } // namespace System } // namespace chip -#endif // CHIP_SYSTEM_CONFIG_USE_SOCKETS +#endif // CHIP_SYSTEM_CONFIG_USE_SOCKETS && !CHIP_SYSTEM_CONFIG_USE_LIBEV