Feature request: more bits in mbbiDirect and mbboDirect records

[Dirk Zimoch]

mbbiDirect and mbboDirect records have 32 bit RVAL fields but only 16 bit VAL fields and the bit fields B0-BF.
It would be useful to have 32 bit VAL fields as well and additional bit fields B10-B1F.

[Ralph Lange (ralph-lange)]

That would seriously increase the memory footprint of existing applications.

How about adding mbbiD16/mbbiD32 and mbboD16/mbboD32 with explicit widths?
Do we need 64 bit types?
Arrays fields could allow variable width, but are harder to handle with respect to single-element reads and writes.

[Ralph Lange (ralph-lange)]

Would it be possible for two record types to use the same record support?

[Dirk Zimoch (dirk.zimoch)]

The problems starts when code needs to include xxxRecord.h and the two record types have different memory layout and the difference is not limited to appending something. When changing VAL to 32 bit this would be a different layout. When adding B* to the end, this would be fine but currently the existing B* are unfortunately not at the end.

Also device support that includes xxxRecord.h and accesses fields directly has the same problem. In this case in particular with the VAL field. In my opinion the whole idea of device supports should be reworked anyway (which means removed). Drivers should not care about record types and not access any fields directly. But that is a different can of worms.

Is memory footprint really a problem nowadays?

[Ralph Lange (ralph-lange)]

Your argument does mean that this should not be a change in the existing records (as that would break existing device supports), but new record types indeed.

Memory footprint is an issue for legacy systems, which might break on update.

Removing device support sounds good - call me when you're done.... ;-)

[Dirk Zimoch (dirk.zimoch)]

> Your argument does mean that this should not be a change in the existing records (as that would break existing device supports), but new record types indeed.

Not necessarily. Device support would have problems to support two different record types with the same code at the same time. But it should be fine with the suggested changes, provided the device support is re-compiled after the change.

Device support for mbbiDirect and mbboDirect typically neither accesses VAL nor B* directly. It only accesses RVAL which is already 32 bit.

Even if a device supports does access VAL and it changed from 16 to 32 bits, the upper 16 bits may be inaccessible to the driver and thus be ignored or 0 all the time. Do harm done.
The same if a device support does access B0-BF but ignores B10-B1F. It simply cannot benefit from the extension.

The only problem occurs if a device support passes the address of the VAL field to a pointer of epicsUint16 and the CPU is a big endian machine. In that case the 16 bits the device support deals with would end up in the upper bits. I will check if I find and device support that does so.

I attach a patch and do some tests.

[Dirk Zimoch (dirk.zimoch)]

Mbbi/oDirect 32 bit patch for 3.16.1

[mdavidsaver (mdavidsaver)]

> That would seriously increase the memory footprint of existing applications.

On linux-x86_64

sizeof(dbCommon) = 600
sizeof(mbboDirectRecord) = 984
sizeof(mbbiDirectRecord) = 896

With Dirk's patch

sizeof(dbCommon) = 600
sizeof(mbboDirectRecord) = 1008
sizeof(mbbiDirectRecord) = 920

This amounts to about a 2.5% increase.

[mdavidsaver (mdavidsaver)]

To me, the first question wrt. changing existing record types is whether this change would break existing device support code. As the new fields default to 0, effective behavior would not change. The main change I see as a concern is widening VAL. I think this would only cause a problem if it were being passed by reference/pointer, which would be unusual for a scalar value. So from an API compatibility prospective I think this change is acceptable.

For the broader question of utility, and eventual expansion to 64-bit, I'm inclined to look for other solutions. Perhaps a server-side filter to do bit-field extractions? I think this would handle all the cases where I've used mbbiDirect record type. Of course this wouldn't do anything for bit field construction.

If this isn't sufficient, then imo. if we're going to change existing mbbo/mbbiDirect, the just go directly to 64-bit now.

[Ralph Lange (ralph-lange)]

Ok, fine.

My gut feeling tells me that such bit width of raw values should be something configurable in the instance, so that we wouldn't even think of creating new record types.
That would require all sorts of bit width related strings, bits, severities, patterns etc. to be arrays, and a decent set of single-element operations on those, working across remote and local links.

[Andrew Johnson (anj)]

Changing VAL to be a (dbAccess) DBF_ULONG means that from a CA client's perspective the VAL field has gone from a (db_access) DBF_SHORT to DBF_DOUBLE (because ULONG gets promoted to DOUBLE when accessed by a CA client). I don't think that's a good idea unfortunately, I suspect it would break client programs.

One solution might be to make VAL be DBF_LONG, but that has the whole sign bit issue (unless we just limit it to 31 bits).

If we're going to produce a new record type another solution might be to use an array for the bit-map, which allows it to be variable length (but that might be harder to program device support for).

To Dirk's point of the API used by device support, there's nothing stopping a record type from publishing its own API that device support is supposed to call to access and modify the record's fields. Nobody has created such a beast yet (that I know of), but it might be a way to introduce new ideas. The record doew still have to use the DTYP & DSET fields (plus INP/OUT for the hardware addressing) for the record to talk to the device support, but beyond the initial set of DSET routines the interface between the two is controlled by the record type designer.

If someone wants to take that on, start by developing any new record type as a stand-alone module that we can review, test and discuss separately from Base. Integrating the results into Base later should be trivial. Please include writing self-tests as part of the development, testing the record-type operations and maybe even provide tests to check that device support is doing its job properly if that's appropriate. Provide soft device support for Soft Channel, Raw Soft Channel and Asynchronous Soft Channel (even Asynchronous Raw Soft Channel?).

[Dirk Zimoch (dirk.zimoch)]

I agree with the idea to change VAL to DBG_LONG, event though this is working around deficits in a completely different part of software (CA) and such work around tend to fire back much later.
Using arrays instead is an even uglier hack in my opinion and is prone to break many clients.

I did not intent to go directly to 64 bit in order not to break the device support API. I think my solution is most compatible with existing device supports. Except for the scenario that a device support passes a pointer to VAL. So far I found none. In particular asyn is safe.

So let's not take this too far. I think this change provides a reasonable benefit for a relatively small price.

Concerning memory footprint: We have had worse, for example when all DBF_FLOAT fields were changed to DBF_DOUBLE. (Which was a justified change in my opinion).

[Andrew Johnson (anj)]

I might be OK with just changing VAL and related fields of the mbb[io]Direct record types to DBF_LONG, but the current patch omits a number of essential changes to the soft device supports devMbbiDirectSoft devMbbiDirectSoftCallback devMbboDirectSoft and devMbboDirectSoftCallback which do things like calling dbGetLink() or dbPutLink() with a pointer to prev->val and a data type code. Gold star for the first person to find and point out the minor issue in the 3.16 version of one of those...

At minimum I think a new patch is in order with those additional changes, and with that additional work I would ask Dirk to publish his work as a branch that we can review for merging.

Contrary to the statement that "asyn is safe" I see that in Asyn4-30 the devAsynUInt32Digital.c writes directly to the VAL field in its initMbboDirect() routine after casting the value it got from to unsigned short, which would zero out the top 16 bits from the driver, and its processMbboDirect() routine also reads and writes the VAL field up to NUM_BITS=16 times in a loop (converting the B* fields into the VAL bit-pattern), so there are incompatibilities there (although not fatal).

Most of the other hardware device supports that I looked at only interacted with the RBV and RVAL fields so should be safe, but there may be some other device support that we've missed.

[Dirk Zimoch (dirk.zimoch)]

Oops, I missed initMbboDirect(). Bit it still will not crash, only ignore the upper bits.
I will start a new branch.