Conversation
marc-mabe
force-pushed
the
new-time-retrieval-api
branch
from
c34e5e4 to
bd5fb4c
Compare
TimWolla
left a comment
TimWolla
left a comment
There was a problem hiding this comment.
The time retrieval functions are not inlined to make it possible to be mocked in unit tests.
Is it really necessary to mock them rather than mocking their implementation default? Not allowing them to be inlined is likely going to make them expensive for basically only being a wrapper.
| } | ||
|
|
||
| /* Assign timeval to timespec */ | ||
| static zend_always_inline void zend_time_val2spec(struct timeval tv, struct timespec *ts) { |
There was a problem hiding this comment.
Taking the tv by value and ts by pointer looks odd. Personally I find taking and returning by value more convenient, so I wonder if we should just do this for all the APIs:
| static zend_always_inline void zend_time_val2spec(struct timeval tv, struct timespec *ts) { | |
| static zend_always_inline struct timespec zend_time_val2spec(struct timeval tv) { |
On Linux both struct timespec and struct timeval are 16 bytes, thus they can be passed and returned in a register pair.
There was a problem hiding this comment.
This comment is also specifically intended for the other reviewers to discuss.
|
|
||
| /* Assign timeval to timespec */ | ||
| static zend_always_inline void zend_time_val2spec(struct timeval tv, struct timespec *ts) { | ||
| ts->tv_sec = (time_t) tv.tv_sec; |
There was a problem hiding this comment.
| ts->tv_sec = (time_t) tv.tv_sec; | |
| ts->tv_sec = tv.tv_sec; |
tv.tv_sec should already be time_t.
| #define ZEND_MILLI_IN_SEC 1000U | ||
| #define ZEND_MICRO_IN_SEC 1000000U |
There was a problem hiding this comment.
| #define ZEND_MILLI_IN_SEC 1000U | |
| #define ZEND_MICRO_IN_SEC 1000000U | |
| #define ZEND_TIME_MILLI_IN_SEC 1000U | |
| #define ZEND_TIME_MICRO_IN_SEC 1000000U |
2 participants
This is split out from #19202 to first introduce the new time retrieval API. Later PRs will follow to refactor time retrieval usages.
Key Changes
Introduced
zend_time.hA new internal header that abstracts system time functions, replacing direct usage of
<time.h>and related platform-specific APIs.Added
zend_time_real_specA unified wrapper around
clock_gettime(),timespec_get(),gettimeofday(), andtime()using the real/wall clock.Returns a
timespecstructure with up-to nanosecond precision.Added
zend_time_real_secA lightweight wrapper around
time(NULL)for simple, low-resolution time retrieval in seconds.Added
zend_time_mono_fallback_nsecA wrapper for
zend_hrtimeor falls back tozend_time_real_spec.Useful for time measurements (e.g. timeout handling) where monotonic time is preferred, but wall time is an acceptable fallback.
Added helper functions
Added utilities to simplify usage of
timevalandtimespecstructures.The time retrieval functions are not inlined to make it possible to be mocked in unit tests.