Repository navigation
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
hloop_now()can report the previous second even whenhloop_now_ms()already reports the next one.hloop_update_time()interprets that rounding error as a system-clock change and unnecessarily recalculates the loop's wall-clock starting point.Example
Suppose the loop starts at 12:00:00.900, and 200 ms passes. The system clock has not been adjusted:
The existing seconds calculation truncates the two parts separately:
But adding first and then truncating gives 12:00:01, consistent with both the system clock and
hloop_now_ms().Because
hloop_update_time()compares the incorrectly calculated second withtime(NULL), it enters its “systemtime changed” path even though both clocks are progressing normally. This can happen repeatedly around second boundaries. Rebuilding the starting point from separately sampled clocks also lets sampling jitter affect later cached timestamps; applications using unsigned elapsed-time subtraction are particularly sensitive to any backward step. This PR addresses the unnecessary recalculation, without claiming that wall-clock timestamps are monotonic.Controlled backward-step regression
The regression test also demonstrates a backward step using controlled clock samples. Starting from a wall-clock origin of 100,900 ms, it supplies these increasing readings:
hloop_now_ms()hloop_now_ms()The first sample models a normal 2 ms delay between reading the high-resolution clock and the wall clock. The old code unnecessarily rebases at that point, then rebases again in the final row. Its cached time goes backward by 1 ms, although both input clocks move forward. The unmodified code reproduced this result in the test; the corrected Debug and Release builds passed. These are deterministic test inputs, not measurements from an application workload.
Changes
hloop_now()from the combined millisecond timestamp, so it agrees withhloop_now_ms() / 1000.hloop_update_time(): the sampled system time and the starting wall time plus elapsed high-resolution time.Real system-clock changes still update the wall-clock baseline.
hloop_now_hrtime()and the high-resolution deadlines used by ordinary relative timers retain their existing behavior. No public API changes are needed.Validation
102001→102000ms reversal.NDEBUG.To run the full focused regression on Linux:
cmake -S . -B build/clock-debug \ -DCMAKE_BUILD_TYPE=Debug -DBUILD_UNITTEST=ON -DBUILD_EXAMPLES=OFF \ -DBUILD_STATIC=ON -DBUILD_SHARED=OFF -DWITH_HTTP=OFF -DWITH_EVPP=ON cmake --build build/clock-debug --target hloop_clock_test -j8 build/clock-debug/bin/hloop_clock_testRepeat with
Releaseand a separate build directory to check optimized behavior. The tests do not adjust the host clock. Other operating systems were not tested.This change concerns timestamp calculation and clock-change detection. It is separate from the timer lifetime work in #910 and the socket-polling starvation fix in #922.
Thanks to Mahmut Şeyhan, who first discovered this bug and reported it in WaterWall PR #346.