Add userTag to epicsTimeStamp

[Andrew Johnson]

See core-talk thread at http://www.aps.anl.gov/epics/core-talk/2017/msg00538.php

Needs conclusion on how the userTag should be defined (32/64, signed/unsigned).

[Andrew Johnson (anj)]

This patch crosses Base-7.0 submodules and contains my initial experiment.

Work needed:
* Design decisions (32/64 bit, signed/unsigned)
* Handle epicsTime::strftime (needs refactoring, since the epicsTime class should *not* contain a userTag which would bloat pass-by-value operations)
* Tests - TSE -> .TIME links, IOC -> pvAccess

[mdavidsaver (mdavidsaver)]

As someone who has thought about use of timing systems in data acquisitions, having a 64-bit tag simply to enable a life-of-facility odometer is not compelling. When applied to secPastEpoch, the same argument suggests expanding to 64-bits unsigned. I hope that this suggests some redundancy in these two ideas. IMO, taking as a timestamp the time at which this counter ticks is more generally useful.

For this reason, I would rather see secPastEpoch expanded to 64-bits along with a 32-bit tag.

[Andrew Johnson (anj)]

The epicsTimeStamp::secPastEpoch field is already unsigned so not subject to the 2038 overflow problem, and when it does hit its limit sometime after 2100 nobody should still be using CA (if they're still using EPICS). Since we don't *need* to change this (and we already have our own epoch so timestamp conversions are already required) I don't see a particular advantage to making that 64-bit.

I did just notice though that db_access.h uses epicsTimeStamp in all the dbr_time_* types, so my experiment would have broken CA if I'd tested with an external CA client. Additional work would thus be required to remain compatible with older CA versions, and it would probably be safer to change the name of the IOC's data type. I just lost my desire to do this.

[Bruce Hill (bhill)]

More input re the need for a 64bit userTag.

SLAC is well into the design and implementation of LCLS-II, and a 64 bit pulseId is baked into the design for all components, including non-EPICS components like our data acquisition.

Thus, while I appreciate Michael's suggestion that there is some redundancy in using a 64bit userTag, not having one makes it difficult to reconstruct the upper 32 bits as that would be facility specific.

In our EPICS timing module driver, we have access to the timing stream with 32bit sec, 32bit nsec, and a 64bit pulseId that increments at 1Mhz. A 32bit pulseId would roll over after approx 4300 sec, so while it's messy to have to provide an API for converting from an epics::pvData::TimeStamp object to our 64bit pulseId, a single reference point would suffice.

Since we'll have a unique mapping from each 64bit pulseId to specific 32bit sec and nsec values, we also think we could convert epicsTimeStamp objects w/ just uint32 sec and nsec to our 64bit pulseId by keeping a few additional reference points as needed to handle any long term drift or ntp corrections.

For V3 clients, our current EPICS server implementations are publishing the upper and lower 32 bits of each pulseId as separate CA PVs, requiring V3 clients to monitor the pulseId PV's separately and do their own correlation between epicsTimeStamp and 64bit pulseId.

We're now starting to look at how V4 clients can handle this and would really love to avoid similar hacks for V4.

[mdavidsaver (mdavidsaver)]

I'll try to summarize the technical part my thinking and discussions with Bruce Hill, and others in the past few weeks.

There is just no simple way to handle userTag/pulse-id in the database where it can moved around in all the necessary ways. These ways being:

1. From a record into server code (eg. QSRV).
2. From one record to another via TSEL to .TIME.
3. Into a record from device support.

The ideas I've had so far are:

A) use/abuse an info() tag

The idea being that device support can provide some extra storage, and store
an uint64_t pointer to this storage in a well known dbInfoNode::pointer.
QSRV then looks for this node, finds the pointer, and reads the userTag.

This easily satisfies #1, and with quite a bit of effort could do #3. However,
There is no obvious way to handle #2.

The difficulty of #3 comes from the need to update every driver which
wants publish userTag. Practically, this means updating the interfaces,
and device support FIFO code of asyn, which would not be a small task.

This approach is also the only one which doesn't require code changes
to Base.

B) Add a member to struct epicsTimeStamp

Add another member eg. epicsTimeStamp::userTag and convert existing
code to copy by structure assignment were needed (from a brief survey,
not often).

This satisfies all of #1, #2, and #3. Unfortunately struct epicsTimeStamp
is used in the CA (de)serialization code, which could be changed, probably
with no/minimal API changes. However, it is also used by the likes for
channelarchiver for its file serialization, which can't be so easily updated.

There would also be some problems with external code leaving the new member
undefined. Though I think this would be fairly minor in practice.

C) Add another field (eg. UTAG) to dbCommon.

Add a member dbCommon::utag.

This loses the no-base-changes benefit of A), and has all the same problems,
except that TSEL=.TIME handling is straightforward.

[Andrew Johnson (anj)]

> C) Add another field (eg. UTAG) to dbCommon.
> ...
> except that TSEL=.TIME handling is straightforward.

I'm not convinced "straightforward" is quite the right word. With EPICS 7 and link support, everything should now be calling dbGetTimeStamp(const struct link *, epicsTimeStamp *) to get a timestamp though the polymorphic lset::getTimestamp() method. That helps solution B), but for C) it would require the utag value to be passed through some kind of side-channel by those link types that support it and it wouldn't be possible to store the tag into intermediate storage (like many device support and lnkCalc.c already do in EPICS 7).

We might be able to implement B) by making use of the db_access vs dbAccess split. All files including dbAccess.h (and other IOC-side headers) would get an epicsTimeStamp with the utag, while all files that include the old db_access.h (i.e. CA clients) would get the old one. We'd have to be careful with the C++ One Definition rule though, maybe epicsTimeStamp itself would become a macro with different expansions?

[Ralph Lange (ralph-lange)]

This sounds like adding a *lot* of complexity (i.e. risk) for implementing a feature that is required by what percentage of our users?

[mdavidsaver (mdavidsaver)]

> I'm not convinced "straightforward" is quite the right word.

I should have been more specific. I'm referring only to DB_LINK. For CA_LINK and beyond, it is not so straightforward.

> ... making use of the db_access vs dbAccess split.

oh, evil. I can see some potential problems, mainly how to deal with
"#include <epicsTime.h>" in two contexts.

> We'd have to be careful with the C++ One Definition rule

Practically, I don't think this is an issue. There is no usage of RTTI.

[Andrew Johnson (anj)]

Ralph: I agree, but with pvAccess providing a user-tag I'm happy to at least consider what would have to be done to support it properly in the IOC database. I assume Michael is being asked to do something like that by SLAC. Other sites with pulsed machines may want to attach tags to their timestamps but just haven't asked, so we don't really know where this might be used if it was available.

Separating the IOC's internal time-stamp data type from that used by CA's api and network protocol seems like a good thing to investigate for future flexibility anyway.

Michael: I guessed you were probably only thinking about DB links, but I don't want to side-step the new link-support APIs. That means only using the dbGetTimestamp(plink, pstamp) API to fetch time-stamp data from plink and write it to pstamp (there may be several links in the chain, and while there's no "db" JSON link type yet I don't want to preclude it in the future).

The epicsTime class needs to provide conversions to/from both definitions, so we can't have two different structure definitions for the same name. Thus I believe the One Definition Rule matters because of these conversion methods in libCom:

tux% nm -C lib/linux-x86_64/libCom.so.3.17.0 | grep epicsTimeStamp
00000000000342b0 T epicsTime::epicsTime(epicsTimeStamp const&)
00000000000342b0 T epicsTime::epicsTime(epicsTimeStamp const&)
0000000000034f60 T epicsTime::operator epicsTimeStamp() const

Those appear to be the only places where epicsTimeStamp appears in any C++ symbol though.

[Bruce Hill (bhill)]

Yes, our LCLS2 timing requirements specify a 64bit pulseId, so we're looking at
how to satisfy that requirement. We'd like to be able to use the pvAccess userTag
instead of having to add it as an additional field via V4.

We considered doing our own local mod to epicsTimeStamp, but rejected that approach because of all the issues w/ client code compatibility and not being able to pass this extended timeStamp via CA.

I'm inclined to approach this by taking advantage of our 1:1 relationship between pulseId and epicsTime secPastEpoch and nSec. Our timing modules have access to the pulseId for any timeStamp
we get from our timing system, so they should be able to translate timeStamps to userTags
when needed. To make this approach work, we'd need a way to register this service, much like
we register our timing system as a generalTimeProvider.