feat: S12.15 example service thread yields 1 ms per tick#285
Conversation
ExampleServiceThread_Run takes an injected SolidSyslogSleepFunction and sleeps 1 ms unconditionally after every SolidSyslog_Service call, replacing the busy-spin that hammered the CPU at 100% when the buffer and store were empty. POSIX and Windows mains pass SolidSyslogPosixSleep and SolidSyslogWindowsSleep respectively. Library API unchanged.
|
Warning Rate limit exceeded
To continue reviewing without waiting, purchase usage credits in the billing tab. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughThis PR implements an idle-yield optimization for the example service thread. The run loop now accepts a sleep callback and yields 1 ms unconditionally after each service tick, replacing the previous busy-spin behavior. POSIX and Windows example entrypoints wire platform sleep functions, and a new test verifies the yield occurs exactly once per iteration. ChangesService Thread Idle Yield
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
Tests/Example/ExampleServiceThreadTest.cpp (1)
19-21: ⚡ Quick winRename
g_-prefixed test globals to plain camelCase.These globals use a prefix style that conflicts with the project naming rule; please rename to camelCase (e.g.,
sleepCallCount,lastSleepMs,sleepShutdown).As per coding guidelines, “Variables must use
camelCasenaming” and “Do not use … prefixes (m_,_, etc.).”🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Tests/Example/ExampleServiceThreadTest.cpp` around lines 19 - 21, Rename the test globals g_sleepCallCount, g_lastSleepMs, and g_sleepShutdown to camelCase names (sleepCallCount, lastSleepMs, sleepShutdown) to comply with project naming rules; update every reference and declaration in this file (and any headers/tests that refer to these symbols) including uses in functions, initializations, and any externs so the new identifiers replace the old ones everywhere to avoid compile errors.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@Tests/Example/ExampleServiceThreadTest.cpp`:
- Around line 19-21: Rename the test globals g_sleepCallCount, g_lastSleepMs,
and g_sleepShutdown to camelCase names (sleepCallCount, lastSleepMs,
sleepShutdown) to comply with project naming rules; update every reference and
declaration in this file (and any headers/tests that refer to these symbols)
including uses in functions, initializations, and any externs so the new
identifiers replace the old ones everywhere to avoid compile errors.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: a470c75e-e06d-4372-b3cb-f49424f6852f
📒 Files selected for processing (6)
DEVLOG.mdExample/Common/ExampleServiceThread.cExample/Common/ExampleServiceThread.hExample/Threaded/main.cExample/Windows/SolidSyslogWindowsExample.cTests/Example/ExampleServiceThreadTest.cpp
☀️ Quality Summary 🚦 build-linux-gcc: 100% successful (✔️ 1091 passed, 🙈 3 skipped) Created by Quality Monitor v1.14.0 (#f3859fd). More details are shown in the GitHub Checks Result. |
Project convention is plain camelCase for variables — Hungarian-style prefixes are forbidden by CLAUDE.md. The g_ prefix was copied from SolidSyslogTlsStreamTest.cpp, which itself violates the rule; propagating it was a mistake.
☀️ Quality Summary 🚦 build-linux-gcc: 100% successful (✔️ 1091 passed, 🙈 3 skipped) Created by Quality Monitor v1.14.0 (#f3859fd). More details are shown in the GitHub Checks Result. |
1 participant
Summary
ExampleServiceThread_Runnow takes an injectedSolidSyslogSleepFunctionand sleeps 1 ms unconditionally after everySolidSyslog_Service()call. Replaces the busy-spin that pegged CPU at 100% when buffer and store were empty.Example/Threaded/main.c(Linux) wiresSolidSyslogPosixSleep;Example/Windows/SolidSyslogWindowsExample.cwiresSolidSyslogWindowsSleep.Example/Common/and the per-platform mains.The original story text suggested
#ifdef _WIN32for the platform sleep, butSolidSyslogSleepFunctionand the POSIX/Windows wirings already exist (added for the TLS handshake retry), so injection was the simpler path. Issue body updated to reflect this.A future story can investigate signalling-based fully-idle wait (eventfd / SetEvent / RTOS task notifications via the buffer abstraction) — out of scope here.
Test plan
Tests/Example/ExampleServiceThreadTest.cpp—SleepFakemirrors the pattern inSolidSyslogTlsStreamTest.cpp; the fake flips the shutdown flag on first call so a new testYieldsOneMillisecondAfterEachServiceTickproves the yield happens once per tick at 1 msSolidSyslogTests.exeran 973/973Closes #277
🤖 Generated with Claude Code
Summary by CodeRabbit
Release Notes
Bug Fixes
Tests