Merge ~info-martin-konrad/epics-base:callbackQueueStatus into epics-base:3.16

Proposed by Martin Konrad
Status: Merged
Approved by: Andrew Johnson
Approved revision: 6f919c3991bc264e91a313415a402250bab463ac
Merged at revision: ec036cb26d6664fa463a076d20cd3b41a09821bd
Proposed branch: ~info-martin-konrad/epics-base:callbackQueueStatus
Merge into: epics-base:3.16
Diff against target: 631 lines (+232/-20)
11 files modified
src/ioc/db/callback.c (+47/-0)
src/ioc/db/callback.h (+9/-0)
src/ioc/db/dbIocRegister.c (+24/-0)
src/ioc/db/dbScan.c (+37/-0)
src/ioc/db/dbScan.h (+9/-0)
src/libCom/ring/epicsRingBytes.c (+28/-3)
src/libCom/ring/epicsRingBytes.h (+3/-0)
src/libCom/ring/epicsRingPointer.cpp (+12/-0)
src/libCom/ring/epicsRingPointer.h (+34/-3)
src/libCom/test/ringBytesTest.c (+21/-13)
src/libCom/test/ringPointerTest.c (+8/-1)
Reviewer Review Type Date Requested Status
Andrew Johnson Approve
mdavidsaver Needs Fixing
Ralph Lange Pending
Review via email: mp+352918@code.launchpad.net

Description of the change

Allow scanOnce and callback queue utilization to be monitored from the IOC shell and iocStats. Fixes lp:1786540

To post a comment you must log in.
Ralph Lange (ralph-lange) wrote :

I like the idea of adding queue health status.

Number of overruns is good and useful data.

Given the common usage pattern of those queues (a large number of entries is put on the queue by a driver or at IOC init for PINI processing, then a number of threads are working on taking items off) I think that an iocStats or manual check of the current usage does not tell much as it most probably misses the moment of high usage.
A simple high water mark mechanism might generate data that is a lot more useful.

I would also consider resetting the stats at every read of their values for easy one-record reporting by iocStats. (Cumulative values are not nice in the archiver.)

Andrew Johnson (anj) wrote :

Core Group review at ESS: Please implement Ralph's high water mark (which needs implementing inside the queueing code) and use epicsAtomic types to protect access to the usage data.

review: Needs Fixing
Martin Konrad (info-martin-konrad) wrote :

I have implemented the high-water mark Ralph suggested. This is in fact very useful on small IOCs which are booting fast. The one I'm mainly testing with is seeing very heavy load after IOC start and I can watch the queue usage drop slowly in the first minutes after IOC boot (so in some cases the PVs showing the current queue usage also make sense).

I'm now also leveraging epicsAtomic to make my code thread safe.

I understand Ralph's concerns about cumulative values. However, I'm a little hesitant to reset the stats at every read since we offer read out from both the IOC shell and via iocStats. If a read by iocStats resets the number of queue overruns once a second chances are I'll never see anything on the IOC shell...

mdavidsaver (mdavidsaver) wrote :

Looking at this again, I find myself wondering about adding so many new public functions. 4 query functions and 1 printer, repeated 3 times. This in addition to the ring buffer feature addition. I'd rather see the 4x query functions combined into one which accepts a struct. Something like:

typedef struct QueueStats {
    size_t size;
    size_t numUsed[NUM_CALLBACK_PRIORITIES];
    size_t maxUsed[NUM_CALLBACK_PRIORITIES];
    size_t numOverflow[NUM_CALLBACK_PRIORITIES];
} QueueStats;

void queryQueue(int reset, QueueStates* result);

I think this would be a better match for the (only) prospective outside user, devIocStats.

The printer functions are accessible through iocsh, so it doesn't seem like they need to be public.

Also, I like the idea of combining fetch with (conditional) reset to zero. I've seen and used this idea in the past, and found it a good fit for how I ended up access these stats.

I'm not going to push for this too much if there is a desire to see this in 3.16.2.

At minimum though, the idea of const-ness should apply to the C API as well as C++. eg. epicsRingPointerGetHighWaterMark() and epicsRingBytesHighWaterMark() should take a pointer to const.

review: Needs Fixing
Martin Konrad (info-martin-konrad) wrote :

Thanks for the feedback. I reworked this to include Michael's comments. I have also updated my IOCStats PR. You might want to look at my code again - this ended up to be quite a few changes.

Andrew Johnson (anj) wrote :

Hi Martin, if you committed any changes I think you forgot to push them to launchpad, this merge request still only shows your original code. There may still be a chance to get it into the final 3.16.2 release.

Martin Konrad (info-martin-konrad) wrote :

Andrew, I double checked, this is my latest code. It has the const-ness change, the reset feature and it has been modified to pull all values at the same time. Note that I have squashed my commits together to avoid cluttering the history.

Andrew Johnson (anj) wrote :

@mdavidsaver Please re-review this merge, should it go into 3.16.2?

Andrew Johnson (anj) wrote :

Some older C compilers (MSVC, older GCC) will fail while building this code, there are several places where it declares variables after the first statement in a block. It's fine to do that in C++ code, but still not in our C code yet.

I don't understand the use of epicsAtomic functions for accessing callbackQueueSize. The queue size must be set before callbackIsInit and can't be changed after that (unless we go through callbackCleanup() but that clears callbackIsInit anyway and if callbackQueueStatus() is called then it returns an error). Did I miss something?

review: Needs Fixing
Martin Konrad (info-martin-konrad) wrote :

I tweaked the code to make pre-C99 compilers happy.

I have also removed the unneeded epicsAtomic from the callbackQueueSize variable.

Let me know if you want me to squash the commits.

Andrew Johnson (anj) wrote :

Thanks for those changes. Once you have created a Merge Request I would ask you to *never* squash commits, as doing that prevents us from seeing what additional changes you have made since we last looked at the proposal (hence my confusion on 2018-10-30). If you really want to hide the evidence of your having taken a U-turn from the final history, resubmitting it as a new Merge Request at the end of the review (or after a major rewrite) is probably the best way to handle that.

However I'm still unsure about accepting this; see my code-specific comments below. Further review from the other core developers is still welcome.

I also think these changes deserve some test code; we have test programs in src/libCom/test for both the bytes and pointer versions of the ring-buffer that could be extended, although the bytes version is single-threaded and just tests basic functionality — no inter-thread issues or tests for a spin-locked ring buffer.

Martin Konrad (info-martin-konrad) wrote :

I'm glad you asked me to write tests - that helped to I catch an issue in the code...

Do we need multi-threaded tests for this feature?

Andrew Johnson (anj) :
Martin Konrad (info-martin-konrad) wrote :

After our discussion I switched from atomics to using the existing spin lock. This should make the code a little bit easier to understand.

Andrew Johnson (anj) wrote :

Merging this, but one change I've made is to rename your QueuePrintStatus() routines and the QueueStatus iocsh commands (and their associated routines and data structures) to QueueShow for consistency with other subsystems. We normally use a Show suffix for routines that print stuff to stdout, and Status for querying whether a subsystem is happy, so your QueueStatus() API routines are fine. This is also necessary to maintain command-line compatibility with the VxWorks shell.

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1diff --git a/src/ioc/db/callback.c b/src/ioc/db/callback.c
2index ae07414..1b6c249 100644
3--- a/src/ioc/db/callback.c
4+++ b/src/ioc/db/callback.c
5@@ -54,6 +54,7 @@ typedef struct cbQueueSet {
6 epicsEventId semWakeUp;
7 epicsRingPointerId queue;
8 int queueOverflow;
9+ int queueOverflows;
10 int shutdown;
11 int threadsConfigured;
12 int threadsRunning;
13@@ -103,6 +104,51 @@ int callbackSetQueueSize(int size)
14 return 0;
15 }
16
17+int callbackQueueStatus(const int reset, callbackQueueStats *result)
18+{
19+ int ret;
20+ if (!callbackIsInit) return -1;
21+ if (result) {
22+ int prio;
23+ result->size = callbackQueueSize;
24+ for(prio = 0; prio < NUM_CALLBACK_PRIORITIES; prio++) {
25+ epicsRingPointerId qId = callbackQueue[prio].queue;
26+ result->numUsed[prio] = epicsRingPointerGetUsed(qId);
27+ result->maxUsed[prio] = epicsRingPointerGetHighWaterMark(qId);
28+ result->numOverflow[prio] = epicsAtomicGetIntT(&callbackQueue[prio].queueOverflows);
29+ }
30+ ret = 0;
31+ } else {
32+ ret = -2;
33+ }
34+ if (reset) {
35+ int prio;
36+ for(prio = 0; prio < NUM_CALLBACK_PRIORITIES; prio++) {
37+ epicsRingPointerResetHighWaterMark(callbackQueue[prio].queue);
38+ }
39+ }
40+ return ret;
41+}
42+
43+void callbackQueuePrintStatus(const int reset)
44+{
45+ callbackQueueStats stats;
46+ if (callbackQueueStatus(reset, &stats) == -1) {
47+ fprintf(stderr, "Callback system not initialized, yet. Please run "
48+ "iocInit before using this command.\n");
49+ } else {
50+ int prio;
51+ printf("PRIORITY HIGH-WATER MARK ITEMS IN Q Q SIZE %% USED Q OVERFLOWS\n");
52+ for (prio = 0; prio < NUM_CALLBACK_PRIORITIES; prio++) {
53+ double qusage = 100.0 * stats.numUsed[prio] / stats.size;
54+ printf("%8s %15d %10d %6d %6.1f %11d\n",
55+ threadNamePrefix[prio], stats.maxUsed[prio],
56+ stats.numUsed[prio], stats.size, qusage,
57+ stats.numOverflow[prio]);
58+ }
59+ }
60+}
61+
62 int callbackParallelThreads(int count, const char *prio)
63 {
64 if (callbackIsInit) {
65@@ -290,6 +336,7 @@ int callbackRequest(CALLBACK *pcallback)
66 if (!pushOK) {
67 epicsInterruptContextMessage(fullMessage[priority]);
68 mySet->queueOverflow = TRUE;
69+ epicsAtomicIncrIntT(&mySet->queueOverflows);
70 return S_db_bufFull;
71 }
72 epicsEventSignal(mySet->semWakeUp);
73diff --git a/src/ioc/db/callback.h b/src/ioc/db/callback.h
74index fa626d1..dd13cdb 100644
75--- a/src/ioc/db/callback.h
76+++ b/src/ioc/db/callback.h
77@@ -48,6 +48,13 @@ typedef epicsCallback CALLBACK;
78
79 typedef void (*CALLBACKFUNC)(struct callbackPvt*);
80
81+typedef struct callbackQueueStats {
82+ int size;
83+ int numUsed[NUM_CALLBACK_PRIORITIES];
84+ int maxUsed[NUM_CALLBACK_PRIORITIES];
85+ int numOverflow[NUM_CALLBACK_PRIORITIES];
86+} callbackQueueStats;
87+
88 #define callbackSetCallback(PFUN, PCALLBACK) \
89 ( (PCALLBACK)->callback = (PFUN) )
90 #define callbackSetPriority(PRIORITY, PCALLBACK) \
91@@ -73,6 +80,8 @@ epicsShareFunc void callbackCancelDelayed(CALLBACK *pcallback);
92 epicsShareFunc void callbackRequestProcessCallbackDelayed(
93 CALLBACK *pCallback, int Priority, void *pRec, double seconds);
94 epicsShareFunc int callbackSetQueueSize(int size);
95+epicsShareFunc int callbackQueueStatus(const int reset, callbackQueueStats *result);
96+void callbackQueuePrintStatus(const int reset);
97 epicsShareFunc int callbackParallelThreads(int count, const char *prio);
98
99 #ifdef __cplusplus
100diff --git a/src/ioc/db/dbIocRegister.c b/src/ioc/db/dbIocRegister.c
101index 4d0b88c..cecf756 100644
102--- a/src/ioc/db/dbIocRegister.c
103+++ b/src/ioc/db/dbIocRegister.c
104@@ -296,6 +296,17 @@ static void scanOnceSetQueueSizeCallFunc(const iocshArgBuf *args)
105 scanOnceSetQueueSize(args[0].ival);
106 }
107
108+/* scanOnceQueueStatus */
109+static const iocshArg scanOnceQueueStatusArg0 = { "reset",iocshArgInt};
110+static const iocshArg * const scanOnceQueueStatusArgs[1] =
111+ {&scanOnceQueueStatusArg0};
112+static const iocshFuncDef scanOnceQueueStatusFuncDef =
113+ {"scanOnceQueueStatus",1,scanOnceQueueStatusArgs};
114+static void scanOnceQueueStatusCallFunc(const iocshArgBuf *args)
115+{
116+ scanOnceQueuePrintStatus(args[0].ival);
117+}
118+
119 /* scanppl */
120 static const iocshArg scanpplArg0 = { "rate",iocshArgDouble};
121 static const iocshArg * const scanpplArgs[1] = {&scanpplArg0};
122@@ -335,6 +346,17 @@ static void callbackSetQueueSizeCallFunc(const iocshArgBuf *args)
123 callbackSetQueueSize(args[0].ival);
124 }
125
126+/* callbackQueueStatus */
127+static const iocshArg callbackQueueStatusArg0 = { "reset", iocshArgInt};
128+static const iocshArg * const callbackQueueStatusArgs[1] =
129+ {&callbackQueueStatusArg0};
130+static const iocshFuncDef callbackQueueStatusFuncDef =
131+ {"callbackQueueStatus",1,callbackQueueStatusArgs};
132+static void callbackQueueStatusCallFunc(const iocshArgBuf *args)
133+{
134+ callbackQueuePrintStatus(args[0].ival);
135+}
136+
137 /* callbackParallelThreads */
138 static const iocshArg callbackParallelThreadsArg0 = { "no of threads", iocshArgInt};
139 static const iocshArg callbackParallelThreadsArg1 = { "priority", iocshArgString};
140@@ -441,12 +463,14 @@ void dbIocRegister(void)
141 iocshRegister(&dbLockShowLockedFuncDef,dbLockShowLockedCallFunc);
142
143 iocshRegister(&scanOnceSetQueueSizeFuncDef,scanOnceSetQueueSizeCallFunc);
144+ iocshRegister(&scanOnceQueueStatusFuncDef,scanOnceQueueStatusCallFunc);
145 iocshRegister(&scanpplFuncDef,scanpplCallFunc);
146 iocshRegister(&scanpelFuncDef,scanpelCallFunc);
147 iocshRegister(&postEventFuncDef,postEventCallFunc);
148 iocshRegister(&scanpiolFuncDef,scanpiolCallFunc);
149
150 iocshRegister(&callbackSetQueueSizeFuncDef,callbackSetQueueSizeCallFunc);
151+ iocshRegister(&callbackQueueStatusFuncDef,callbackQueueStatusCallFunc);
152 iocshRegister(&callbackParallelThreadsFuncDef,callbackParallelThreadsCallFunc);
153
154 /* Needed before callback system is initialized */
155diff --git a/src/ioc/db/dbScan.c b/src/ioc/db/dbScan.c
156index e5c78fe..e0e14e0 100644
157--- a/src/ioc/db/dbScan.c
158+++ b/src/ioc/db/dbScan.c
159@@ -24,6 +24,7 @@
160 #include "cantProceed.h"
161 #include "dbDefs.h"
162 #include "ellLib.h"
163+#include "epicsAtomic.h"
164 #include "epicsEvent.h"
165 #include "epicsMutex.h"
166 #include "epicsPrint.h"
167@@ -63,6 +64,7 @@ static volatile enum ctl scanCtl;
168 static int onceQueueSize = 1000;
169 static epicsEventId onceSem;
170 static epicsRingBytesId onceQ;
171+static int onceQOverruns = 0;
172 static epicsThreadId onceTaskId;
173 static void *exitOnce;
174
175@@ -676,6 +678,7 @@ int scanOnceCallback(struct dbCommon *precord, once_complete cb, void *usr)
176 if (!pushOK) {
177 if (newOverflow) errlogPrintf("scanOnce: Ring buffer overflow\n");
178 newOverflow = FALSE;
179+ epicsAtomicIncrIntT(&onceQOverruns);
180 } else {
181 newOverflow = TRUE;
182 }
183@@ -722,6 +725,40 @@ int scanOnceSetQueueSize(int size)
184 return 0;
185 }
186
187+int scanOnceQueueStatus(const int reset, scanOnceQueueStats *result)
188+{
189+ int ret;
190+ if (!onceQ) return -1;
191+ if (result) {
192+ result->size = epicsRingBytesSize(onceQ) / sizeof(onceEntry);
193+ result->numUsed = epicsRingBytesUsedBytes(onceQ) / sizeof(onceEntry);
194+ result->maxUsed = epicsRingBytesHighWaterMark(onceQ) / sizeof(onceEntry);
195+ result->numOverflow = epicsAtomicGetIntT(&onceQOverruns);
196+ ret = 0;
197+ } else {
198+ ret = -2;
199+ }
200+ if (reset) {
201+ epicsRingBytesResetHighWaterMark(onceQ);
202+ }
203+ return ret;
204+}
205+
206+void scanOnceQueuePrintStatus(const int reset)
207+{
208+ scanOnceQueueStats stats;
209+ if (scanOnceQueueStatus(reset, &stats) == -1) {
210+ fprintf(stderr, "scanOnce system not initialized, yet. Please run "
211+ "iocInit before using this command.\n");
212+ } else {
213+ double qusage = 100.0 * stats.numUsed / stats.size;
214+ printf("PRIORITY HIGH-WATER MARK ITEMS IN Q Q SIZE %% USED Q OVERFLOWS\n");
215+ printf("%8s %15d %10d %6d %6.1f %11d\n", "scanOnce", stats.maxUsed,
216+ stats.numUsed, stats.size, qusage,
217+ epicsAtomicGetIntT(&onceQOverruns));
218+ }
219+}
220+
221 static void initOnce(void)
222 {
223 if ((onceQ = epicsRingBytesLockedCreate(sizeof(onceEntry)*onceQueueSize)) == NULL) {
224diff --git a/src/ioc/db/dbScan.h b/src/ioc/db/dbScan.h
225index d483a0c..830d3a8 100644
226--- a/src/ioc/db/dbScan.h
227+++ b/src/ioc/db/dbScan.h
228@@ -42,6 +42,13 @@ struct dbCommon;
229 typedef void (*io_scan_complete)(void *usr, IOSCANPVT, int prio);
230 typedef void (*once_complete)(void *usr, struct dbCommon*);
231
232+typedef struct scanOnceQueueStats {
233+ int size;
234+ int numUsed;
235+ int maxUsed;
236+ int numOverflow;
237+} scanOnceQueueStats;
238+
239 epicsShareFunc long scanInit(void);
240 epicsShareFunc void scanRun(void);
241 epicsShareFunc void scanPause(void);
242@@ -57,6 +64,8 @@ epicsShareFunc double scanPeriod(int scan);
243 epicsShareFunc int scanOnce(struct dbCommon *);
244 epicsShareFunc int scanOnceCallback(struct dbCommon *, once_complete cb, void *usr);
245 epicsShareFunc int scanOnceSetQueueSize(int size);
246+epicsShareFunc int scanOnceQueueStatus(const int reset, scanOnceQueueStats *result);
247+void scanOnceQueuePrintStatus(const int reset);
248
249 /*print periodic lists*/
250 epicsShareFunc int scanppl(double rate);
251diff --git a/src/libCom/ring/epicsRingBytes.c b/src/libCom/ring/epicsRingBytes.c
252index cb7e52e..ab048e4 100644
253--- a/src/libCom/ring/epicsRingBytes.c
254+++ b/src/libCom/ring/epicsRingBytes.c
255@@ -38,6 +38,7 @@ typedef struct ringPvt {
256 volatile int nextPut;
257 volatile int nextGet;
258 int size;
259+ int highWaterMark;
260 volatile char buffer[1]; /* actually larger */
261 }ringPvt;
262
263@@ -47,6 +48,7 @@ epicsShareFunc epicsRingBytesId epicsShareAPI epicsRingBytesCreate(int size)
264 if(!pring)
265 return NULL;
266 pring->size = size + SLOP;
267+ pring->highWaterMark = 0;
268 pring->nextGet = 0;
269 pring->nextPut = 0;
270 pring->lock = 0;
271@@ -118,7 +120,7 @@ epicsShareFunc int epicsShareAPI epicsRingBytesPut(
272 {
273 ringPvt *pring = (ringPvt *)id;
274 int nextGet, nextPut, size;
275- int freeCount, copyCount, topCount;
276+ int freeCount, copyCount, topCount, used;
277
278 if (pring->lock) epicsSpinLock(pring->lock);
279 nextGet = pring->nextGet;
280@@ -131,8 +133,9 @@ epicsShareFunc int epicsShareAPI epicsRingBytesPut(
281 if (pring->lock) epicsSpinUnlock(pring->lock);
282 return 0;
283 }
284- if (nbytes)
285+ if (nbytes) {
286 memcpy ((void *)&pring->buffer[nextPut], value, nbytes);
287+ }
288 nextPut += nbytes;
289 }
290 else {
291@@ -143,8 +146,9 @@ epicsShareFunc int epicsShareAPI epicsRingBytesPut(
292 }
293 topCount = size - nextPut;
294 copyCount = (nbytes > topCount) ? topCount : nbytes;
295- if (copyCount)
296+ if (copyCount) {
297 memcpy ((void *)&pring->buffer[nextPut], value, copyCount);
298+ }
299 nextPut += copyCount;
300 if (nextPut == size) {
301 int nLeft = nbytes - copyCount;
302@@ -155,6 +159,10 @@ epicsShareFunc int epicsShareAPI epicsRingBytesPut(
303 }
304 pring->nextPut = nextPut;
305
306+ used = nextPut - nextGet;
307+ if (used < 0) used += pring->size;
308+ if (used > pring->highWaterMark) pring->highWaterMark = used;
309+
310 if (pring->lock) epicsSpinUnlock(pring->lock);
311 return nbytes;
312 }
313@@ -224,3 +232,20 @@ epicsShareFunc int epicsShareAPI epicsRingBytesIsFull(epicsRingBytesId id)
314 {
315 return (epicsRingBytesFreeBytes(id) <= 0);
316 }
317+
318+epicsShareFunc int epicsShareAPI epicsRingBytesHighWaterMark(epicsRingBytesIdConst id)
319+{
320+ ringPvt *pring = (ringPvt *)id;
321+ return pring->highWaterMark;
322+}
323+
324+epicsShareFunc void epicsShareAPI epicsRingBytesResetHighWaterMark(epicsRingBytesId id)
325+{
326+ ringPvt *pring = (ringPvt *)id;
327+ int used;
328+ if (pring->lock) epicsSpinLock(pring->lock);
329+ used = pring->nextGet - pring->nextPut;
330+ if (used < 0) used += pring->size;
331+ pring->highWaterMark = used;
332+ if (pring->lock) epicsSpinUnlock(pring->lock);
333+}
334diff --git a/src/libCom/ring/epicsRingBytes.h b/src/libCom/ring/epicsRingBytes.h
335index 011829b..3dc0081 100644
336--- a/src/libCom/ring/epicsRingBytes.h
337+++ b/src/libCom/ring/epicsRingBytes.h
338@@ -24,6 +24,7 @@ extern "C" {
339 #include "shareLib.h"
340
341 typedef void *epicsRingBytesId;
342+typedef void const *epicsRingBytesIdConst;
343
344 epicsShareFunc epicsRingBytesId epicsShareAPI epicsRingBytesCreate(int nbytes);
345 /* Same, but secured by a spinlock */
346@@ -39,6 +40,8 @@ epicsShareFunc int epicsShareAPI epicsRingBytesUsedBytes(epicsRingBytesId id);
347 epicsShareFunc int epicsShareAPI epicsRingBytesSize(epicsRingBytesId id);
348 epicsShareFunc int epicsShareAPI epicsRingBytesIsEmpty(epicsRingBytesId id);
349 epicsShareFunc int epicsShareAPI epicsRingBytesIsFull(epicsRingBytesId id);
350+epicsShareFunc int epicsShareAPI epicsRingBytesHighWaterMark(epicsRingBytesIdConst id);
351+epicsShareFunc void epicsShareAPI epicsRingBytesResetHighWaterMark(epicsRingBytesId id);
352
353 #ifdef __cplusplus
354 }
355diff --git a/src/libCom/ring/epicsRingPointer.cpp b/src/libCom/ring/epicsRingPointer.cpp
356index 9c144ce..709ab65 100644
357--- a/src/libCom/ring/epicsRingPointer.cpp
358+++ b/src/libCom/ring/epicsRingPointer.cpp
359@@ -90,3 +90,15 @@ epicsShareFunc int epicsShareAPI epicsRingPointerIsFull(epicsRingPointerId id)
360 voidPointer *pvoidPointer = reinterpret_cast<voidPointer*>(id);
361 return((pvoidPointer->isFull()) ? 1 : 0);
362 }
363+
364+epicsShareFunc int epicsShareAPI epicsRingPointerGetHighWaterMark(epicsRingPointerIdConst id)
365+{
366+ voidPointer const *pvoidPointer = reinterpret_cast<voidPointer const*>(id);
367+ return(pvoidPointer->getHighWaterMark());
368+}
369+
370+epicsShareFunc void epicsShareAPI epicsRingPointerResetHighWaterMark(epicsRingPointerId id)
371+{
372+ voidPointer *pvoidPointer = reinterpret_cast<voidPointer*>(id);
373+ pvoidPointer->resetHighWaterMark();
374+}
375diff --git a/src/libCom/ring/epicsRingPointer.h b/src/libCom/ring/epicsRingPointer.h
376index 48d6203..68bf8f5 100644
377--- a/src/libCom/ring/epicsRingPointer.h
378+++ b/src/libCom/ring/epicsRingPointer.h
379@@ -40,18 +40,22 @@ public: /* Functions */
380 int getSize() const;
381 bool isEmpty() const;
382 bool isFull() const;
383+ int getHighWaterMark() const;
384+ void resetHighWaterMark();
385
386 private: /* Prevent compiler-generated member functions */
387 /* default constructor, copy constructor, assignment operator */
388 epicsRingPointer();
389 epicsRingPointer(const epicsRingPointer &);
390 epicsRingPointer& operator=(const epicsRingPointer &);
391+ int getUsedNoLock() const;
392
393 private: /* Data */
394 epicsSpinId lock;
395 volatile int nextPush;
396 volatile int nextPop;
397 int size;
398+ int highWaterMark;
399 T * volatile * buffer;
400 };
401
402@@ -59,6 +63,7 @@ extern "C" {
403 #endif /*__cplusplus */
404
405 typedef void *epicsRingPointerId;
406+typedef void const *epicsRingPointerIdConst;
407
408 epicsShareFunc epicsRingPointerId epicsShareAPI epicsRingPointerCreate(int size);
409 /* Same, but secured by a spinlock */
410@@ -74,6 +79,8 @@ epicsShareFunc int epicsShareAPI epicsRingPointerGetUsed(epicsRingPointerId id)
411 epicsShareFunc int epicsShareAPI epicsRingPointerGetSize(epicsRingPointerId id);
412 epicsShareFunc int epicsShareAPI epicsRingPointerIsEmpty(epicsRingPointerId id);
413 epicsShareFunc int epicsShareAPI epicsRingPointerIsFull(epicsRingPointerId id);
414+epicsShareFunc int epicsShareAPI epicsRingPointerGetHighWaterMark(epicsRingPointerIdConst id);
415+epicsShareFunc void epicsShareAPI epicsRingPointerResetHighWaterMark(epicsRingPointerId id);
416
417 /* This routine was incorrectly named in previous releases */
418 #define epicsRingPointerSize epicsRingPointerGetSize
419@@ -95,7 +102,8 @@ epicsShareFunc int epicsShareAPI epicsRingPointerIsFull(epicsRingPointerId id);
420
421 template <class T>
422 inline epicsRingPointer<T>::epicsRingPointer(int sz, bool locked) :
423- lock(0), nextPush(0), nextPop(0), size(sz+1), buffer(new T* [sz+1])
424+ lock(0), nextPush(0), nextPop(0), size(sz+1), highWaterMark(0),
425+ buffer(new T* [sz+1])
426 {
427 if (locked)
428 lock = epicsSpinCreate();
429@@ -121,6 +129,8 @@ inline bool epicsRingPointer<T>::push(T *p)
430 }
431 buffer[next] = p;
432 nextPush = newNext;
433+ int used = getUsedNoLock();
434+ if (used > highWaterMark) highWaterMark = used;
435 if (lock) epicsSpinUnlock(lock);
436 return(true);
437 }
438@@ -162,11 +172,18 @@ inline int epicsRingPointer<T>::getFree() const
439 }
440
441 template <class T>
442-inline int epicsRingPointer<T>::getUsed() const
443+inline int epicsRingPointer<T>::getUsedNoLock() const
444 {
445- if (lock) epicsSpinLock(lock);
446 int n = nextPush - nextPop;
447 if (n < 0) n += size;
448+ return n;
449+}
450+
451+template <class T>
452+inline int epicsRingPointer<T>::getUsed() const
453+{
454+ if (lock) epicsSpinLock(lock);
455+ int n = getUsedNoLock();
456 if (lock) epicsSpinUnlock(lock);
457 return n;
458 }
459@@ -196,6 +213,20 @@ inline bool epicsRingPointer<T>::isFull() const
460 return((count == 0) || (count == size));
461 }
462
463+template <class T>
464+inline int epicsRingPointer<T>::getHighWaterMark() const
465+{
466+ return highWaterMark;
467+}
468+
469+template <class T>
470+inline void epicsRingPointer<T>::resetHighWaterMark()
471+{
472+ if (lock) epicsSpinLock(lock);
473+ highWaterMark = getUsedNoLock();
474+ if (lock) epicsSpinUnlock(lock);
475+}
476+
477 #endif /* __cplusplus */
478
479 #endif /* INCepicsRingPointerh */
480diff --git a/src/libCom/test/ringBytesTest.c b/src/libCom/test/ringBytesTest.c
481index 6cef933..bb91d02 100644
482--- a/src/libCom/test/ringBytesTest.c
483+++ b/src/libCom/test/ringBytesTest.c
484@@ -30,7 +30,8 @@ typedef struct info {
485 epicsRingBytesId ring;
486 }info;
487
488-static void check(epicsRingBytesId ring, int expectedFree)
489+static void check(epicsRingBytesId ring, int expectedFree,
490+ int expectedHighWaterMark)
491 {
492 int expectedUsed = RINGSIZE - expectedFree;
493 int expectedEmpty = (expectedUsed == 0);
494@@ -39,11 +40,14 @@ static void check(epicsRingBytesId ring, int expectedFree)
495 int nUsed = epicsRingBytesUsedBytes(ring);
496 int isEmpty = epicsRingBytesIsEmpty(ring);
497 int isFull = epicsRingBytesIsFull(ring);
498+ int highWaterMark = epicsRingBytesHighWaterMark(ring);
499
500 testOk(nFree == expectedFree, "Free: %d == %d", nFree, expectedFree);
501 testOk(nUsed == expectedUsed, "Used: %d == %d", nUsed, expectedUsed);
502 testOk(isEmpty == expectedEmpty, "Empty: %d == %d", isEmpty, expectedEmpty);
503 testOk(isFull == expectedFull, "Full: %d == %d", isFull, expectedFull);
504+ testOk(highWaterMark == expectedHighWaterMark, "HighWaterMark: %d == %d",
505+ highWaterMark, expectedHighWaterMark);
506 }
507
508 MAIN(ringBytesTest)
509@@ -55,7 +59,7 @@ MAIN(ringBytesTest)
510 char get[RINGSIZE+1];
511 epicsRingBytesId ring;
512
513- testPlan(245);
514+ testPlan(292);
515
516 pinfo = calloc(1,sizeof(info));
517 if (!pinfo) {
518@@ -70,50 +74,54 @@ MAIN(ringBytesTest)
519 if (!ring) {
520 testAbort("epicsRingBytesCreate failed");
521 }
522- check(ring, RINGSIZE);
523+ check(ring, RINGSIZE, 0);
524
525 for (i = 0 ; i < sizeof(put) ; i++)
526 put[i] = i;
527 for(i = 0 ; i < RINGSIZE ; i++) {
528 n = epicsRingBytesPut(ring, put, i);
529 testOk(n==i, "ring put %d", i);
530- check(ring, RINGSIZE-i);
531+ check(ring, RINGSIZE-i, i);
532 n = epicsRingBytesGet(ring, get, i);
533 testOk(n==i, "ring get %d", i);
534- check(ring, RINGSIZE);
535+ check(ring, RINGSIZE, i);
536 testOk(memcmp(put,get,i)==0, "get matches write");
537 }
538
539+ epicsRingBytesResetHighWaterMark(ring);
540+
541 for(i = 0 ; i < RINGSIZE ; i++) {
542 n = epicsRingBytesPut(ring, put+i, 1);
543 testOk(n==1, "ring put 1, %d", i);
544- check(ring, RINGSIZE-1-i);
545+ check(ring, RINGSIZE-1-i, i + 1);
546 }
547 n = epicsRingBytesPut(ring, put+RINGSIZE, 1);
548 testOk(n==0, "put to full ring");
549- check(ring, 0);
550+ check(ring, 0, RINGSIZE);
551 for(i = 0 ; i < RINGSIZE ; i++) {
552 n = epicsRingBytesGet(ring, get+i, 1);
553 testOk(n==1, "ring get 1, %d", i);
554- check(ring, 1+i);
555+ check(ring, 1+i, RINGSIZE);
556 }
557 testOk(memcmp(put,get,RINGSIZE)==0, "get matches write");
558 n = epicsRingBytesGet(ring, get+RINGSIZE, 1);
559 testOk(n==0, "get from empty ring");
560- check(ring, RINGSIZE);
561+ check(ring, RINGSIZE, RINGSIZE);
562+
563+ epicsRingBytesResetHighWaterMark(ring);
564
565 n = epicsRingBytesPut(ring, put, RINGSIZE+1);
566 testOk(n==0, "ring put beyond ring capacity (%d, expected 0)",n);
567- check(ring, RINGSIZE);
568+ check(ring, RINGSIZE, 0);
569 n = epicsRingBytesPut(ring, put, 1);
570 testOk(n==1, "ring put %d", 1);
571- check(ring, RINGSIZE-1);
572+ check(ring, RINGSIZE-1, 1);
573 n = epicsRingBytesPut(ring, put, RINGSIZE);
574 testOk(n==0, "ring put beyond ring capacity (%d, expected 0)",n);
575- check(ring, RINGSIZE-1);
576+ check(ring, RINGSIZE-1, 1);
577 n = epicsRingBytesGet(ring, get, 1);
578 testOk(n==1, "ring get %d", 1);
579- check(ring, RINGSIZE);
580+ check(ring, RINGSIZE, 1);
581
582 epicsRingBytesDelete(ring);
583 epicsEventDestroy(consumerEvent);
584diff --git a/src/libCom/test/ringPointerTest.c b/src/libCom/test/ringPointerTest.c
585index 65a3494..d351708 100644
586--- a/src/libCom/test/ringPointerTest.c
587+++ b/src/libCom/test/ringPointerTest.c
588@@ -64,6 +64,7 @@ static void testSingle(void)
589 testOk1(epicsRingPointerGetFree(ring)==rsize);
590 testOk1(epicsRingPointerGetSize(ring)==rsize);
591 testOk1(epicsRingPointerGetUsed(ring)==0);
592+ testOk1(epicsRingPointerGetHighWaterMark(ring)==0);
593
594 testOk1(epicsRingPointerPop(ring)==NULL);
595
596@@ -75,6 +76,10 @@ static void testSingle(void)
597 testOk1(epicsRingPointerGetFree(ring)==rsize-1);
598 testOk1(epicsRingPointerGetSize(ring)==rsize);
599 testOk1(epicsRingPointerGetUsed(ring)==1);
600+ testOk1(epicsRingPointerGetHighWaterMark(ring)==1);
601+
602+ epicsRingPointerResetHighWaterMark(ring);
603+ testOk1(epicsRingPointerGetHighWaterMark(ring)==1);
604
605 testDiag("Fill it up");
606 for(i=2; i<2*rsize; i++) {
607@@ -92,6 +97,7 @@ static void testSingle(void)
608 testOk1(epicsRingPointerGetFree(ring)==0);
609 testOk1(epicsRingPointerGetSize(ring)==rsize);
610 testOk1(epicsRingPointerGetUsed(ring)==rsize);
611+ testOk1(epicsRingPointerGetHighWaterMark(ring)==rsize);
612
613 testDiag("Drain it out");
614 for(i=1; i<2*rsize; i++) {
615@@ -108,6 +114,7 @@ static void testSingle(void)
616 testOk1(epicsRingPointerGetFree(ring)==rsize);
617 testOk1(epicsRingPointerGetSize(ring)==rsize);
618 testOk1(epicsRingPointerGetUsed(ring)==0);
619+ testOk1(epicsRingPointerGetHighWaterMark(ring)==rsize);
620
621 testDiag("Fill it up again");
622 for(i=2; i<2*rsize; i++) {
623@@ -236,7 +243,7 @@ MAIN(ringPointerTest)
624 {
625 int prio = epicsThreadGetPrioritySelf();
626
627- testPlan(37);
628+ testPlan(42);
629 testSingle();
630 if (prio)
631 epicsThreadSetPriority(epicsThreadGetIdSelf(), epicsThreadPriorityScanLow);

Subscribers

People subscribed via source and target branches