Merge lp://staging/~epics-core/epics-base/monotonictime into lp://staging/~epics-core/epics-base/3.16

Proposed by mdavidsaver
Status: Rejected
Rejected by: mdavidsaver
Proposed branch: lp://staging/~epics-core/epics-base/monotonictime
Merge into: lp://staging/~epics-core/epics-base/3.16
Diff against target: 416 lines (+262/-1)
12 files modified
src/libCom/osi/Makefile (+1/-0)
src/libCom/osi/epicsTime.h (+11/-0)
src/libCom/osi/os/Darwin/osdMonotonic.c (+32/-0)
src/libCom/osi/os/Darwin/osdTime.cpp (+3/-0)
src/libCom/osi/os/RTEMS/osdTime.cpp (+3/-0)
src/libCom/osi/os/WIN32/osdMonotonic.c (+53/-0)
src/libCom/osi/os/WIN32/osdTime.cpp (+3/-0)
src/libCom/osi/os/iOS/osdMonotonic.c (+31/-0)
src/libCom/osi/os/posix/osdMonotonic.c (+75/-0)
src/libCom/osi/os/posix/osdTime.cpp (+3/-0)
src/libCom/osi/os/vxWorks/osdTime.cpp (+3/-0)
src/libCom/test/epicsTimeTest.cpp (+44/-1)
To merge this branch: bzr merge lp://staging/~epics-core/epics-base/monotonictime
Reviewer Review Type Date Requested Status
mdavidsaver Disapprove
Review via email: mp+269124@code.staging.launchpad.net

Description of the change

As per lp:1392516 add a function epicsMonotonicGet() which returns the time in nanoseconds (epicsUInt64) since some unspecified local reference time.

Implementations for posix (clock_gettime w/ CLOCK_MONOTONIC), windows (QueryPerformanceCounter or GetTickCount), and mac (mach_absolute_time). vxWorks will probably use clock_gettime with CLOCK_REALTIME since I find no monotonic time source documented. The win32 code runs with WINE, and the posix code is test on linux and rtems. No testing for darwin/ios.

Also adds epicsMonotonicResolution() which estimates the resolution of epicsMonotonicGet() (minimum non-zero difference). However, this isn't guaranteed, and is just wrong in some cases, so this could go away.

To post a comment you must log in.
Revision history for this message
mdavidsaver (mdavidsaver) wrote :

Documentation waiting on review of function names.

Revision history for this message
Ralph Lange (ralph-lange) wrote :

Should we consider this for 3.14/3.15?

lp:1392516 gives no hint which version the user is basing his requirement on, but 3.16 might not be his favourite target release series.

Revision history for this message
mdavidsaver (mdavidsaver) wrote :

> Should we consider this for 3.14/3.15?

I'd be happy to see this added to 3.14 and/or 3.15. Do you want it there?

review: Needs Information
Revision history for this message
Andrew Johnson (anj) wrote :

Some VxWorks sites/boards may have a suitable time source available; there is a 64-bit time-base register in most PowerPC CPUs and the routine 'vxTimeBaseGet()' retrieves the value of that counter. All my BSPs here at the APS provide a routine 'sysUsSince()' which on PPC uses the time-base value to calculate time differences between calls in UINT32 microseconds (~71 minutes wraparound).

It would be good to define a VxWorks API like my sysAtReboot() one that is looked up at run-time and permits a monotonic time-source to be provided by the BSP, but falls back to something standard such as tickGet() if not.

Not finished reviewing yet...

Revision history for this message
Andrew Johnson (anj) wrote :

As I suspected Intel (Pentium+) CPUs have a similar 64-bit TSC (Time Stamp Counter) register, accessible in VxWorks using void pentiumTscGet64(long long int *);

I will take a look at implementing this for VxWorks.

Revision history for this message
Andrew Johnson (anj) wrote :

Your '#define EPICS_EXPOSE_LIBCOM_MONOTONIC_PRIVATE' lines should appear before *any* EPICS #include lines; you can't guarantee that in the future we won't need to add "#include <epicsTime.h>" to errlog.h or cantProceed.h for example.

On the Mac, call mach_timebase_info() once to find the clock resolution. I think it's safe to assume that the value returned by mach_absolute_time() is a true integer count.

Should the resolution value really be a single uint64? The mach_timebase_info() approach of returning both a numerator and denominator allows it to return a more accurate ratio. Listing 2 at https://developer.apple.com/library/mac/qa/qa1398/_index.html demonstrates. However I'm sure the smaller embedded systems would prefer to not have to do much 64-bit math, so I'm not necessarily advocating that we should do anything different, just posing the question.

Can explain in writing which subsystems you think should be changed to use the monotonic clock? I suspect that may turn out to be a rather large can of worms (the like of which I have delved into before). I realize that's not relevant to this branch, but I would like to start thinking about it.

12681. By mdavidsaver

move EPICS_EXPOSE_LIBCOM_MONOTONIC_PRIVATE before all local includes

Revision history for this message
mdavidsaver (mdavidsaver) wrote :

> Your '#define EPICS_EXPOSE_LIBCOM_MONOTONIC_PRIVATE' lines should appear before *any* EPICS #include lines

oops, your right. Fixed.

> On the Mac, call mach_timebase_info() once to find the clock resolution. I think it's safe to assume that the value returned by mach_absolute_time() is a true integer count.

This seem reasonable, but I'll leave the change to someone who can actually test it. Since I can't compile for this target I'll stick to the documented API.

> Should the resolution value really be a single uint64? ...

Well, right now epicsMonotonicResolution() is mostly decoration. There is no requirement to call it. I almost removed it since for some backends it doesn't return anything meaningful. I only kept it because found it usefulness in the unit test.

Do you really mean to suggest that epicsMonotonicGet() return a value in arbitrary "tick" units, then have a separate ticks to nanoseconds converter function?

Right now mac would be the only target(s) to benefit from this. That said, if for other targets this could be a no-op macro or always_inline ticks2ns function, so the additional runtime cost would be zero.

In the end I just don't have an idea about the cost in added runtime and/or accumulated bias of working only in nanoseconds vs. ticks. In the absence of at least some small evidence that this makes a difference, I'm inclined not to make the API more complicated.

Revision history for this message
mdavidsaver (mdavidsaver) wrote :

> Can explain in writing which subsystems you think should be changed to use the monotonic clock? ...

In the interest of keeping focus here I'll post this to lp:1392516.

Revision history for this message
mdavidsaver (mdavidsaver) wrote :

Andrew, where do things stand on this?

Revision history for this message
mdavidsaver (mdavidsaver) wrote :

Ready to merge as far as I'm concerned.

Revision history for this message
Andrew Johnson (anj) wrote :

F2F 3/17/2017: Implementation on MacOS still needs a bit of work. Also VxWorks needs its own implementation.

Revision history for this message
mdavidsaver (mdavidsaver) wrote :

I'll have to depend on @anj to create/test a vxworks implementation. I do find some evidence that clock_gettime() is provided by vxworks.

http://www.vxdev.com/docs/vx55man/vxworks/ref/clockLib.html

Please elaborate on (or simply make) the changes needed for Darwin/iOS.

Revision history for this message
mdavidsaver (mdavidsaver) wrote :
review: Disapprove

Unmerged revisions

12681. By mdavidsaver

move EPICS_EXPOSE_LIBCOM_MONOTONIC_PRIVATE before all local includes

12680. By mdavidsaver

libCom/osi: fetch monotonic time

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
The diff is not available at this time. You can reload the page or download it.

Subscribers

People subscribed via source and target branches