Merge lp:~dirk.zimoch/epics-base/named-soft-events into lp:~epics-core/epics-base/3.14
- named-soft-events
- Merge into 3.14
Status: | Superseded |
---|---|
Proposed branch: | lp:~dirk.zimoch/epics-base/named-soft-events |
Merge into: | lp:~epics-core/epics-base/3.14 |
Diff against target: |
573 lines (+178/-106) (has conflicts) 10 files modified
documentation/RELEASE_NOTES.html (+7/-0) src/db/dbCommon.dbd (+3/-2) src/db/dbIocRegister.c (+2/-2) src/db/dbScan.c (+95/-76) src/db/dbScan.h (+6/-2) src/dev/softDev/devEventSoft.c (+8/-2) src/rec/calcoutRecord.c (+11/-12) src/rec/calcoutRecord.dbd (+10/-1) src/rec/eventRecord.c (+23/-6) src/rec/eventRecord.dbd (+13/-3) Text conflict in documentation/RELEASE_NOTES.html |
To merge this branch: | bzr merge lp:~dirk.zimoch/epics-base/named-soft-events |
Related bugs: |
Reviewer | Review Type | Date Requested | Status |
---|---|---|---|
EPICS Core Developers | Pending | ||
Review via email: mp+26190@code.launchpad.net |
This proposal has been superseded by a proposal from 2010-10-27.
Commit message
Description of the change
Soft events can be strings now.
Example:
record (event, "$(EVENTREC)") {
field (VAL, "Injection Trigger")
}
record (ai, "$(OTHERREC)") {
field (EVNT, "Injection Trigger")
}
record (calcout, "$(CALCREC)") {
field (OEVT, "Injection Trigger")
}
Discussion:
Do we need hashing of the strings to speed up event triggering? Probably not when we can assume a small number of events and event names that differ early.
Andrew Johnson (anj) wrote : | # |
Dirk Zimoch (dirk.zimoch) wrote : | # |
Backward compatibility:
There are two fundamentally different types of "events". Hardware events from an timing/event system and software event. They share only the name. Typically a hardware event is bound to a software event using the event record. The VAL field contains the software event and the INP field contains the hardware event. So far the event record binds a numerical hardware event to a numerical software event (the numbers may be different).
Example:
record (event, "$(NAME)") {
field (DTYP, "APS event receiver")
field (INP, "#C$(CARD) S$(HARDEVENT) @")
field (SCAN, "I/O Intr")
field (VAL, "$(SOFTEVENT)")
field (PRIO, "HIGH")
}
With this modification the records binds a numerical hardware event to a string software event (which may still consist of digits only if you like). Thus the change should be fully backward compatible.
However I will ask Timo and Babak to comment on this issue.
Performance:
I assumed a relatively small number of software event on a single IOC. Thus the O(n) behavior of a linear search (compared to the previous O(1) behaviour) would not be significant. Maybe that assumption is too optimistic.
Nevertheless I will try to re-work that piece of code. I have to see if I can reach O(1), e.g. using a hash value. At least O(log(n)) should be possible using a tree structure instead of a list.
Timo Korhonen (timo-korhonen) wrote : | # |
I have not yet had the time to look at the code, but to not hold up the process, I make here some comments:
Dirk's description of what happens is correct. The 'hardware' event numbers are totally separated from the software event numbers. This can be very confusing to people who are using the system without knowing the deep details.
The event record support implementation, as Dirk explained, does essentially only the linking from a hard to a soft event. For this to work, the event receiver is programmed to create an interrupt upon receiving a particular event number. The interrupt routine calls (via a hook) scanIOrequest for event records with EVR device support (if any).
This applies to event systems from the APS version up to now (Micro-Research 230 and older.)
However, there is the new set of drivers that (mainly) Michael Davidsaver at BNL is developing. He probably should also take a look to see if and how this would affect his implementation (although I think it will be more or less the same story - linking hard and soft events.)
In all cases that I have seen the number of soft events fired from hard events has been really small (3-4) and I cannot think of a use case where one would like to dispatch a large number of different events within one IOC.
In the worst case, if the possibility to refer directly by numbers is kept and thus one can have the O(1) behaviour, I do not see any dangers in this. If O(1) can be achieved with this solution, all the better.
Babak Kalantari (babak-kalantari) wrote : | # |
In general I like the idea of named events. I have faced situations where I was lost about the used soft events in an IOC and it was tedious to track down what HW event fired what SW event to avoild clashes.
My comments:
1) As far as I understood the ordring of adding/removing scan lists is not affected. I think it is important that the position of the event nodes in the lists remains the same in both cases (event / named_event).
2) The new method has introduced an additional lock for each priority list (event_list_lock) and for accessing a node in the event list first the lock of the corresponding priority has be acquired, the priority list be found, and the lock is then released. This will also introduce some performance penalty (in addition to strcmp). In the current method this is achieved by means of 2-dimentional array of [priority][event].
I know that normally number of soft events in an IOC is not big, nevertheless I think one has to study the performance effects of the new appraoch more deeply.
Ralph Lange (ralph-lange) wrote : | # |
I don't like all these string comparisons at run time.
Couldn't the implementation register an event string at first use (using a simple register utility) and then use integer event numbers, so there are no changes in run time behaviour?
mdavidsaver (mdavidsaver) wrote : | # |
> However, there is the new set of drivers that (mainly) Michael Davidsaver at
> BNL is developing. He probably should also take a look to see if and how this
> would affect his implementation (although I think it will be more or less the
> same story - linking hard and soft events.)
Named DB events will cause a small disruption, but the benefit is enough to justify it.
This will only effect me since I am using a longoutRecord instead of an eventRecord. I can't put an eventRecord in an autosave monitor set because it posts the VAL field whenever the event occurs instead of just when the VAL field changes. I could switch to using a stringoutRecord, and restrict 3.14 to strings which can be converted into integers.
mdavidsaver (mdavidsaver) wrote : | # |
> I don't like all these string comparisons at run time.
>
> Couldn't the implementation register an event string at first use (using a
> simple register utility) and then use integer event numbers, so there are no
> changes in run time behaviour?
A simple thing might be to hash the string into an integer. To avoid collisions keep a list of previously hashed values. If a collision is detected then add spaces to the string until it produces a unique hash. Since the table is thus one-to-one it can be used for reverse lookups (ie scanpel()).
An objection to this is that computed hash list grows without bound if someone were to enter lots of different strings (typos?). A way to avoid this would be to separate event registration from use as Ralph suggests. Having for example, "int create_
This would be a significant change to how post_event() is used since the caller would be required to remember the integer key. So I imagine a shortcut "post_event(char*)" which does the hash every time would be needed.
Dirk Zimoch (dirk.zimoch) wrote : | # |
I thought about ways to convert strings to numbers (or pointers) beforehand. The problem is that the event a record posts may change each time the record processes. This is unlikely but nevertheless possible. It would require a special treatment of event.VAL and calcout.OEVT. Whenever the field is changed, the special support could convert the string into a pointer to the scan list which is then stored in another field.
While this is possible with special support, I have no hope for event records reading the event from INP or SIOL. Is there any way to find out if a link target has changed it value without actually comparing the values? In the worst case, the string has to be converted to pointer each time the event record processes when in simulation mode or using soft device support.
This approach means that the code to convert string to pointer has to go into the soft device support and any other device support that modifies event.VAL. My previous approach was backward compatible to existing device supports.
Andrew Johnson (anj) wrote : | # |
Minor issue: We already have a macro EPICS_DEPRECATED in compilerDepende
Dirk Zimoch (dirk.zimoch) wrote : | # |
> Minor issue: We already have a macro EPICS_DEPRECATED in
> compilerDepende
> GNUC. Please use this in dbScan.h instead of your __attribute__() macro, we
> have worked hard to keep architecture-
> code-base only, preferably within libCom.
OK. I missed that macro. And I have a typo in my macro. I forgot the trailing __ for the non-gnu compilers. (has nobody tried to compile this on Windows so far?)
Dirk Zimoch (dirk.zimoch) wrote : | # |
I modified the code to run with O(1) in most cases. Only if the event name actually changed, the event list is searched. Checking for change costs only one strcmp when the value did not change. The check is only done if event.VAL or calcout.VAL is read though event.INP or event.SIOL.
I also replaced the semaphore lock with something quicker.
I will commit the modifications as soon as I have checked everything and have run some tests.
Dirk Zimoch (dirk.zimoch) wrote : | # |
Performance test
I wrote a test that creates 255 events (named 1 ... 255) and processes one of them 1 million times. Here are the results:
mirror-3.14
2.389 seconds
named-soft-events
3.150 seconds
named-soft-events2
2.223 seconds
(running on Ubuntu Linux in a vmWare box on Windows7 on my 1.3 GHz dual core laptop using 1 CPU core for vmWare)
Ben Franksen (bfrk) wrote : | # |
On Dienstag, 26. Oktober 2010, Dirk Zimoch wrote:
> Performance test
>
> I wrote a test that creates 255 events (named 1 ... 255) and
> processes one of them 1 million times. Here are the results:
> mirror-3.14
> 2.389 seconds
> named-soft-events
> 3.150 seconds
> named-soft-events2
> 2.223 seconds
>
> (running on Ubuntu Linux in a vmWare box on Windows7 on my 1.3 GHz
> dual core laptop using 1 CPU core for vmWare)
To me these numbers suggest that your hashing O(1) solution may not be
worth the added complexity and that doing string comparisons on small
strings is not very expensive. Could you do a test with somewhat longer
event names, around 20 to 30 characters long? What is the costs for the
better performance of named-soft-events2 over named-soft-events in
lines of code?
Cheers
Ben
--
"Never confuse what is natural with what is habitual."
J. Lewis Muir (jlmuir) wrote : | # |
On 10/26/10 3:41 PM, Ben F. wrote:
> On Dienstag, 26. Oktober 2010, Dirk Zimoch wrote:
>> Performance test
>>
>> I wrote a test that creates 255 events (named 1 ... 255) and
>> processes one of them 1 million times. Here are the results:
>> mirror-3.14
>> 2.389 seconds
>> named-soft-events
>> 3.150 seconds
>> named-soft-events2
>> 2.223 seconds
>>
>> (running on Ubuntu Linux in a vmWare box on Windows7 on my 1.3 GHz
>> dual core laptop using 1 CPU core for vmWare)
>
> To me these numbers suggest that your hashing O(1) solution may not be
> worth the added complexity and that doing string comparisons on small
> strings is not very expensive. Could you do a test with somewhat longer
> event names, around 20 to 30 characters long? What is the costs for the
> better performance of named-soft-events2 over named-soft-events in
> lines of code?
I'm not against looking at lines of code, but I would not say that fewer
lines of code is always better. To me, readability is the more
important metric. If the code is twice as long as some other code to do
the same thing, but easier to understand, I'd choose the code that's
twice as long and easier to understand.
Lewis
Dirk Zimoch (dirk.zimoch) wrote : | # |
In version 2 I use two function calls, the first one translates the event string to a pointer to the scan list and the second one posts the event with this pointer, which I store in a new SPEC_NOMOD field of the record. This is probably the reason why this approach is even faster than the original implementation with a lookup table. It eliminates one indirection.
The added complexity is that one must make sure to call the translation function only when necessary. That means that the record and/or device support code must find out when the event name has changed. Doing that wrongly reduces the performance to the original approach of doing the string search every time.
Can anyone tell me how I can publish version 2 here in launchpad without overwriting version 1?
And how to publish the test setup?
Dirk Zimoch (dirk.zimoch) wrote : | # |
I tested long event names like "Event with a very long name number 123" (up to 38 chars, the first 35 of them the same for all 255 events)
named-soft-events
3.145 seconds
named-soft-events2
2.381 seconds
More or less the same as with short event names. (Numbers vary a bit from turn to turn)
- 12075. By Dirk Zimoch <dirk@ubuntu>
-
use EPICS_DEPRECATED macro
- 12076. By Dirk Zimoch <dirk@ubuntu>
-
new approach to handle named events efficiently
- 12077. By Dirk Zimoch <dirk@ubuntu>
-
bugfix: devEventSoft read_event with constant input and tse = epicsTimeEventD
eviceTime did not read timestamp - 12078. By Dirk Zimoch <dirk@ubuntu>
-
merged and conflict in RELEASE_NOTES.html solved
- 12079. By Dirk Zimoch <dirk@ubuntu>
-
typo in comment
Ben Franksen (bfrk) wrote : | # |
On Mittwoch, 27. Oktober 2010, Dirk Zimoch wrote:
> I tested long event names like "Event with a very long name number
> 123" (up to 38 chars, the first 35 of them the same for all 255
> events) named-soft-events
> 3.145 seconds
> named-soft-events2
> 2.381 seconds
>
> More or less the same as with short event names. (Numbers vary a bit
> from turn to turn)
Interesting. I'd have expected the ratio between the two versions to
increase for longer strings. I did some research and found that gcc
heavily optimises strcmp, resulting (on certain architectures) in
machine code that uses 16-byte instructions, see
http://
You learn a bit more every day...
Cheers
Ben
--
"Never confuse what is natural with what is habitual."
Dirk Zimoch (dirk.zimoch) wrote : | # |
Unmerged revisions
- 12079. By Dirk Zimoch <dirk@ubuntu>
-
typo in comment
- 12078. By Dirk Zimoch <dirk@ubuntu>
-
merged and conflict in RELEASE_NOTES.html solved
- 12077. By Dirk Zimoch <dirk@ubuntu>
-
bugfix: devEventSoft read_event with constant input and tse = epicsTimeEventD
eviceTime did not read timestamp - 12076. By Dirk Zimoch <dirk@ubuntu>
-
new approach to handle named events efficiently
- 12075. By Dirk Zimoch <dirk@ubuntu>
-
use EPICS_DEPRECATED macro
- 12074. By Dirk Zimoch <dirk@ubuntu>
-
Merged with 3.14
- 12073. By Dirk Zimoch <dirk@ubuntu>
-
release notes updated
- 12072. By Dirk Zimoch <dirk@ubuntu>
-
stringlength and mutex issues fixed
- 12071. By Dirk Zimoch <dirk@ubuntu>
-
string val field
- 12070. By Dirk Zimoch <dirk@ubuntu>
-
OEVT size was missing
Preview Diff
1 | === modified file 'documentation/RELEASE_NOTES.html' |
2 | --- documentation/RELEASE_NOTES.html 2010-10-26 15:49:26 +0000 |
3 | +++ documentation/RELEASE_NOTES.html 2010-10-27 19:58:47 +0000 |
4 | @@ -13,6 +13,7 @@ |
5 | <h2 align="center">Changes between 3.14.11 and 3.14.12</h2> |
6 | <!-- Insert new items immediately below here ... --> |
7 | |
8 | +<<<<<<< TREE |
9 | <h4>Added windows-x64 target</h4> |
10 | |
11 | <p>64-bit binaries for Microsoft Windows platforms can now be built using the |
12 | @@ -121,6 +122,12 @@ |
13 | |
14 | <p>The fields DTYP and INP/OUT can now be specified in any order.</p> |
15 | |
16 | +======= |
17 | +<h4>Named Soft Events</h4> |
18 | + |
19 | +<p>Soft events can now be meaningful strings instead of numbers 1-255. |
20 | + |
21 | +>>>>>>> MERGE-SOURCE |
22 | <h4>Rewrite epicsThreadOnce()</h4> |
23 | |
24 | <p>Michael Davidsaver suggested a better implementation of epicsThreadOnce() |
25 | |
26 | === modified file 'src/db/dbCommon.dbd' |
27 | --- src/db/dbCommon.dbd 2009-04-23 20:35:02 +0000 |
28 | +++ src/db/dbCommon.dbd 2010-10-27 19:58:47 +0000 |
29 | @@ -43,10 +43,11 @@ |
30 | special(SPC_SCAN) |
31 | interest(1) |
32 | } |
33 | - field(EVNT,DBF_SHORT) { |
34 | - prompt("Event Number") |
35 | + field(EVNT,DBF_STRING) { |
36 | + prompt("Event Name") |
37 | promptgroup(GUI_SCAN) |
38 | special(SPC_SCAN) |
39 | + size(40) |
40 | interest(1) |
41 | } |
42 | field(TSE,DBF_SHORT) { |
43 | |
44 | === modified file 'src/db/dbIocRegister.c' |
45 | --- src/db/dbIocRegister.c 2009-01-16 20:50:40 +0000 |
46 | +++ src/db/dbIocRegister.c 2010-10-27 19:58:47 +0000 |
47 | @@ -266,11 +266,11 @@ |
48 | { scanppl(args[0].dval);} |
49 | |
50 | /* scanpel */ |
51 | -static const iocshArg scanpelArg0 = { "event number",iocshArgInt}; |
52 | +static const iocshArg scanpelArg0 = { "event name",iocshArgString}; |
53 | static const iocshArg * const scanpelArgs[1] = {&scanpelArg0}; |
54 | static const iocshFuncDef scanpelFuncDef = {"scanpel",1,scanpelArgs}; |
55 | static void scanpelCallFunc(const iocshArgBuf *args) |
56 | -{ scanpel(args[0].ival);} |
57 | +{ scanpel(args[0].sval);} |
58 | |
59 | /* scanpiol */ |
60 | static const iocshFuncDef scanpiolFuncDef = {"scanpiol",0}; |
61 | |
62 | === modified file 'src/db/dbScan.c' |
63 | --- src/db/dbScan.c 2009-04-03 17:46:26 +0000 |
64 | +++ src/db/dbScan.c 2010-10-27 19:58:47 +0000 |
65 | @@ -101,12 +101,13 @@ |
66 | |
67 | /* EVENT */ |
68 | |
69 | -#define MAX_EVENTS 256 |
70 | -typedef struct event_scan_list { |
71 | - CALLBACK callback; |
72 | - scan_list scan_list; |
73 | -} event_scan_list; |
74 | -static event_scan_list *pevent_list[NUM_CALLBACK_PRIORITIES][MAX_EVENTS]; |
75 | +typedef struct event_list { |
76 | + CALLBACK callback[NUM_CALLBACK_PRIORITIES]; |
77 | + scan_list scan_list[NUM_CALLBACK_PRIORITIES]; |
78 | + struct event_list *next; |
79 | + char event_name[MAX_STRING_SIZE]; |
80 | +} event_list; |
81 | +static event_list * volatile pevent_list[256]; |
82 | |
83 | |
84 | /* IO_EVENT*/ |
85 | @@ -204,35 +205,24 @@ |
86 | recGblRecordError(-1, (void *)precord, |
87 | "scanAdd detected illegal SCAN value"); |
88 | } else if (scan == menuScanEvent) { |
89 | - int evnt; |
90 | + char* eventname; |
91 | int prio; |
92 | - event_scan_list *pesl; |
93 | + event_list *pel; |
94 | |
95 | - evnt = precord->evnt; |
96 | - if (evnt < 0 || evnt >= MAX_EVENTS) { |
97 | + eventname = precord->evnt; |
98 | + if (strlen(eventname) >= MAX_STRING_SIZE) { |
99 | recGblRecordError(S_db_badField, (void *)precord, |
100 | - "scanAdd detected illegal EVNT value"); |
101 | - precord->scan = menuScanPassive; |
102 | + "scanAdd: too long EVNT value"); |
103 | return; |
104 | } |
105 | prio = precord->prio; |
106 | if (prio < 0 || prio >= NUM_CALLBACK_PRIORITIES) { |
107 | recGblRecordError(-1, (void *)precord, |
108 | "scanAdd: illegal prio field"); |
109 | - precord->scan = menuScanPassive; |
110 | return; |
111 | } |
112 | - pesl = pevent_list[prio][evnt]; |
113 | - if (pesl == NULL) { |
114 | - pesl = dbCalloc(1, sizeof(event_scan_list)); |
115 | - pevent_list[prio][evnt] = pesl; |
116 | - pesl->scan_list.lock = epicsMutexMustCreate(); |
117 | - callbackSetCallback(eventCallback, &pesl->callback); |
118 | - callbackSetPriority(prio, &pesl->callback); |
119 | - callbackSetUser(pesl, &pesl->callback); |
120 | - ellInit(&pesl->scan_list.list); |
121 | - } |
122 | - addToList(precord, &pesl->scan_list); |
123 | + pel = eventNameToHandle(eventname); |
124 | + if (pel) addToList(precord, &pel->scan_list[prio]); |
125 | } else if (scan == menuScanI_O_Intr) { |
126 | io_scan_list *piosl = NULL; |
127 | int prio; |
128 | @@ -287,31 +277,25 @@ |
129 | recGblRecordError(-1, (void *)precord, |
130 | "scanDelete detected illegal SCAN value"); |
131 | } else if (scan == menuScanEvent) { |
132 | - int evnt; |
133 | + char* eventname; |
134 | int prio; |
135 | - event_scan_list *pesl; |
136 | + event_list *pel; |
137 | scan_list *psl = 0; |
138 | |
139 | - evnt = precord->evnt; |
140 | - if (evnt < 0 || evnt >= MAX_EVENTS) { |
141 | - recGblRecordError(S_db_badField, (void *)precord, |
142 | - "scanAdd detected illegal EVNT value"); |
143 | - precord->scan = menuScanPassive; |
144 | - return; |
145 | - } |
146 | + eventname = precord->evnt; |
147 | prio = precord->prio; |
148 | if (prio < 0 || prio >= NUM_CALLBACK_PRIORITIES) { |
149 | recGblRecordError(-1, (void *)precord, |
150 | - "scanAdd: illegal prio field"); |
151 | - precord->scan = menuScanPassive; |
152 | + "scanDelete detected illegal PRIO field"); |
153 | return; |
154 | } |
155 | - pesl = pevent_list[prio][evnt]; |
156 | - if (pesl) psl = &pesl->scan_list; |
157 | - if (!pesl || !psl) |
158 | - recGblRecordError(-1, (void *)precord, |
159 | - "scanDelete for bad evnt"); |
160 | - else |
161 | + do /* multitheading: make sure pel is consistent */ |
162 | + pel = pevent_list[0]; |
163 | + while (pel != pevent_list[0]); |
164 | + for (; pel; pel=pel->next) { |
165 | + if (strcmp(pel->event_name, eventname) == 0) break; |
166 | + } |
167 | + if (pel && (psl = &pel->scan_list[prio])) |
168 | deleteFromList(precord, psl); |
169 | } else if (scan == menuScanI_O_Intr) { |
170 | io_scan_list *piosl=NULL; |
171 | @@ -372,21 +356,22 @@ |
172 | return 0; |
173 | } |
174 | |
175 | -int scanpel(int event_number) /* print event list */ |
176 | +int scanpel(char* eventname) /* print event list */ |
177 | { |
178 | char message[80]; |
179 | - int prio, evnt; |
180 | - event_scan_list *pesl; |
181 | - |
182 | - for (evnt = 0; evnt < MAX_EVENTS; evnt++) { |
183 | - if (event_number && evnt<event_number) continue; |
184 | - if (event_number && evnt>event_number) break; |
185 | - for (prio = 0; prio < NUM_CALLBACK_PRIORITIES; prio++) { |
186 | - pesl = pevent_list[prio][evnt]; |
187 | - if (!pesl) continue; |
188 | - if (ellCount(&pesl->scan_list.list) == 0) continue; |
189 | - sprintf(message, "Event %d Priority %s", evnt, priorityName[prio]); |
190 | - printList(&pesl->scan_list, message); |
191 | + int prio; |
192 | + event_list *pel; |
193 | + |
194 | + do /* multitheading: make sure pel is consistent */ |
195 | + pel = pevent_list[0]; |
196 | + while (pel != pevent_list[0]); |
197 | + for (; pel; pel = pel->next) { |
198 | + if (!eventname || strcmp(pel->event_name, eventname) == 0) { |
199 | + for (prio = 0; prio < NUM_CALLBACK_PRIORITIES; prio++) { |
200 | + if (ellCount(&pel->scan_list[prio].list) == 0) continue; |
201 | + sprintf(message, "Event \"%s\" Priority %s", pel->event_name, priorityName[prio]); |
202 | + printList(&pel->scan_list[prio], message); |
203 | + } |
204 | } |
205 | } |
206 | return 0; |
207 | @@ -412,39 +397,73 @@ |
208 | |
209 | |
210 | static void eventCallback(CALLBACK *pcallback) |
211 | { |
212 | - event_scan_list *pesl; |
213 | + scan_list *psl; |
214 | |
215 | - callbackGetUser(pesl, pcallback); |
216 | - scanList(&pesl->scan_list); |
217 | + callbackGetUser(psl, pcallback); |
218 | + scanList(psl); |
219 | } |
220 | |
221 | static void initEvent(void) |
222 | { |
223 | - int evnt, prio; |
224 | - |
225 | +} |
226 | + |
227 | +event_list *eventNameToHandle(char *eventname) |
228 | +{ |
229 | + int prio; |
230 | + event_list *pel; |
231 | + static epicsMutexId lock = NULL; |
232 | + |
233 | + if (!lock) lock = epicsMutexMustCreate(); |
234 | + if (!eventname || eventname[0] == 0) return NULL; |
235 | + epicsMutexMustLock(lock); |
236 | + for (pel = pevent_list[0]; pel; pel=pel->next) { |
237 | + if (strcmp(pel->event_name, eventname) == 0) break; |
238 | + } |
239 | + if (pel == NULL) { |
240 | + pel = dbCalloc(1, sizeof(event_list)); |
241 | + strcpy(pel->event_name, eventname); |
242 | + for (prio = 0; prio < NUM_CALLBACK_PRIORITIES; prio++) { |
243 | + callbackSetUser(&pel->scan_list[prio], &pel->callback[prio]); |
244 | + callbackSetPriority(prio, &pel->callback[prio]); |
245 | + callbackSetCallback(eventCallback, &pel->callback[prio]); |
246 | + pel->scan_list[prio].lock = epicsMutexMustCreate(); |
247 | + ellInit(&pel->scan_list[prio].list); |
248 | + } |
249 | + pel->next=pevent_list[0]; |
250 | + pevent_list[0]=pel; |
251 | + { /* backward compatibility */ |
252 | + char* p; |
253 | + long e = strtol(eventname, &p, 0); |
254 | + if (*p == 0 && e > 0 && e <= 255) |
255 | + pevent_list[e]=pel; |
256 | + } |
257 | + } |
258 | + epicsMutexUnlock(lock); |
259 | + return pel; |
260 | +} |
261 | + |
262 | +void postEvent(event_list *pel) |
263 | +{ |
264 | + int prio; |
265 | + |
266 | + if (scanCtl != ctlRun) return; |
267 | + if (!pel) return; |
268 | for (prio = 0; prio < NUM_CALLBACK_PRIORITIES; prio++) { |
269 | - for (evnt = 0; evnt < MAX_EVENTS; evnt++) { |
270 | - pevent_list[prio][evnt] = NULL; |
271 | - } |
272 | + if (ellCount(&pel->scan_list[prio].list) >0) |
273 | + callbackRequest(&pel->callback[prio]); |
274 | } |
275 | } |
276 | |
277 | +/* backward compatibility */ |
278 | void post_event(int event) |
279 | { |
280 | - int prio; |
281 | - event_scan_list *pesl; |
282 | - |
283 | - if (scanCtl != ctlRun) return; |
284 | - if (event < 0 || event >= MAX_EVENTS) { |
285 | - errMessage(-1, "illegal event passed to post_event"); |
286 | - return; |
287 | - } |
288 | - for (prio=0; prio<NUM_CALLBACK_PRIORITIES; prio++) { |
289 | - pesl = pevent_list[prio][event]; |
290 | - if (!pesl) continue; |
291 | - if (ellCount(&pesl->scan_list.list) >0) |
292 | - callbackRequest((void *)pesl); |
293 | - } |
294 | + event_list* pel; |
295 | + |
296 | + if (event <= 0 || event > 255) return; |
297 | + do { /* multitheading: make sure pel is consistent */ |
298 | + pel = pevent_list[event]; |
299 | + } while (pel != pevent_list[event]); |
300 | + postEvent(pel); |
301 | } |
302 | |
303 | |
304 | void scanIoInit(IOSCANPVT *ppioscanpvt) |
305 | |
306 | === modified file 'src/db/dbScan.h' |
307 | --- src/db/dbScan.h 2010-10-05 19:27:37 +0000 |
308 | +++ src/db/dbScan.h 2010-10-27 19:58:47 +0000 |
309 | @@ -19,6 +19,7 @@ |
310 | |
311 | #include "menuScan.h" |
312 | #include "shareLib.h" |
313 | +#include "compilerDependencies.h" |
314 | |
315 | #ifdef __cplusplus |
316 | extern "C" { |
317 | @@ -36,6 +37,7 @@ |
318 | struct io_scan_list; |
319 | |
320 | typedef struct io_scan_list *IOSCANPVT; |
321 | +typedef struct event_list *EVENTPVT; |
322 | |
323 | struct dbCommon; |
324 | |
325 | @@ -43,7 +45,9 @@ |
326 | epicsShareFunc void scanRun(void); |
327 | epicsShareFunc void scanPause(void); |
328 | |
329 | -epicsShareFunc void post_event(int event); |
330 | +epicsShareFunc EVENTPVT eventNameToHandle(char* event); |
331 | +epicsShareFunc void postEvent(EVENTPVT epvt); |
332 | +epicsShareFunc void post_event(int event) EPICS_DEPRECATED; |
333 | epicsShareFunc void scanAdd(struct dbCommon *); |
334 | epicsShareFunc void scanDelete(struct dbCommon *); |
335 | epicsShareFunc double scanPeriod(int scan); |
336 | @@ -54,7 +58,7 @@ |
337 | epicsShareFunc int scanppl(double rate); |
338 | |
339 | /*print event lists*/ |
340 | -epicsShareFunc int scanpel(int event_number); |
341 | +epicsShareFunc int scanpel(char *event_name); |
342 | |
343 | /*print io_event list*/ |
344 | epicsShareFunc int scanpiol(void); |
345 | |
346 | === modified file 'src/dev/softDev/devEventSoft.c' |
347 | --- src/dev/softDev/devEventSoft.c 2010-10-05 19:27:37 +0000 |
348 | +++ src/dev/softDev/devEventSoft.c 2010-10-27 19:58:47 +0000 |
349 | @@ -51,7 +51,7 @@ |
350 | /* INP must be CONSTANT, PV_LINK, DB_LINK or CA_LINK*/ |
351 | switch (prec->inp.type) { |
352 | case CONSTANT: |
353 | - if (recGblInitConstantLink(&prec->inp, DBF_USHORT, &prec->val)) |
354 | + if (recGblInitConstantLink(&prec->inp, DBF_STRING, &prec->val)) |
355 | prec->udf = FALSE; |
356 | break; |
357 | case PV_LINK: |
358 | @@ -69,9 +69,15 @@ |
359 | static long read_event(eventRecord *prec) |
360 | { |
361 | long status; |
362 | + char newEvent[MAX_STRING_SIZE]; |
363 | |
364 | - status = dbGetLink(&prec->inp, DBR_USHORT, &prec->val, 0, 0); |
365 | + if (prec->inp.type == CONSTANT) return 0; |
366 | + status = dbGetLinkValue(&prec->inp, DBR_STRING, newEvent, 0, 0); |
367 | if (!status) { |
368 | + if (strcmp(newEvent, prec->val) != 0) { |
369 | + strcpy(prec->val, newEvent); |
370 | + prec->epvt = eventNameToHandle(prec->val); |
371 | + } |
372 | prec->udf = FALSE; |
373 | if (prec->tsel.type == CONSTANT && |
374 | prec->tse == epicsTimeEventDeviceTime) |
375 | |
376 | === modified file 'src/rec/calcoutRecord.c' |
377 | --- src/rec/calcoutRecord.c 2010-04-05 18:49:18 +0000 |
378 | +++ src/rec/calcoutRecord.c 2010-10-27 19:58:47 +0000 |
379 | @@ -197,6 +197,8 @@ |
380 | callbackSetUser(prec, &prpvt->checkLinkCb); |
381 | prpvt->cbScheduled = 0; |
382 | |
383 | + prec->epvt = eventNameToHandle(prec->oevt); |
384 | + |
385 | if (pcalcoutDSET->init_record) pcalcoutDSET->init_record(prec); |
386 | prec->pval = prec->val; |
387 | prec->mlst = prec->val; |
388 | @@ -357,6 +359,9 @@ |
389 | } |
390 | db_post_events(prec, plinkValid, DBE_VALUE); |
391 | return 0; |
392 | + case(calcoutRecordOEVT): |
393 | + prec->epvt = eventNameToHandle(prec->oevt); |
394 | + return 0; |
395 | default: |
396 | recGblDbaddrError(S_db_badChoice, paddr, "calc: special"); |
397 | return(S_db_badChoice); |
398 | @@ -539,27 +544,21 @@ |
399 | if (prec->nsev < INVALID_ALARM ) { |
400 | /* Output the value */ |
401 | status = writeValue(prec); |
402 | - /* post event if output event != 0 */ |
403 | - if (prec->oevt > 0) { |
404 | - post_event((int)prec->oevt); |
405 | - } |
406 | + /* post output event if set */ |
407 | + if (prec->epvt) postEvent(prec->epvt); |
408 | } else switch (prec->ivoa) { |
409 | case menuIvoaContinue_normally: |
410 | status = writeValue(prec); |
411 | - /* post event if output event != 0 */ |
412 | - if (prec->oevt > 0) { |
413 | - post_event((int)prec->oevt); |
414 | - } |
415 | + /* post output event if set */ |
416 | + if (prec->epvt) postEvent(prec->epvt); |
417 | break; |
418 | case menuIvoaDon_t_drive_outputs: |
419 | break; |
420 | case menuIvoaSet_output_to_IVOV: |
421 | prec->oval = prec->ivov; |
422 | status = writeValue(prec); |
423 | - /* post event if output event != 0 */ |
424 | - if (prec->oevt > 0) { |
425 | - post_event((int)prec->oevt); |
426 | - } |
427 | + /* post output event if set */ |
428 | + if (prec->epvt) postEvent(prec->epvt); |
429 | break; |
430 | default: |
431 | status = -1; |
432 | |
433 | === modified file 'src/rec/calcoutRecord.dbd' |
434 | --- src/rec/calcoutRecord.dbd 2010-10-04 18:46:09 +0000 |
435 | +++ src/rec/calcoutRecord.dbd 2010-10-27 19:58:47 +0000 |
436 | @@ -255,10 +255,19 @@ |
437 | prompt("OCAL Valid") |
438 | interest(1) |
439 | } |
440 | - field(OEVT,DBF_USHORT) { |
441 | + field(OEVT,DBF_STRING) { |
442 | prompt("Event To Issue") |
443 | promptgroup(GUI_CLOCK) |
444 | + special(SPC_MOD) |
445 | asl(ASL0) |
446 | + size(40) |
447 | + } |
448 | + %#include "dbScan.h" |
449 | + field(EPVT, DBF_NOACCESS) { |
450 | + prompt("Event private") |
451 | + special(SPC_NOMOD) |
452 | + interest(4) |
453 | + extra("EVENTPVT epvt") |
454 | } |
455 | field(IVOA,DBF_MENU) { |
456 | prompt("INVALID output action") |
457 | |
458 | === modified file 'src/rec/eventRecord.c' |
459 | --- src/rec/eventRecord.c 2010-10-05 19:27:37 +0000 |
460 | +++ src/rec/eventRecord.c 2010-10-27 19:58:47 +0000 |
461 | @@ -32,6 +32,7 @@ |
462 | #include "errMdef.h" |
463 | #include "recSup.h" |
464 | #include "recGbl.h" |
465 | +#include "special.h" |
466 | #include "menuYesNo.h" |
467 | #define GEN_SIZE_OFFSET |
468 | #include "eventRecord.h" |
469 | @@ -43,7 +44,7 @@ |
470 | #define initialize NULL |
471 | static long init_record(eventRecord *, int); |
472 | static long process(eventRecord *); |
473 | -#define special NULL |
474 | +static long special(DBADDR *, int); |
475 | static long get_value(eventRecord *, struct valueDes *); |
476 | #define cvt_dbaddr NULL |
477 | #define get_array_info NULL |
478 | @@ -103,9 +104,11 @@ |
479 | } |
480 | |
481 | if (prec->siol.type == CONSTANT) { |
482 | - recGblInitConstantLink(&prec->siol,DBF_USHORT,&prec->sval); |
483 | + recGblInitConstantLink(&prec->siol,DBF_STRING,&prec->sval); |
484 | } |
485 | |
486 | + prec->epvt = eventNameToHandle(prec->val); |
487 | + |
488 | if( (pdset=(struct eventdset *)(prec->dset)) && (pdset->init_record) ) |
489 | status=(*pdset->init_record)(prec); |
490 | return(status); |
491 | @@ -123,7 +126,7 @@ |
492 | if ( !pact && prec->pact ) return(0); |
493 | prec->pact = TRUE; |
494 | |
495 | - if(prec->val>0) post_event((int)prec->val); |
496 | + postEvent(prec->epvt); |
497 | |
498 | recGblGetTimeStamp(prec); |
499 | |
500 | @@ -137,10 +140,21 @@ |
501 | return(status); |
502 | } |
503 | |
504 | + |
505 | |
506 | +static long special(DBADDR *paddr, int after) |
507 | +{ |
508 | + eventRecord *prec = (eventRecord *)paddr->precord; |
509 | + |
510 | + if (!after) return 0; |
511 | + if (dbGetFieldIndex(paddr) == eventRecordVAL) { |
512 | + prec->epvt = eventNameToHandle(prec->val); |
513 | + } |
514 | +} |
515 | |
516 | + |
517 | |
518 | static long get_value(eventRecord *prec, struct valueDes *pvdes) |
519 | { |
520 | - pvdes->field_type = DBF_USHORT; |
521 | + pvdes->field_type = DBF_STRING; |
522 | pvdes->no_elements=1; |
523 | pvdes->pvalue = (void *)(&prec->val); |
524 | return(0); |
525 | @@ -177,10 +191,13 @@ |
526 | return(status); |
527 | } |
528 | if (prec->simm == menuYesNoYES){ |
529 | - status=dbGetLink(&(prec->siol),DBR_USHORT, |
530 | + status=dbGetLink(&(prec->siol),DBR_STRING, |
531 | &(prec->sval),0,0); |
532 | if (status==0) { |
533 | - prec->val=prec->sval; |
534 | + if (strcmp(prec->sval, prec->val) != 0) { |
535 | + strcpy(prec->val, prec->sval); |
536 | + prec->epvt = eventNameToHandle(prec->val); |
537 | + } |
538 | prec->udf=FALSE; |
539 | } |
540 | } else { |
541 | |
542 | === modified file 'src/rec/eventRecord.dbd' |
543 | --- src/rec/eventRecord.dbd 2002-07-12 21:35:43 +0000 |
544 | +++ src/rec/eventRecord.dbd 2010-10-27 19:58:47 +0000 |
545 | @@ -9,10 +9,19 @@ |
546 | #************************************************************************* |
547 | recordtype(event) { |
548 | include "dbCommon.dbd" |
549 | - field(VAL,DBF_USHORT) { |
550 | - prompt("Event Number To Post") |
551 | + field(VAL,DBF_STRING) { |
552 | + prompt("Event Name To Post") |
553 | promptgroup(GUI_INPUTS) |
554 | + special(SPC_MOD) |
555 | asl(ASL0) |
556 | + size(40) |
557 | + } |
558 | + %#include "dbScan.h" |
559 | + field(EPVT, DBF_NOACCESS) { |
560 | + prompt("Event private") |
561 | + special(SPC_NOMOD) |
562 | + interest(4) |
563 | + extra("EVENTPVT epvt") |
564 | } |
565 | field(INP,DBF_INLINK) { |
566 | prompt("Input Specification") |
567 | @@ -24,8 +33,9 @@ |
568 | promptgroup(GUI_INPUTS) |
569 | interest(1) |
570 | } |
571 | - field(SVAL,DBF_USHORT) { |
572 | + field(SVAL,DBF_STRING) { |
573 | prompt("Simulation Value") |
574 | + size(40) |
575 | } |
576 | field(SIML,DBF_INLINK) { |
577 | prompt("Sim Mode Location") |
What effect would merging this code have on sites that use an event timing system?
All distributed event networks pass events around as 8-bit numbers, and I believe the event receiver code announces such an event by calling post_event() with the relevant number.
I do not like the linear search for the posted event name, calling strcmp() against every item on the event_scan_list until the right name is found. This would have a fairly significant effect on performance, which matters on systems where the events happen quickly.
I want this code to be reviewed by someone with more knowledge of how those event systems work, but I don't currently think that this is the right solution.