Single thread timer - #609
skywojciechowskim wants to merge 6 commits into
Conversation
|
Pull request must be merged with a description containing the required fields, Summary: If there is no jira releated to this change, please put 'Jira: NO-JIRA'. Description can be changed by editing the top comment on your pull request and making a new commit. |
There was a problem hiding this comment.
Copilot review overview
🟡 Changes recommended
Unresolved critical and moderate issues affect compilation, callback-thread guarantees, timer lifecycle, and delay handling.
Get a fresh assessment by requesting another Copilot review.
Review effort: Lite
Findings: 2
Open (5)
What changed in this PR
Refactors timers to use a shared GLib event-loop thread instead of one thread per timer.
Changes:
- Adds shared GLib timer scheduling and callback management.
- Replaces per-timer thread state with GLib source IDs.
- Adds GLib/GStreamer build dependencies.
| File | Summary |
|---|---|
common/source/Timer.cpp |
Implements shared timer scheduling; unresolved lifecycle, threading, range, cleanup, and compilation issues remain. |
common/include/Timer.h |
Replaces thread-based state with a GLib source ID. |
common/CMakeLists.txt |
Adds required GLib/GStreamer dependencies. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| m_loop = g_main_loop_new(nullptr, FALSE); | ||
| m_thread = std::thread([this]() { g_main_loop_run(m_loop); }); |
| static_cast<guint>(timeout.count()), | ||
| [](gpointer data) -> gboolean | ||
| { | ||
| auto callback = CommonTimerLoop::instance().getTimerCallback(static_cast<Timer *>(data)); |
| m_timerId = g_timeout_add( | ||
| static_cast<guint>(timeout.count()), |
| auto callback = CommonTimerLoop::instance().getTimerCallback(static_cast<Timer *>(data)); | ||
| if (callback) | ||
| { | ||
| if (m_active && m_callback) | ||
| { | ||
| lock.unlock(); | ||
| m_callback(); | ||
| } | ||
| callback(); | ||
| CommonTimerLoop::instance().removeTimerCallback(static_cast<Timer *>(data)); |
| bool Timer::isActive() const | ||
| { | ||
| return m_active; | ||
| return CommonTimerLoop::instance().getTimerCallback(this) != nullptr; |
|
Coverage statistics of your commit: |
Summary: Do not create a dedicated thread for each timer to improve the performance
Type: Fix
Test Plan: UT/CT, Fullstack
Jira: RDKEMW-24084