Hi -
> > [...]
> > How about for now only changing the pmfg_event api to return
> > nsec-timespec records, but not yet actually accepting the
> > PM_TYPE_EVENT_HIGHRES type? This would make the API future-proof for
> > when/if a real pmda appears that actually generates HIGHRES events,
> > but in the mean time we don't have to bloat the code with it.
>
> Well, its not "future" (for proofing), its long since been supported,
Yes, in theory, but absent hypothetical/unavailable out-of-tree PMDAs,
it is not exactly a high priority feature. With the above-suggested
change to the fetchgroups API now done, HIGHRES support will be able
to slide in without zero further API/ABI changes.
> so new code needs to work with it (not all PMDAs live in-tree).
That standard is being applied inconsistently. e.g., there's no event
support at all in the python bindings, for example, never mind
HIGHRES. Other examples available on request, but I'd rather not
belabour the point.
> Let's just get it done and not waste time arguing about small stuff
> like this, thanks.
Stop trying to cut off discussion!
> > > 2. Sentinel values
> > >
> > > I'm not so much against sentinels in the way Mark seemed to be, as
> > > it looks kinda handy, but the choice of zero as a sentinel integer
> > > value strikes me as questionable. [...prefer -1...]
> >
> > OK, will look into that. [...]
A second thought appears here. We support unsigned integer types, for
whom a -1 sentinel becomes an inconveniently large number. The
sentinel-checking logic would have to be metric-type specific and IMHO
unsightly, whereas zero is OK everywhere. But, then again ...
Remember, the point of sentinel values is to be convenient for
computation & output, should the application *not care* about the
presence of errors. (If the application cared about errors, it would
use individual status integers.) So what matters is not how easily
the sentinel can be identified, but rather how smoothly an *unchecked*
sentinel value would mesh in with real values, with minimal disruption
of application data flow.
Do you see why 0 is a little more attractive choice?
> [...]
> dstruct.c: double zero = 0.0;
> dstruct.c: mynan = zero / zero;
It turns out this is blessed by my reading of IEEE-754, so switched to that.
> [re. pm context save/restore]
>
> Right, but this is such an unlikely situation (that an application would
> choose to use both pmFG contexts *and* non-pmFG PMAPI contexts *and* that
> it is buggy in its management of its non-pmFG contexts), that it is not
> worth such complications in every entry/exit from all fetchgroup APIs.
I think you're right on this one. (If removed, there will be
qa/src/fetchgroup.c impact.)
So now on pcpfans.git fche/fetchgroup is your stuff plus all that I
have pcp-hacking time for the next little while:
commit ac59f4abcb5d135c8f81efa1b2a6f80dd1de0e10
Author: Frank Ch. Eigler <fche@xxxxxxxxxx>
Date: Wed Jan 20 12:23:38 2016 -0500
fetchgroup sentinels: use 0/0 for float & double
It was reported that some platforms don't have nan("") and nanf("").
IEEE-754 indicates 0/0 should generate a quiet-NaN, which is fine.
commit 0ea56b7d9224c589b226fdade61be978dd4f53be
Author: Frank Ch. Eigler <fche@xxxxxxxxxx>
Date: Wed Jan 20 11:14:06 2016 -0500
fetchgroup events: use timespec rather than timeval
In preparation for smooth future extension to PM_TYPE_EVENT_HIGHRES,
represent all event records' timestamps with a nanosecond-precision
timespec rather than microsecond-precision timeval. C, man pages,
python bindings updated. QA outputs are unmodified.
- FChE
|