Merge ~bfrk/epics-base:scalar-get-optimization into ~epics-core/epics-base/+git/epics-base:7.0
- Git
- lp:~bfrk/epics-base
- scalar-get-optimization
- Merge into 7.0
Status: | Superseded |
---|---|
Proposed branch: | ~bfrk/epics-base:scalar-get-optimization |
Merge into: | ~epics-core/epics-base/+git/epics-base:7.0 |
Prerequisite: | ~bfrk/epics-base:write-filters |
Diff against target: |
538 lines (+121/-140) 10 files modified
modules/database/src/ioc/db/dbAccess.c (+6/-6) modules/database/src/ioc/db/dbAccessDefs.h (+1/-1) modules/database/src/ioc/db/dbChannel.c (+52/-4) modules/database/src/ioc/db/dbChannel.h (+9/-5) modules/database/src/ioc/db/dbDbLink.c (+21/-56) modules/database/src/ioc/db/dbTest.c (+16/-16) modules/database/src/ioc/db/dbUnitTest.c (+2/-2) modules/database/src/ioc/db/db_field_log.h (+14/-2) modules/database/src/ioc/dbStatic/link.h (+0/-3) modules/database/src/std/filters/arr.c (+0/-45) |
Related bugs: |
Reviewer | Review Type | Date Requested | Status |
---|---|---|---|
EPICS Core Developers | Pending | ||
Review via email: mp+381307@code.launchpad.net |
This proposal has been superseded by a proposal from 2020-04-14.
Commit message
Description of the change
The idea here is to make the special optimization for one-element get requests without options available to all code that calls dbChannelGet. It started out as an attempt to disentangle this logic from dbDbGetValue (this goal is indeed achieved) and then moved on from that point.
Ben Franksen (bfrk) wrote : | # |
- 4559b17... by Ben Franksen
-
refactor db_field_log and filters to get rid of dbfl_type_rec
This refactor simplifies and streamlines the code associated with server
side filters. Apart from immediate benefits (clearer code, less duplication)
it is also hoped that this will make it easier to add write filters.The data pointer dbfl_ref.field can now either point to a copy owned by a
filter, or it can point to the original data owned by a record. In the
latter case, the dbfl_ref.dtor is NULL.The dbExtractArray* functions are unified to the single function
dbExtractArray and stripped of conversion functionality. This is redundant
because we always call dbGet after applying filters, which takes care of
conversion. Accordingly, dbChannelMakeArrayCopy is now obsolete and its
single use (in the ts filter) replaced with dbExtractArray. Instead, we add
the helper function dbChannelGetArrayInfo to wrap the common boilerplate
around calls to the get_array_info method, used in both arr.c and ts.c. - ab53d16... by Ben Franksen
-
make it clearer what the result of wrapArrayIndices will be
- ccd096b... by Ben Franksen
-
add macro dbfl_has_copy to db_field_log.h and use it in dbAccess.c
It encapsulates the slightly tricky logic to decide whether a pointer
to a db_field_log has ownership of the data or not. - bdad846... by Ben Franksen
-
Merge branch 'dbChannelForDB
Links' into work
mdavidsaver (mdavidsaver) wrote : | # |
Do you consider this MR ready? If so, please make an Approve review yourself. Or Needs Fixing if you have additional work planned.
wrt. changing the signature of dbGetField(), what is your plan for handling API compatibility with existing out of tree code? A quick search doesn't find as many usages as I was expecting, but there are some (eg. caPutLog)
https:/
I've a similar question about dbChannelGet*(), although this should have an easier answer.
In terms of the content, I'm not so enthusiastic about bulking up dbChannelGet() which has so far been a thin wrapper around dbGet(). Why can't this optimization be applied to dbGet()?
If this optimization to dbChannelGet() is judged worthwhile, some test coverage will be necessary.
Unmerged commits
- b5cf567... by Ben Franksen
-
move optimization for scalar requests from link to channel
The optimization is now available for all dbChannels, instead of only for DB
links, and also for requests involving filters. The code more clearly points
out where we optimize this special case and under what conditions. - ccd096b... by Ben Franksen
-
add macro dbfl_has_copy to db_field_log.h and use it in dbAccess.c
It encapsulates the slightly tricky logic to decide whether a pointer
to a db_field_log has ownership of the data or not. - 4559b17... by Ben Franksen
-
refactor db_field_log and filters to get rid of dbfl_type_rec
This refactor simplifies and streamlines the code associated with server
side filters. Apart from immediate benefits (clearer code, less duplication)
it is also hoped that this will make it easier to add write filters.The data pointer dbfl_ref.field can now either point to a copy owned by a
filter, or it can point to the original data owned by a record. In the
latter case, the dbfl_ref.dtor is NULL.The dbExtractArray* functions are unified to the single function
dbExtractArray and stripped of conversion functionality. This is redundant
because we always call dbGet after applying filters, which takes care of
conversion. Accordingly, dbChannelMakeArrayCopy is now obsolete and its
single use (in the ts filter) replaced with dbExtractArray. Instead, we add
the helper function dbChannelGetArrayInfo to wrap the common boilerplate
around calls to the get_array_info method, used in both arr.c and ts.c. - ab53d16... by Ben Franksen
-
make it clearer what the result of wrapArrayIndices will be
- bdad846... by Ben Franksen
-
Merge branch 'dbChannelForDB
Links' into work - 1457519... by Dirk Zimoch
-
clean up code structure
- e419914... by Dirk Zimoch
-
Release notes updated
- ed0b4d4... by Dirk Zimoch
-
set number of planned link filter tests
- fc61f20... by Dirk Zimoch
-
removed unnecessary recGblSetSevr call
- 8a8de85... by Dirk Zimoch
-
re-order link filter tests to alternate between success and failure
Preview Diff
1 | diff --git a/modules/database/src/ioc/db/dbAccess.c b/modules/database/src/ioc/db/dbAccess.c |
2 | index 3709049..4fb56cc 100644 |
3 | --- a/modules/database/src/ioc/db/dbAccess.c |
4 | +++ b/modules/database/src/ioc/db/dbAccess.c |
5 | @@ -867,14 +867,14 @@ static long getAttrValue(DBADDR *paddr, short dbrType, |
6 | return 0; |
7 | } |
8 | |
9 | -long dbGetField(DBADDR *paddr, short dbrType, |
10 | - void *pbuffer, long *options, long *nRequest) |
11 | +long dbGetField(DBADDR *paddr,short dbrType, |
12 | + void *pbuffer, long *options, long *nRequest, void *pflin) |
13 | { |
14 | dbCommon *precord = paddr->precord; |
15 | long status = 0; |
16 | |
17 | dbScanLock(precord); |
18 | - status = dbGet(paddr, dbrType, pbuffer, options, nRequest, NULL); |
19 | + status = dbGet(paddr, dbrType, pbuffer, options, nRequest, pflin); |
20 | dbScanUnlock(precord); |
21 | return status; |
22 | } |
23 | @@ -903,10 +903,10 @@ long dbGet(DBADDR *paddr, short dbrType, |
24 | no_elements = capacity = pfl->no_elements; |
25 | } |
26 | |
27 | - /* Update field info from record |
28 | - * may modify paddr->pfield |
29 | + /* Update field info from record (if neccessary); |
30 | + * may modify paddr->pfield. |
31 | */ |
32 | - if ((!pfl || (pfl->type==dbfl_type_ref && !pfl->u.r.dtor)) && |
33 | + if (!dbfl_has_copy(pfl) && |
34 | paddr->pfldDes->special == SPC_DBADDR && |
35 | (prset = dbGetRset(paddr)) && |
36 | prset->get_array_info) { |
37 | diff --git a/modules/database/src/ioc/db/dbAccessDefs.h b/modules/database/src/ioc/db/dbAccessDefs.h |
38 | index 4a95989..805dfd4 100644 |
39 | --- a/modules/database/src/ioc/db/dbAccessDefs.h |
40 | +++ b/modules/database/src/ioc/db/dbAccessDefs.h |
41 | @@ -243,7 +243,7 @@ epicsShareFunc devSup* dbDTYPtoDevSup(dbRecordType *prdes, int dtyp); |
42 | epicsShareFunc devSup* dbDSETtoDevSup(dbRecordType *prdes, dset *pdset); |
43 | epicsShareFunc long dbGetField( |
44 | struct dbAddr *,short dbrType,void *pbuffer,long *options, |
45 | - long *nRequest); |
46 | + long *nRequest,void *pfl); |
47 | epicsShareFunc long dbGet( |
48 | struct dbAddr *,short dbrType,void *pbuffer,long *options, |
49 | long *nRequest,void *pfl); |
50 | diff --git a/modules/database/src/ioc/db/dbChannel.c b/modules/database/src/ioc/db/dbChannel.c |
51 | index 4c1215c..551cb39 100644 |
52 | --- a/modules/database/src/ioc/db/dbChannel.c |
53 | +++ b/modules/database/src/ioc/db/dbChannel.c |
54 | @@ -31,6 +31,7 @@ |
55 | #include "dbBase.h" |
56 | #include "dbChannel.h" |
57 | #include "dbCommon.h" |
58 | +#include "dbConvertFast.h" |
59 | #include "dbEvent.h" |
60 | #include "dbLock.h" |
61 | #include "dbStaticLib.h" |
62 | @@ -623,14 +624,61 @@ long dbChannelOpen(dbChannel *chan) |
63 | } |
64 | |
65 | /* Only use dbChannelGet() if the record is already locked. */ |
66 | -long dbChannelGet(dbChannel *chan, short type, void *pbuffer, |
67 | - long *options, long *nRequest, void *pfl) |
68 | +long dbChannelGet(dbChannel *chan, short dbrType, void *pbuffer, |
69 | + long *options, long *nRequest, db_field_log *pfl) |
70 | { |
71 | - return dbGet(&chan->addr, type, pbuffer, options, nRequest, pfl); |
72 | + dbAddr addr = chan->addr; /* structure copy */ |
73 | + int pfl_has_copy = pfl && (pfl->type != dbfl_type_ref || pfl->u.r.dtor); |
74 | + |
75 | + if (pfl) { |
76 | + addr.field_size = pfl->field_size; |
77 | + addr.field_type = pfl->field_type; |
78 | + addr.no_elements = pfl->no_elements; |
79 | + switch ((enum dbfl_type)pfl->type) { |
80 | + case dbfl_type_val: addr.pfield = &pfl->u.v.field; break; |
81 | + case dbfl_type_ref: addr.pfield = pfl->u.r.field; break; |
82 | + } |
83 | + } |
84 | + |
85 | + /* |
86 | + * Try to optimize scalar requests by caching (just) the conversion |
87 | + * routine. This is possible only if we have no options, the field and the |
88 | + * request have exactly one element, and we have no special processing to |
89 | + * do. Note that if the db_field_log has already copied the data, dbGet |
90 | + * does not call get_array_info. |
91 | + */ |
92 | + if ((options && *options) |
93 | + || addr.no_elements > 1 |
94 | + || (nRequest && *nRequest > 1) |
95 | + || (!pfl_has_copy && addr.special == SPC_DBADDR) |
96 | + || addr.special == SPC_ATTRIBUTE) |
97 | + { |
98 | + chan->getCvt = NULL; |
99 | + return dbGet(&addr, dbrType, pbuffer, options, nRequest, pfl); |
100 | + } |
101 | + |
102 | + /* |
103 | + * Optimization for scalar requests with no options, |
104 | + * no special processing, and where the channel has only one element. |
105 | + */ |
106 | + if (chan->getCvt && chan->lastGetdbrType == dbrType) { |
107 | + return chan->getCvt(addr.pfield, pbuffer, &addr); |
108 | + } else { |
109 | + unsigned short dbfType = addr.field_type; |
110 | + |
111 | + if (dbrType < 0 || dbrType > DBR_ENUM || dbfType > DBF_DEVICE) |
112 | + return S_db_badDbrtype; |
113 | + |
114 | + chan->getCvt = dbFastGetConvertRoutine[dbfType][dbrType]; |
115 | + chan->lastGetdbrType = dbrType; |
116 | + if (nRequest) |
117 | + *nRequest = addr.no_elements; |
118 | + return chan->getCvt(addr.pfield, pbuffer, &addr); |
119 | + } |
120 | } |
121 | |
122 | long dbChannelGetField(dbChannel *chan, short dbrType, void *pbuffer, |
123 | - long *options, long *nRequest, void *pfl) |
124 | + long *options, long *nRequest, db_field_log *pfl) |
125 | { |
126 | dbCommon *precord = chan->addr.precord; |
127 | long status = 0; |
128 | diff --git a/modules/database/src/ioc/db/dbChannel.h b/modules/database/src/ioc/db/dbChannel.h |
129 | index 7bfc9b0..2dc929b 100644 |
130 | --- a/modules/database/src/ioc/db/dbChannel.h |
131 | +++ b/modules/database/src/ioc/db/dbChannel.h |
132 | @@ -49,6 +49,8 @@ typedef struct evSubscrip { |
133 | |
134 | typedef struct chFilter chFilter; |
135 | |
136 | +typedef long fastConvert(void *from, void *to, const dbAddr *paddr); |
137 | + |
138 | /* A dbChannel points to a record field, and can have multiple filters */ |
139 | typedef struct dbChannel { |
140 | const char *name; |
141 | @@ -59,14 +61,16 @@ typedef struct dbChannel { |
142 | ELLLIST filters; /* list of filters as created from JSON */ |
143 | ELLLIST pre_chain; /* list of filters to be called pre-event-queue */ |
144 | ELLLIST post_chain; /* list of filters to be called post-event-queue */ |
145 | + /* Support for optimizing scalar requests */ |
146 | + fastConvert *getCvt; /* fast get conversion function */ |
147 | + short lastGetdbrType; /* last dbChannelGet dbrType */ |
148 | } dbChannel; |
149 | |
150 | /* Prototype for the channel event function that is called in filter stacks |
151 | * |
152 | * When invoked the scan lock for the record associated with 'chan' _may_ be locked. |
153 | - * If pLog->type==dbfl_type_ref and pLog->u.r.dtor is NULL, then dbScanLock() must |
154 | - * be called before accessing the data, as this indicates the data is owned by the |
155 | - * record. |
156 | + * Unless dbfl_has_copy(pLog), it must call dbScanLock before accessing the data, |
157 | + * as this indicates the data is still owned by the record. |
158 | * |
159 | * This function has ownership of the field log pLog, if it wishes to discard |
160 | * this update it should free the field log with db_delete_field_log() and |
161 | @@ -208,9 +212,9 @@ epicsShareExtern unsigned short dbDBRnewToDBRold[]; |
162 | |
163 | |
164 | epicsShareFunc long dbChannelGet(dbChannel *chan, short type, |
165 | - void *pbuffer, long *options, long *nRequest, void *pfl); |
166 | + void *pbuffer, long *options, long *nRequest, db_field_log *pfl); |
167 | epicsShareFunc long dbChannelGetField(dbChannel *chan, short type, |
168 | - void *pbuffer, long *options, long *nRequest, void *pfl); |
169 | + void *pbuffer, long *options, long *nRequest, db_field_log *pfl); |
170 | epicsShareFunc long dbChannelPut(dbChannel *chan, short type, |
171 | const void *pbuffer, long nRequest); |
172 | epicsShareFunc long dbChannelPutField(dbChannel *chan, short type, |
173 | diff --git a/modules/database/src/ioc/db/dbDbLink.c b/modules/database/src/ioc/db/dbDbLink.c |
174 | index 4e1dc69..611ee7f 100644 |
175 | --- a/modules/database/src/ioc/db/dbDbLink.c |
176 | +++ b/modules/database/src/ioc/db/dbDbLink.c |
177 | @@ -57,7 +57,6 @@ |
178 | #include "dbBase.h" |
179 | #include "dbBkpt.h" |
180 | #include "dbCommonPvt.h" |
181 | -#include "dbConvertFast.h" |
182 | #include "dbConvert.h" |
183 | #include "db_field_log.h" |
184 | #include "db_access_routines.h" |
185 | @@ -134,9 +133,7 @@ static void dbDbRemoveLink(struct dbLocker *locker, struct link *plink) |
186 | /* locker is NULL when an isolated IOC is closing its links */ |
187 | if (locker) { |
188 | plink->value.pv_link.pvt = 0; |
189 | - plink->value.pv_link.getCvt = 0; |
190 | plink->value.pv_link.pvlMask = 0; |
191 | - plink->value.pv_link.lastGetdbrType = 0; |
192 | ellDelete(&precord->bklnk, &plink->value.pv_link.backlinknode); |
193 | dbLockSetSplit(locker, plink->precord, precord); |
194 | } |
195 | @@ -166,10 +163,10 @@ static long dbDbGetValue(struct link *plink, short dbrType, void *pbuffer, |
196 | { |
197 | struct pv_link *ppv_link = &plink->value.pv_link; |
198 | dbChannel *chan = linkChannel(plink); |
199 | - DBADDR *paddr = &chan->addr; |
200 | dbCommon *precord = plink->precord; |
201 | db_field_log *pfl = NULL; |
202 | long status; |
203 | + long nRequest = 1; /* used if pnRequest == NULL */ |
204 | |
205 | /* scan passive records if link is process passive */ |
206 | if (ppv_link->pvlMask & pvlOptPP) { |
207 | @@ -178,59 +175,27 @@ static long dbDbGetValue(struct link *plink, short dbrType, void *pbuffer, |
208 | return status; |
209 | } |
210 | |
211 | - if (ppv_link->getCvt && ppv_link->lastGetdbrType == dbrType) |
212 | - { |
213 | - /* shortcut: scalar with known conversion, no filter */ |
214 | - status = ppv_link->getCvt(dbChannelField(chan), pbuffer, paddr); |
215 | - } |
216 | - else if (dbChannelFinalElements(chan) == 1 && (!pnRequest || *pnRequest == 1) |
217 | - && dbChannelSpecial(chan) != SPC_DBADDR |
218 | - && dbChannelSpecial(chan) != SPC_ATTRIBUTE |
219 | - && ellCount(&chan->filters) == 0) |
220 | - { |
221 | - /* simple scalar: set up shortcut */ |
222 | - unsigned short dbfType = dbChannelFinalFieldType(chan); |
223 | - |
224 | - if (dbrType < 0 || dbrType > DBR_ENUM || dbfType > DBF_DEVICE) |
225 | - return S_db_badDbrtype; |
226 | - |
227 | - ppv_link->getCvt = dbFastGetConvertRoutine[dbfType][dbrType]; |
228 | - ppv_link->lastGetdbrType = dbrType; |
229 | - status = ppv_link->getCvt(dbChannelField(chan), pbuffer, paddr); |
230 | - } |
231 | - else |
232 | - { |
233 | - /* filter, array, or special */ |
234 | - ppv_link->getCvt = NULL; |
235 | - |
236 | - /* |
237 | - * For the moment, empty arrays are not supported by EPICS. |
238 | - * See the remark in src/std/filters/arr.c for details. |
239 | - */ |
240 | - if (dbChannelFinalElements(chan) <= 0) /* empty array request */ |
241 | - return S_db_badField; |
242 | - |
243 | - if (ellCount(&chan->filters)) { |
244 | - /* If filters are involved in a read, create field log and run filters */ |
245 | - pfl = db_create_read_log(chan); |
246 | - if (!pfl) |
247 | - return S_db_noMemory; |
248 | - |
249 | - pfl = dbChannelRunPreChain(chan, pfl); |
250 | - pfl = dbChannelRunPostChain(chan, pfl); |
251 | - } |
252 | - |
253 | - status = dbChannelGet(chan, dbrType, pbuffer, NULL, pnRequest, pfl); |
254 | - |
255 | - if (pfl) |
256 | - db_delete_field_log(pfl); |
257 | - |
258 | - if (status) |
259 | - return status; |
260 | - |
261 | - if (pnRequest && *pnRequest <= 0) /* empty array result */ |
262 | - return S_db_badField; |
263 | + /* If filters are involved in a read, create field log and run filters */ |
264 | + if (ellCount(&chan->filters)) { |
265 | + pfl = db_create_read_log(chan); |
266 | + if (!pfl) |
267 | + return S_db_noMemory; |
268 | + pfl = dbChannelRunPreChain(chan, pfl); |
269 | + pfl = dbChannelRunPostChain(chan, pfl); |
270 | } |
271 | + if (!pnRequest) |
272 | + pnRequest = &nRequest; |
273 | + status = dbChannelGet(chan, dbrType, pbuffer, NULL, pnRequest, pfl); |
274 | + if (pfl) |
275 | + db_delete_field_log(pfl); |
276 | + if (status) |
277 | + return status; |
278 | + /* |
279 | + * For the moment, empty arrays are not supported by EPICS. |
280 | + * See the remark in src/std/filters/arr.c for details. |
281 | + */ |
282 | + if (*pnRequest <= 0) /* empty array result */ |
283 | + return S_db_badField; |
284 | |
285 | if (!status && precord != dbChannelRecord(chan)) |
286 | recGblInheritSevr(plink->value.pv_link.pvlMask & pvlOptMsMode, |
287 | diff --git a/modules/database/src/ioc/db/dbTest.c b/modules/database/src/ioc/db/dbTest.c |
288 | index 206d6c9..1193954 100644 |
289 | --- a/modules/database/src/ioc/db/dbTest.c |
290 | +++ b/modules/database/src/ioc/db/dbTest.c |
291 | @@ -341,14 +341,14 @@ long dbgf(const char *pname) |
292 | no_elements = MIN(addr.no_elements, sizeof(buffer)/addr.field_size); |
293 | if (addr.dbr_field_type == DBR_ENUM) { |
294 | long status = dbGetField(&addr, DBR_STRING, pbuffer, |
295 | - &options, &no_elements); |
296 | + &options, &no_elements, NULL); |
297 | |
298 | printBuffer(status, DBR_STRING, pbuffer, 0L, 0L, |
299 | no_elements, &msg_Buff, 10); |
300 | } |
301 | else { |
302 | long status = dbGetField(&addr, addr.dbr_field_type, pbuffer, |
303 | - &options, &no_elements); |
304 | + &options, &no_elements, NULL); |
305 | |
306 | printBuffer(status, addr.dbr_field_type, pbuffer, 0L, 0L, |
307 | no_elements, &msg_Buff, 10); |
308 | @@ -487,7 +487,7 @@ long dbtgf(const char *pname) |
309 | ret_options = req_options; |
310 | no_elements = 0; |
311 | status = dbGetField(&addr, addr.dbr_field_type, pbuffer, |
312 | - &ret_options, &no_elements); |
313 | + &ret_options, &no_elements, NULL); |
314 | printBuffer(status, addr.dbr_field_type, pbuffer, |
315 | req_options, ret_options, no_elements, pMsgBuff, tab_size); |
316 | |
317 | @@ -496,62 +496,62 @@ long dbtgf(const char *pname) |
318 | |
319 | dbr_type = DBR_STRING; |
320 | no_elements = MIN(addr.no_elements,((sizeof(buffer))/MAX_STRING_SIZE)); |
321 | - status = dbGetField(&addr,dbr_type,pbuffer,&ret_options,&no_elements); |
322 | + status = dbGetField(&addr,dbr_type,pbuffer,&ret_options,&no_elements,NULL); |
323 | printBuffer(status,dbr_type,pbuffer,0L,0L,no_elements,pMsgBuff,tab_size); |
324 | |
325 | dbr_type = DBR_CHAR; |
326 | no_elements = MIN(addr.no_elements,((sizeof(buffer))/sizeof(epicsInt8))); |
327 | - status = dbGetField(&addr,dbr_type,pbuffer,&ret_options,&no_elements); |
328 | + status = dbGetField(&addr,dbr_type,pbuffer,&ret_options,&no_elements,NULL); |
329 | printBuffer(status,dbr_type,pbuffer,0L,0L,no_elements,pMsgBuff,tab_size); |
330 | |
331 | dbr_type = DBR_UCHAR; |
332 | no_elements = MIN(addr.no_elements,((sizeof(buffer))/sizeof(epicsUInt8))); |
333 | - status = dbGetField(&addr,dbr_type,pbuffer,&ret_options,&no_elements); |
334 | + status = dbGetField(&addr,dbr_type,pbuffer,&ret_options,&no_elements,NULL); |
335 | printBuffer(status,dbr_type,pbuffer,0L,0L,no_elements,pMsgBuff,tab_size); |
336 | |
337 | dbr_type = DBR_SHORT; |
338 | no_elements = MIN(addr.no_elements,((sizeof(buffer))/sizeof(epicsInt16))); |
339 | - status = dbGetField(&addr,dbr_type,pbuffer,&ret_options,&no_elements); |
340 | + status = dbGetField(&addr,dbr_type,pbuffer,&ret_options,&no_elements,NULL); |
341 | printBuffer(status,dbr_type,pbuffer,0L,0L,no_elements,pMsgBuff,tab_size); |
342 | |
343 | dbr_type = DBR_USHORT; |
344 | no_elements = MIN(addr.no_elements,((sizeof(buffer))/sizeof(epicsUInt16))); |
345 | - status = dbGetField(&addr,dbr_type,pbuffer,&ret_options,&no_elements); |
346 | + status = dbGetField(&addr,dbr_type,pbuffer,&ret_options,&no_elements,NULL); |
347 | printBuffer(status,dbr_type,pbuffer,0L,0L,no_elements,pMsgBuff,tab_size); |
348 | |
349 | dbr_type = DBR_LONG; |
350 | no_elements = MIN(addr.no_elements,((sizeof(buffer))/sizeof(epicsInt32))); |
351 | - status = dbGetField(&addr,dbr_type,pbuffer,&ret_options,&no_elements); |
352 | + status = dbGetField(&addr,dbr_type,pbuffer,&ret_options,&no_elements,NULL); |
353 | printBuffer(status,dbr_type,pbuffer,0L,0L,no_elements,pMsgBuff,tab_size); |
354 | |
355 | dbr_type = DBR_ULONG; |
356 | no_elements = MIN(addr.no_elements,((sizeof(buffer))/sizeof(epicsUInt32))); |
357 | - status = dbGetField(&addr,dbr_type,pbuffer,&ret_options,&no_elements); |
358 | + status = dbGetField(&addr,dbr_type,pbuffer,&ret_options,&no_elements,NULL); |
359 | printBuffer(status,dbr_type,pbuffer,0L,0L,no_elements,pMsgBuff,tab_size); |
360 | |
361 | dbr_type = DBR_INT64; |
362 | no_elements = MIN(addr.no_elements,((sizeof(buffer))/sizeof(epicsInt64))); |
363 | - status = dbGetField(&addr,dbr_type,pbuffer,&ret_options,&no_elements); |
364 | + status = dbGetField(&addr,dbr_type,pbuffer,&ret_options,&no_elements,NULL); |
365 | printBuffer(status,dbr_type,pbuffer,0L,0L,no_elements,pMsgBuff,tab_size); |
366 | |
367 | dbr_type = DBR_UINT64; |
368 | no_elements = MIN(addr.no_elements,((sizeof(buffer))/sizeof(epicsUInt64))); |
369 | - status = dbGetField(&addr,dbr_type,pbuffer,&ret_options,&no_elements); |
370 | + status = dbGetField(&addr,dbr_type,pbuffer,&ret_options,&no_elements,NULL); |
371 | printBuffer(status,dbr_type,pbuffer,0L,0L,no_elements,pMsgBuff,tab_size); |
372 | |
373 | dbr_type = DBR_FLOAT; |
374 | no_elements = MIN(addr.no_elements,((sizeof(buffer))/sizeof(epicsFloat32))); |
375 | - status = dbGetField(&addr,dbr_type,pbuffer,&ret_options,&no_elements); |
376 | + status = dbGetField(&addr,dbr_type,pbuffer,&ret_options,&no_elements,NULL); |
377 | printBuffer(status,dbr_type,pbuffer,0L,0L,no_elements,pMsgBuff,tab_size); |
378 | |
379 | dbr_type = DBR_DOUBLE; |
380 | no_elements = MIN(addr.no_elements,((sizeof(buffer))/sizeof(epicsFloat64))); |
381 | - status = dbGetField(&addr,dbr_type,pbuffer,&ret_options,&no_elements); |
382 | + status = dbGetField(&addr,dbr_type,pbuffer,&ret_options,&no_elements,NULL); |
383 | printBuffer(status,dbr_type,pbuffer,0L,0L,no_elements,pMsgBuff,tab_size); |
384 | |
385 | dbr_type = DBR_ENUM; |
386 | no_elements = MIN(addr.no_elements,((sizeof(buffer))/sizeof(epicsEnum16))); |
387 | - status = dbGetField(&addr,dbr_type,pbuffer,&ret_options,&no_elements); |
388 | + status = dbGetField(&addr,dbr_type,pbuffer,&ret_options,&no_elements,NULL); |
389 | printBuffer(status,dbr_type,pbuffer,0L,0L,no_elements,pMsgBuff,tab_size); |
390 | |
391 | pmsg[0] = '\0'; |
392 | @@ -651,7 +651,7 @@ long dbtpf(const char *pname, const char *pvalue) |
393 | |
394 | printf("Put as DBR_%-6s Ok, result as ", dbr[put_type]); |
395 | status = dbGetField(&addr, addr.dbr_field_type, pbuffer, |
396 | - &options, &no_elements); |
397 | + &options, &no_elements, NULL); |
398 | printBuffer(status, addr.dbr_field_type, pbuffer, 0L, 0L, |
399 | no_elements, pMsgBuff, tab_size); |
400 | } |
401 | diff --git a/modules/database/src/ioc/db/dbUnitTest.c b/modules/database/src/ioc/db/dbUnitTest.c |
402 | index 458a28b..6846ef5 100644 |
403 | --- a/modules/database/src/ioc/db/dbUnitTest.c |
404 | +++ b/modules/database/src/ioc/db/dbUnitTest.c |
405 | @@ -197,7 +197,7 @@ void testdbVGetFieldEqual(const char* pv, short dbrType, va_list ap) |
406 | return; |
407 | } |
408 | |
409 | - status = dbGetField(&addr, dbrType, pod.bytes, NULL, &nReq); |
410 | + status = dbGetField(&addr, dbrType, pod.bytes, NULL, &nReq, NULL); |
411 | if (status) { |
412 | testFail("dbGetField(\"%s\", %d, ...) -> %#lx (%s)", pv, dbrType, status, errSymMsg(status)); |
413 | return; |
414 | @@ -270,7 +270,7 @@ void testdbGetArrFieldEqual(const char* pv, short dbfType, long nRequest, unsign |
415 | return; |
416 | } |
417 | |
418 | - status = dbGetField(&addr, dbfType, gbuf, NULL, &nRequest); |
419 | + status = dbGetField(&addr, dbfType, gbuf, NULL, &nRequest, NULL); |
420 | if (status) { |
421 | testFail("dbGetField(\"%s\", %d, ...) -> %#lx", pv, dbfType, status); |
422 | |
423 | diff --git a/modules/database/src/ioc/db/db_field_log.h b/modules/database/src/ioc/db/db_field_log.h |
424 | index a801142..daebc84 100644 |
425 | --- a/modules/database/src/ioc/db/db_field_log.h |
426 | +++ b/modules/database/src/ioc/db/db_field_log.h |
427 | @@ -92,8 +92,8 @@ struct dbfl_val { |
428 | * db_delete_field_log(). Any code which changes a dbfl_type_ref |
429 | * field log to another type, or to reference different data, |
430 | * must explicitly call the dtor function. |
431 | - * If the dtor is NULL, then this means the array data is still owned |
432 | - * by a record. |
433 | + * If the dtor is NULL and no_elements > 0, then this means the array |
434 | + * data is still owned by a record. See the macro dbfl_has_copy below. |
435 | */ |
436 | struct dbfl_ref { |
437 | dbfl_freeFunc *dtor; /* Callback to free filter-allocated resources */ |
438 | @@ -121,6 +121,18 @@ typedef struct db_field_log { |
439 | } u; |
440 | } db_field_log; |
441 | |
442 | +/* |
443 | + * Whether a db_field_log* owns the field data. If this is the case, then the |
444 | + * db_field_log is fully decoupled from the record: there is no need to lock |
445 | + * the record when accessing the data, nor to call any rset methods (like |
446 | + * get_array_info) because this has already been done when we made the copy. A |
447 | + * special case here is that of no (remaining) data (i.e. no_elements==0). In |
448 | + * this case, making a copy is redundant, so we have no dtor. But conceptually |
449 | + * the db_field_log still owns the (empty) data. |
450 | + */ |
451 | +#define dbfl_has_copy(p)\ |
452 | + ((p) && ((p)->type==dbfl_type_val || (p)->u.r.dtor || (p)->no_elements==0)) |
453 | + |
454 | #ifdef __cplusplus |
455 | } |
456 | #endif |
457 | diff --git a/modules/database/src/ioc/dbStatic/link.h b/modules/database/src/ioc/dbStatic/link.h |
458 | index 2e3c778..a55900d 100644 |
459 | --- a/modules/database/src/ioc/dbStatic/link.h |
460 | +++ b/modules/database/src/ioc/dbStatic/link.h |
461 | @@ -77,15 +77,12 @@ struct macro_link { |
462 | }; |
463 | |
464 | struct dbCommon; |
465 | -typedef long (*LINKCVT)(); |
466 | |
467 | struct pv_link { |
468 | ELLNODE backlinknode; |
469 | char *pvname; /* pvname link points to */ |
470 | void *pvt; /* CA or DB private */ |
471 | - LINKCVT getCvt; /* input conversion function */ |
472 | short pvlMask; /* Options mask */ |
473 | - short lastGetdbrType; /* last dbrType for DB or CA get */ |
474 | }; |
475 | |
476 | struct jlink; |
477 | diff --git a/modules/database/src/std/filters/arr.c b/modules/database/src/std/filters/arr.c |
478 | index 1e506a2..dbdf49b 100644 |
479 | --- a/modules/database/src/std/filters/arr.c |
480 | +++ b/modules/database/src/std/filters/arr.c |
481 | @@ -99,57 +99,12 @@ static db_field_log* filter(void* pvt, dbChannel *chan, db_field_log *pfl) |
482 | long nTarget; |
483 | void *pTarget; |
484 | long offset = 0; |
485 | -<<<<<<< modules/database/src/std/filters/arr.c |
486 | - long nSource = dbChannelElements(chan); |
487 | - long capacity = nSource; |
488 | - void *pdst; |
489 | - |
490 | - switch (pfl->type) { |
491 | - case dbfl_type_val: |
492 | - /* Only filter arrays */ |
493 | - break; |
494 | - |
495 | - case dbfl_type_rec: |
496 | - /* Extract from record */ |
497 | - if (dbChannelSpecial(chan) == SPC_DBADDR && |
498 | - nSource > 1 && |
499 | - (prset = dbGetRset(&chan->addr)) && |
500 | - prset->get_array_info) |
501 | - { |
502 | - void *pfieldsave = dbChannelField(chan); |
503 | - prec = dbChannelRecord(chan); |
504 | - dbScanLock(prec); |
505 | - prset->get_array_info(&chan->addr, &nSource, &offset); |
506 | - nTarget = wrapArrayIndices(&start, my->incr, &end, nSource); |
507 | - pfl->type = dbfl_type_ref; |
508 | - pfl->stat = prec->stat; |
509 | - pfl->sevr = prec->sevr; |
510 | - pfl->time = prec->time; |
511 | - pfl->field_type = dbChannelFieldType(chan); |
512 | - pfl->field_size = dbChannelFieldSize(chan); |
513 | - pfl->no_elements = nTarget; |
514 | - if (nTarget) { |
515 | - pdst = freeListCalloc(my->arrayFreeList); |
516 | - if (pdst) { |
517 | - pfl->u.r.dtor = freeArray; |
518 | - pfl->u.r.pvt = my->arrayFreeList; |
519 | - offset = (offset + start) % dbChannelElements(chan); |
520 | - dbExtractArrayFromRec(&chan->addr, pdst, nTarget, capacity, |
521 | - offset, my->incr); |
522 | - pfl->u.r.field = pdst; |
523 | - } |
524 | - } |
525 | - dbScanUnlock(prec); |
526 | - dbChannelField(chan) = pfieldsave; |
527 | - } |
528 | -======= |
529 | long nSource = pfl->no_elements; |
530 | void *pSource = pfl->u.r.field; |
531 | |
532 | switch (pfl->type) { |
533 | case dbfl_type_val: |
534 | /* TODO Treat scalars as arrays with 1 element */ |
535 | ->>>>>>> modules/database/src/std/filters/arr.c |
536 | break; |
537 | |
538 | case dbfl_type_ref: |
I light of the bug I introduced with 3f13fff I will rebase this and the other branches I proposed for merging.