Skip to content
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

az_platform_clock_msec POSIX implementation #2364

Open
CIPop opened this issue Oct 18, 2022 · 1 comment
Open

az_platform_clock_msec POSIX implementation #2364

CIPop opened this issue Oct 18, 2022 · 1 comment
Labels
Azure.Core bug This issue requires a change to an existing behavior in the product in order to be resolved. Client This issue points to a problem in the data-plane of the library.

Comments

@CIPop
Copy link
Member

CIPop commented Oct 18, 2022

az_posix.c implements the az_platform_clock_msec by measuring CPU time, not wall time. This API isn't used today.
In ANSI C/POSIX, clock_t clock() is to approximate the CPU time spent in a process and the current naming creates confusion.

A few ways to fix this:

  1. clock_gettime() clock_getres(). The problem is that POSIX doesn't guarantee availability of (all) timing functions. We'll need runtime code to discover enabled functionality.
  2. Use the c time() epoch timer with 1 second accuracy.

Also, please consider the API breaking change to rename the API name to AZ_NODISCARD az_result az_platform_time_msec(int64_t* out_clock_msec) to further reduce confusion on the purpose of the API.

@ghost ghost added the needs-triage Workflow: This is a new issue that needs to be triaged to the appropriate team. label Oct 18, 2022
@RickWinter RickWinter added bug This issue requires a change to an existing behavior in the product in order to be resolved. Client This issue points to a problem in the data-plane of the library. Azure.Core labels Oct 25, 2022
@ghost ghost removed the needs-triage Workflow: This is a new issue that needs to be triaged to the appropriate team. label Oct 25, 2022
@CIPop
Copy link
Member Author

CIPop commented Oct 25, 2022

The decision is to:

  1. Require POSIX clock_gettime / clock_getres and CLOCK_MONOTONIC and provide a POSIX compliant implementation using these APIs.
  2. Maintain the name of the function as-is to avoid breaking changes.

Based on man-pages:
CLOCK_MONOTONIC: Clock that cannot be set and represents monotonic time since—as described by POSIX—"some unspecified point in the past". On Linux, that point corresponds to the number of seconds that the system has been running since it was booted.
The CLOCK_MONOTONIC clock is not affected by discontinuous jumps in the system time (e.g., if the system administrator manually changes the clock), but is affected by the incremental adjustments performed by adjtime(3) and NTP. This clock does not count time that the system is suspended.

I have checked that RTOSes implementing POSIX layers do have support for above APIs:

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Azure.Core bug This issue requires a change to an existing behavior in the product in order to be resolved. Client This issue points to a problem in the data-plane of the library.
Projects
None yet
Development

No branches or pull requests

2 participants