Skip to content

Refactor basic time usages #19202

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

marc-mabe
Copy link
Contributor

@marc-mabe marc-mabe commented Jul 21, 2025

This refactors the internal usages of the current time.

  • Introduced php_time.h to be used instead of system time.h
  • Added php_realtime_get, php_realtime_spec and php_realtime_res as a wrapper around timespec_get[res], clock_get[time|res], gettimeofday and time using the realtime clock
  • Added php_monotime_get, php_monotime_spec and php_monotime_res as a wrapper around timespec_get[res], clock_get[time|res] using the monotonic clock
  • Added helper macros for working with timeval and timespec
  • Replace direct usages of the system time API with php_*
  • migrate to timespec for current time but keep timeval for timeouts

As a result:

  • The code is better readable because system differences are encapsulated
  • php_realtime_get will not fail as it falls back timespec_get -> clock_gettime -> gettimeofday -> time and time will never fail
  • php_realtime_get will be available for sure because of the same
  • Y2038 problem of WIN64 due to long of timeval.tv_sec should be avoided
  • Increased time resolution of microtime() / gettimeofday(true) without BC break
  • uniqid refactored a bit to not wait for the next time but increments if the current time is less or equal to the previous call. This should be faster and resolve a theoretical issue if the real time got modified in between (ntp)

Even thought gettimeofday may not be available it wasn't checked everywhere before and compiling would have been failed already before. Availability should now be checked but as it falls back and it must have been available before I removed the conditions around microtime, gettimeofday and uniqid

This should fix #17856

PS: It's not fully refactored yet but a general pre-review would be welcome.

@marc-mabe marc-mabe force-pushed the current_time_wrapper branch from b3abaf4 to c81c27f Compare July 22, 2025 08:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

time() and friends have Y2038 problem on 64 Windows
1 participant