EPICS Controls Argonne National Laboratory

Experimental Physics and
Industrial Control System

2002  2003  2004  2005  2006  2007  2008  2009  2010  2011  2012  2013  2014  2015  2016  2017  2018  2019  <20202021  2022  2023  2024  Index 2002  2003  2004  2005  2006  2007  2008  2009  2010  2011  2012  2013  2014  2015  2016  2017  2018  2019  <20202021  2022  2023  2024 
<== Date ==> <== Thread ==>

Subject: [Merge] ~bfrk/epics-base:scalar-get-optimization into epics-base:7.0
From: Ben Franksen via Core-talk <core-talk at aps.anl.gov>
To: mp+382204 at code.launchpad.net
Date: Tue, 14 Apr 2020 10:44:22 -0000
Ben Franksen has proposed merging ~bfrk/epics-base:scalar-get-optimization into epics-base:7.0 with ~bfrk/epics-base:remove-dbfl_type_rec as a prerequisite.

Requested reviews:
  EPICS Core Developers (epics-core)

For more details, see:
https://code.launchpad.net/~bfrk/epics-base/+git/epics-base/+merge/382204

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.
-- 
Your team EPICS Core Developers is requested to review the proposed merge of ~bfrk/epics-base:scalar-get-optimization into epics-base:7.0.
diff --git a/modules/database/src/ioc/db/dbChannel.c b/modules/database/src/ioc/db/dbChannel.c
index 4c1215c..551cb39 100644
--- a/modules/database/src/ioc/db/dbChannel.c
+++ b/modules/database/src/ioc/db/dbChannel.c
@@ -31,6 +31,7 @@
 #include "dbBase.h"
 #include "dbChannel.h"
 #include "dbCommon.h"
+#include "dbConvertFast.h"
 #include "dbEvent.h"
 #include "dbLock.h"
 #include "dbStaticLib.h"
@@ -623,14 +624,61 @@ long dbChannelOpen(dbChannel *chan)
 }
 
 /* Only use dbChannelGet() if the record is already locked. */
-long dbChannelGet(dbChannel *chan, short type, void *pbuffer,
-        long *options, long *nRequest, void *pfl)
+long dbChannelGet(dbChannel *chan, short dbrType, void *pbuffer,
+        long *options, long *nRequest, db_field_log *pfl)
 {
-    return dbGet(&chan->addr, type, pbuffer, options, nRequest, pfl);
+    dbAddr addr = chan->addr; /* structure copy */
+    int pfl_has_copy = pfl && (pfl->type != dbfl_type_ref || pfl->u.r.dtor);
+
+    if (pfl) {
+        addr.field_size = pfl->field_size;
+        addr.field_type = pfl->field_type;
+        addr.no_elements = pfl->no_elements;
+        switch ((enum dbfl_type)pfl->type) {
+            case dbfl_type_val: addr.pfield = &pfl->u.v.field; break;
+            case dbfl_type_ref: addr.pfield = pfl->u.r.field; break;
+        }
+    }
+
+    /*
+     * Try to optimize scalar requests by caching (just) the conversion
+     * routine. This is possible only if we have no options, the field and the
+     * request have exactly one element, and we have no special processing to
+     * do. Note that if the db_field_log has already copied the data, dbGet
+     * does not call get_array_info.
+     */
+    if ((options && *options)
+        || addr.no_elements > 1
+        || (nRequest && *nRequest > 1)
+        || (!pfl_has_copy && addr.special == SPC_DBADDR)
+        || addr.special == SPC_ATTRIBUTE)
+    {
+        chan->getCvt = NULL;
+        return dbGet(&addr, dbrType, pbuffer, options, nRequest, pfl);
+    }
+
+    /*
+     * Optimization for scalar requests with no options,
+     * no special processing, and where the channel has only one element.
+     */
+    if (chan->getCvt && chan->lastGetdbrType == dbrType) {
+        return chan->getCvt(addr.pfield, pbuffer, &addr);
+    } else {
+        unsigned short dbfType = addr.field_type;
+
+        if (dbrType < 0 || dbrType > DBR_ENUM || dbfType > DBF_DEVICE)
+            return S_db_badDbrtype;
+
+        chan->getCvt = dbFastGetConvertRoutine[dbfType][dbrType];
+        chan->lastGetdbrType = dbrType;
+        if (nRequest)
+            *nRequest = addr.no_elements;
+        return chan->getCvt(addr.pfield, pbuffer, &addr);
+    }
 }
 
 long dbChannelGetField(dbChannel *chan, short dbrType, void *pbuffer,
-        long *options, long *nRequest, void *pfl)
+        long *options, long *nRequest, db_field_log *pfl)
 {
     dbCommon *precord = chan->addr.precord;
     long status = 0;
diff --git a/modules/database/src/ioc/db/dbChannel.h b/modules/database/src/ioc/db/dbChannel.h
index cb47b10..2dc929b 100644
--- a/modules/database/src/ioc/db/dbChannel.h
+++ b/modules/database/src/ioc/db/dbChannel.h
@@ -49,6 +49,8 @@ typedef struct evSubscrip {
 
 typedef struct chFilter chFilter;
 
+typedef long fastConvert(void *from, void *to, const dbAddr *paddr);
+
 /* A dbChannel points to a record field, and can have multiple filters */
 typedef struct dbChannel {
     const char *name;
@@ -59,6 +61,9 @@ typedef struct dbChannel {
     ELLLIST filters;          /* list of filters as created from JSON */
     ELLLIST pre_chain;        /* list of filters to be called pre-event-queue */
     ELLLIST post_chain;       /* list of filters to be called post-event-queue */
+    /* Support for optimizing scalar requests */
+    fastConvert *getCvt;      /* fast get conversion function */
+    short lastGetdbrType;     /* last dbChannelGet dbrType */
 } dbChannel;
 
 /* Prototype for the channel event function that is called in filter stacks
@@ -207,9 +212,9 @@ epicsShareExtern unsigned short dbDBRnewToDBRold[];
 
 
 epicsShareFunc long dbChannelGet(dbChannel *chan, short type,
-        void *pbuffer, long *options, long *nRequest, void *pfl);
+        void *pbuffer, long *options, long *nRequest, db_field_log *pfl);
 epicsShareFunc long dbChannelGetField(dbChannel *chan, short type,
-        void *pbuffer, long *options, long *nRequest, void *pfl);
+        void *pbuffer, long *options, long *nRequest, db_field_log *pfl);
 epicsShareFunc long dbChannelPut(dbChannel *chan, short type,
         const void *pbuffer, long nRequest);
 epicsShareFunc long dbChannelPutField(dbChannel *chan, short type,
diff --git a/modules/database/src/ioc/db/dbDbLink.c b/modules/database/src/ioc/db/dbDbLink.c
index 4e1dc69..611ee7f 100644
--- a/modules/database/src/ioc/db/dbDbLink.c
+++ b/modules/database/src/ioc/db/dbDbLink.c
@@ -57,7 +57,6 @@
 #include "dbBase.h"
 #include "dbBkpt.h"
 #include "dbCommonPvt.h"
-#include "dbConvertFast.h"
 #include "dbConvert.h"
 #include "db_field_log.h"
 #include "db_access_routines.h"
@@ -134,9 +133,7 @@ static void dbDbRemoveLink(struct dbLocker *locker, struct link *plink)
     /* locker is NULL when an isolated IOC is closing its links */
     if (locker) {
         plink->value.pv_link.pvt = 0;
-        plink->value.pv_link.getCvt = 0;
         plink->value.pv_link.pvlMask = 0;
-        plink->value.pv_link.lastGetdbrType = 0;
         ellDelete(&precord->bklnk, &plink->value.pv_link.backlinknode);
         dbLockSetSplit(locker, plink->precord, precord);
     }
@@ -166,10 +163,10 @@ static long dbDbGetValue(struct link *plink, short dbrType, void *pbuffer,
 {
     struct pv_link *ppv_link = &plink->value.pv_link;
     dbChannel *chan = linkChannel(plink);
-    DBADDR *paddr = &chan->addr;
     dbCommon *precord = plink->precord;
     db_field_log *pfl = NULL;
     long status;
+    long nRequest = 1; /* used if pnRequest == NULL */
 
     /* scan passive records if link is process passive  */
     if (ppv_link->pvlMask & pvlOptPP) {
@@ -178,59 +175,27 @@ static long dbDbGetValue(struct link *plink, short dbrType, void *pbuffer,
             return status;
     }
 
-    if (ppv_link->getCvt && ppv_link->lastGetdbrType == dbrType)
-    {
-        /* shortcut: scalar with known conversion, no filter */
-        status = ppv_link->getCvt(dbChannelField(chan), pbuffer, paddr);
-    }
-    else if (dbChannelFinalElements(chan) == 1 && (!pnRequest || *pnRequest == 1)
-                && dbChannelSpecial(chan) != SPC_DBADDR
-                && dbChannelSpecial(chan) != SPC_ATTRIBUTE
-                && ellCount(&chan->filters) == 0)
-    {
-        /* simple scalar: set up shortcut */
-        unsigned short dbfType = dbChannelFinalFieldType(chan);
-
-        if (dbrType < 0 || dbrType > DBR_ENUM || dbfType > DBF_DEVICE)
-            return S_db_badDbrtype;
-
-        ppv_link->getCvt = dbFastGetConvertRoutine[dbfType][dbrType];
-        ppv_link->lastGetdbrType = dbrType;
-        status = ppv_link->getCvt(dbChannelField(chan), pbuffer, paddr);
-    }
-    else
-    {
-        /* filter, array, or special */
-        ppv_link->getCvt = NULL;
-
-        /*
-         * For the moment, empty arrays are not supported by EPICS.
-         * See the remark in src/std/filters/arr.c for details.
-         */
-        if (dbChannelFinalElements(chan) <= 0) /* empty array request */
-            return S_db_badField;
-
-        if (ellCount(&chan->filters)) {
-            /* If filters are involved in a read, create field log and run filters */
-            pfl = db_create_read_log(chan);
-            if (!pfl)
-                return S_db_noMemory;
-
-            pfl = dbChannelRunPreChain(chan, pfl);
-            pfl = dbChannelRunPostChain(chan, pfl);
-        }
-
-        status = dbChannelGet(chan, dbrType, pbuffer, NULL, pnRequest, pfl);
-
-        if (pfl)
-            db_delete_field_log(pfl);
-
-        if (status)
-            return status;
-
-        if (pnRequest && *pnRequest <= 0) /* empty array result */
-            return S_db_badField;
+    /* If filters are involved in a read, create field log and run filters */
+    if (ellCount(&chan->filters)) {
+        pfl = db_create_read_log(chan);
+        if (!pfl)
+            return S_db_noMemory;
+        pfl = dbChannelRunPreChain(chan, pfl);
+        pfl = dbChannelRunPostChain(chan, pfl);
     }
+    if (!pnRequest)
+        pnRequest = &nRequest;
+    status = dbChannelGet(chan, dbrType, pbuffer, NULL, pnRequest, pfl);
+    if (pfl)
+        db_delete_field_log(pfl);
+    if (status)
+        return status;
+    /*
+     * For the moment, empty arrays are not supported by EPICS.
+     * See the remark in src/std/filters/arr.c for details.
+     */
+    if (*pnRequest <= 0) /* empty array result */
+        return S_db_badField;
 
     if (!status && precord != dbChannelRecord(chan))
         recGblInheritSevr(plink->value.pv_link.pvlMask & pvlOptMsMode,
diff --git a/modules/database/src/ioc/dbStatic/link.h b/modules/database/src/ioc/dbStatic/link.h
index 2e3c778..a55900d 100644
--- a/modules/database/src/ioc/dbStatic/link.h
+++ b/modules/database/src/ioc/dbStatic/link.h
@@ -77,15 +77,12 @@ struct macro_link {
 };
 
 struct dbCommon;
-typedef long (*LINKCVT)();
 
 struct pv_link {
     ELLNODE	backlinknode;
     char	*pvname;	/* pvname link points to */
     void	*pvt;		/* CA or DB private */
-    LINKCVT	getCvt;		/* input conversion function */
     short	pvlMask;	/* Options mask */
-    short	lastGetdbrType;	/* last dbrType for DB or CA get */
 };
 
 struct jlink;

Replies:
Re: [Merge] ~bfrk/epics-base:scalar-get-optimization into epics-base:7.0 Ben Franksen via Core-talk

Navigate by Date:
Prev: [Merge] ~bfrk/epics-base:scalar-get-optimization into epics-base:7.0 Ben Franksen via Core-talk
Next: Re: [Merge] ~bfrk/epics-base:scalar-get-optimization into epics-base:7.0 Ben Franksen via Core-talk
Index: 2002  2003  2004  2005  2006  2007  2008  2009  2010  2011  2012  2013  2014  2015  2016  2017  2018  2019  <20202021  2022  2023  2024 
Navigate by Thread:
Prev: [Merge] ~bfrk/epics-base:scalar-get-optimization into epics-base:7.0 Ben Franksen via Core-talk
Next: Re: [Merge] ~bfrk/epics-base:scalar-get-optimization into epics-base:7.0 Ben Franksen via Core-talk
Index: 2002  2003  2004  2005  2006  2007  2008  2009  2010  2011  2012  2013  2014  2015  2016  2017  2018  2019  <20202021  2022  2023  2024 
ANJ, 14 Apr 2020 Valid HTML 4.01! · Home · News · About · Base · Modules · Extensions · Distributions · Download ·
· Search · EPICS V4 · IRMIS · Talk · Bugs · Documents · Links · Licensing ·