Project

General

Profile

Feature #5747

Add support to artdaq::Fragment for specifying sizes in bytes

Added by Kurt Biery almost 7 years ago. Updated over 6 years ago.

Status:
Closed
Priority:
Normal
Assignee:
Category:
-
Target version:
Start date:
03/24/2014
Due date:
04/18/2014
% Done:

100%

Estimated time:
50.00 h
Experiment:
-
Co-Assignees:
Duration: 26

Description

After some discussion, we've realized that it would be very helpful to provide methods in artdaq::Fragment that explicitly deal with bytes rather than RawDataType. This would allow users of this class to use the byte-centric interface to the class with less confusion, and would still allow the underlying implementation to use whatever data representation is the most efficient.

We also decided/realized that adding methods to the interface, rather than changing the interface, would be best in regards to backward compatibility.

This Issue is intended to track the work that is done to make these changes.

History

#1 Updated by Kurt Biery almost 7 years ago

  • Due date set to 04/18/2014
  • Status changed from New to Assigned
  • Assignee set to John Freeman
  • Target version set to v1_06_00
  • Estimated time set to 50.00 h

#2 Updated by Kurt Biery almost 7 years ago

Here are some notes on this subject from an email exchange between John and Chris Green. I'm including these for reference since it is often helpful to have notes on why various choices were made after the initial development work is done.

On 3/21/14, 4:10 PM, John Freeman wrote:

Hi Kurt,

I've attached a proposed new Fragment.hh interface at the following section of the artdaq Wiki :

https://cdcvs.fnal.gov/redmine/projects/artdaq/wiki/Proposed_Fragmenthh

As I mentioned yesterday, to come up with a byte-based analogue of a function, I just added "Bytes" to the end of it, so "foo()" becomes "fooBytes()". While I thought about things, a couple of issues came up; these are discussed in the source code comments (just search throughout the file for the token "JOHN" to find these discussions), but to summarize them here:

1) A drawback of a factory function is that you have to dynamically allocate the artdaq::Fragment object in the body of the function (meaning you lose the option of creating the object on the stack), or you can pass it a default-constructed artdaq::Fragment by reference along with the usual constructor arguments, but then it's not so much a constructor as it is an initialization function at this point. Is this something we care about?artdaq::Framg

Hi John,

Almost everything is fine and non-controversial, but I'm curious as to why the factory function isn't simply:

static Fragment FragmentBytes(size_t n) { return Fragment(ceil(n / float(sizeof(RawDataType)))); }

C++2011 best practices advise returning by value in cases like this: move semantics will make the appropriate efficiency savings when possible (e.g. for auto f(Fragment::FragmentBytes(fSize)). If someone actually wants something on the heap, then:

std::unique_ptr<Fragment> fptr(new Fragment(Fragment::FragmentBytes(fSize)));

should do the trick (again, move semantics will move the allocated vector under the covers without allocating a new one).

Does this seem reasonable?

2) It seems to make more sense to use a "char" instead of a "uint8_t" type to represent a byte. This is because (A) standard library memory management functions -- e.g., ifstream::read() -- use pointer-to-char, and (B) as discussed yesterday, "cout" converts uint8_t, so at least users who deal with a "char" will know to cast it as an integer rather than be unpleasantly surprised. Right now I have a typedef -- i.e., "typedef char byte_t". Should that typedef be there, or should we just explicitly use a "char"? The advantage is that "byte_t" makes it clear what we're doing, the disadvantage is that the user loses the guarantee that functions returning pointer-to-byte_t will return a char* (even if we never intend to change the typedef).

The preponderance of internet advice I was able to find advocates the use of the C99 type uint8_t. Using "char" to me implies you have real character data. I understand that in the past that was the only thing available, but with the "flavor-free" type definition, it seems to jar me. I don't feel strongly enough to argue the point beyond this, however. Perhaps a compromise:

typedef uint8_t byte_t;

?

Best,
Chris.

Cheers,

John

--
Chris Green <>, FNAL CD/ADSS/SSI; 'phone (630) 840-2167.
IRC: ;
chissgreen (AIM, Yahoo); (MSNM);
chris.h.green (Google Talk).

#3 Updated by Kurt Biery almost 7 years ago

Hi John,
When you get a chance, will you add some unit tests for the new Fragment methods that you added. These would go in artdaq/test/DAQdata/Fragment_t.cc, I believe.

Along with whatever you can dream up, the sorts of things that I think that we should test is that using the new methods results in the desired behavior with regards to fractional number of "words". E.g. specify a resize to 99 bytes and verifying that the actual resize was to 104 bytes.
Thanks,
Kurt

#4 Updated by John Freeman almost 7 years ago

  • Status changed from Assigned to Closed
  • % Done changed from 0 to 100

Added the byte-by-byte artdaq::Fragment interface functions ; additionally, added a set of unit tests for these functions at Kurt's suggestion in Fragment_t.cc

#5 Updated by Kurt Biery over 6 years ago

  • Target version changed from v1_06_00 to v1_07_00

Also available in: Atom PDF