From 10f2b102885e8974333cedafdd4df3facfbab037 Mon Sep 17 00:00:00 2001 From: Matthias Richter Date: Thu, 28 Jan 2016 11:35:10 +0100 Subject: [PATCH 01/13] adding a first draft of the in-memory primitive structures --- format/memory-format.h | 69 ++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 69 insertions(+) create mode 100644 format/memory-format.h diff --git a/format/memory-format.h b/format/memory-format.h new file mode 100644 index 0000000000000..410f118263074 --- /dev/null +++ b/format/memory-format.h @@ -0,0 +1,69 @@ +#ifndef MEMORY_FORMAT_H +#define MEMORY_FORMAT_H +//**************************************************************************** +//* This file is free software: you can redistribute it and/or modify * +//* it under the terms of the GNU General Public License as published by * +//* the Free Software Foundation, either version 3 of the License, or * +//* (at your option) any later version. * +//* * +//* Primary Authors: Matthias Richter * +//* * +//* The authors make no claims about the suitability of this software for * +//* any purpose. It is provided "as is" without express or implied warranty. * +//**************************************************************************** + +// @file memory_format.h +// @author Matthias Richter +// @since 2016-01-28 +// @brief Primitives for the ALICE O2 in-memory format + +// use the standard definitions of int variables +#include + +namespace AliceO2 { +namespace Format { + /** + * Basic struct for the data block meta data + * + * In total 24 byte are used per data block to descibe meta data + */ + struct BlockMetaData_t { + /** Size and version of the struct */ + int16_t mStructSize; + /** Subsystem or detector */ + char mDataOrigin[3+1]; // find if there is a consistent 3-letter naming scheme in ALICE, enforce 0 at the end + /** Main specification, e.g. raw, clusters, tracks */ + char mDataDescriptor[8]; + /** A system or detector specific sub specification */ + int32_t mSubSpec; + }; + + /** + * Block data header to be commonly used for all in-memory data blocks + * + * To be decided: use unions to fit different members for transfer and in-memory? + * + * The struct size could be handled by the transport layer in the multi-header + * approach, having the size as member keeps the possibility of sub headers + */ + struct BlockData_t + { + /** 4 bytes of a magic string */ + int32_t mMagicString; + /** size and version of the struct */ + int16_t mStructSize; + /** flags, one of the bits to indicate that a sub header is following */ + int16_t mFlags; + /** size of the data block in memory */ + int64_t mPayloadSize; // keep it for storage on disk + /** header serialization method for transfer */ + int64_t mHeaderSerializationMethod; // use string + /** payload serialization method for transfer */ + int64_t mPayloadSerializationMethod; // use string + /** data block meta data: type, origin, specification */ + BlockMetaData_t mMetaData; + }; + +}; // namespace Format +}; // namespace AliceO2 +#endif From 65f92566cc5053629763c70453dd9fda5083561d Mon Sep 17 00:00:00 2001 From: Matthias Richter Date: Mon, 8 Feb 2016 12:33:13 +0100 Subject: [PATCH 02/13] Updating the primitive data headers The update concludes the discussion in the CWG4 meeting --- format/header_versions.h | 5 ++ format/memory-format.h | 103 +++++++++++++++++++++++++++------------ 2 files changed, 77 insertions(+), 31 deletions(-) create mode 100644 format/header_versions.h diff --git a/format/header_versions.h b/format/header_versions.h new file mode 100644 index 0000000000000..2c902f01af21c --- /dev/null +++ b/format/header_versions.h @@ -0,0 +1,5 @@ +#ifndef HEADER_VERSIONS_H +#define HEADER_VERSIONS_H +#endif + +//TODO: to be filled diff --git a/format/memory-format.h b/format/memory-format.h index 410f118263074..8a54706ffe847 100644 --- a/format/memory-format.h +++ b/format/memory-format.h @@ -19,49 +19,90 @@ // use the standard definitions of int variables #include +#include "header_versions.h" +/** +The defined headers are a tradeoff to provide the necessary information in a lightweight way and to allow for evolution and compatibility. + +General header format +- Starts with basic header information, never serialized, with unique version number +- Strict policy enforced: no changes to members (e.g. width) or sequence of members +- New members can be appended +- All basic header structs are defined with fixed endianess and padding +- Header-stack concept: optional headers can follow the basic header +*/ namespace AliceO2 { namespace Format { /** - * Basic struct for the data block meta data + * Data header to be commonly used for all in-memory data blocks + * + * Unique header version; struct size included for consistency check + * and to facilitate later implementation of conversion handlers. + * + * A magic string makes identification of header simpler, e.g. after + * a data corruption; great help for low level debugging + * + * PayloadSize is a redundant information, to be used for integrity + * check and mandatory for disk dumped data * - * In total 24 byte are used per data block to descibe meta data + * Payload serialization method defined in the header, allows to build + * common functionality. Framework can choose the right tool for + * de-serialization */ - struct BlockMetaData_t { - /** Size and version of the struct */ - int16_t mStructSize; + struct DataHeader_t { + /** 4 bytes of a magic string */ + int32_t mMagicString; + /** size of the struct */ + int32_t mStructSize; + /** header version, bookkeeping in the software */ + int32_t mHeaderVersion; + /** Flags field, valid bits and their meaning defined by the header version */ + int32_t mFlags; + /** size of payload in memory */ + int64_t mPayloadSize; + /** payload serialization method for transfer */ + char mPayloadSerializationMethod[7+1]; + /** Payload meta data: Subsystem or detector */ + char mDataOrigin[3+1]; + /** Payload meta data: Data description, e.g. raw, clusters, tracks */ + char mDataDescriptor[15+1]; + /** Payload meta data: A system or detector specific sub specification */ + int64_t mSubSpec; + }; + + /** + * Helper struct for the payload meta data + * + * This struct is an addition to DataHeader_t to allow implementation + * of operators. All meta data members are directly included in DataHeader_t + * for easier access and consistency. + */ + struct PayloadMetaData_t { /** Subsystem or detector */ - char mDataOrigin[3+1]; // find if there is a consistent 3-letter naming scheme in ALICE, enforce 0 at the end - /** Main specification, e.g. raw, clusters, tracks */ - char mDataDescriptor[8]; + char mDataOrigin[3+1]; + /** Data description, e.g. raw, clusters, tracks */ + char mDataDescriptor[15+1]; /** A system or detector specific sub specification */ - int32_t mSubSpec; + int64_t mSubSpec; }; /** - * Block data header to be commonly used for all in-memory data blocks - * - * To be decided: use unions to fit different members for transfer and in-memory? - * - * The struct size could be handled by the transport layer in the multi-header - * approach, having the size as member keeps the possibility of sub headers + * Header-stack:: optional headers can follow the basic header + * A next header is indicated in the flag member of preceeding header + * Optional headers consist of a fixed NextHeaderDescription and a variable + * NextHeaderContent */ - struct BlockData_t - { - /** 4 bytes of a magic string */ - int32_t mMagicString; - /** size and version of the struct */ - int16_t mStructSize; - /** flags, one of the bits to indicate that a sub header is following */ - int16_t mFlags; - /** size of the data block in memory */ - int64_t mPayloadSize; // keep it for storage on disk - /** header serialization method for transfer */ - int64_t mHeaderSerializationMethod; // use string - /** payload serialization method for transfer */ - int64_t mPayloadSerializationMethod; // use string - /** data block meta data: type, origin, specification */ - BlockMetaData_t mMetaData; + struct NextHeaderDescription_t { + /** size of this next header description */ + int32_t mStructSize; + /** size of the next header payload */ + int32_t mNextHeaderContentSize; + /** Common flags for all next-headers, includes next-header flag */ + int32_t mFlags; + /** Descriptor */ + char mHeaderDescriptor[15+1]; + /** serialization method */ + char mSerializationMethod[7+1]; }; }; // namespace Format From 7301a7de0f675f56c6e22c6b89d7eb0cdc886e31 Mon Sep 17 00:00:00 2001 From: Matthias Richter Date: Mon, 8 Feb 2016 23:27:09 +0100 Subject: [PATCH 03/13] first draft of messageList class and iterator API --- format/message_list.h | 112 ++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 112 insertions(+) create mode 100644 format/message_list.h diff --git a/format/message_list.h b/format/message_list.h new file mode 100644 index 0000000000000..4157a88fcccaf --- /dev/null +++ b/format/message_list.h @@ -0,0 +1,112 @@ +//-*- Mode: C++ -*- + +#ifndef MESSAGELIST_H +#define MESSAGELIST_H +//**************************************************************************** +//* This file is free software: you can redistribute it and/or modify * +//* it under the terms of the GNU General Public License as published by * +//* the Free Software Foundation, either version 3 of the License, or * +//* (at your option) any later version. * +//* * +//* Primary Authors: Matthias Richter * +//* * +//* The authors make no claims about the suitability of this software for * +//* any purpose. It is provided "as is" without express or implied warranty. * +//**************************************************************************** + +// @file messageList.h +// @author Matthias Richter +// @since 2016-02-11 +// @brief Container class for messages in the O2 framework + +#include "memory-format.h" + +namespace AliceO2 { +namespace Format { + +// Ideally it does not matter for the implementation of the container class +// whether the type is the message container or the pointer to the payload +template +class messageList { + public: + messageList(); + messageList(const messageList& other); + messageList& operator=(const messageList& other); + ~messageList(); + + /** add data block to list */ + int add(T* headerMsg, T* payloadMsg); + /** number of data blocks in the list */ + size_t size() {return mDataArray.size();} + /** clear the list */ + void clear() {mDataArray.clear();} + /** check if list is empty */ + bool empty() {mDataArray.empty();} + + /** + * messagePair describes the two sequential messages for header and payload + * of a data block + * + * TODO: decide whether to use pointer to message or pointer to payload in the + * message. Whith the template approach and appropriate conversion operators in the + * message class possibly both ways can be served at the same time + */ + struct messagePair { + DataHeader_t header; + T* payload; + }; + typedef vector::iterator pairIt_t + + class iterator { + public: + typedef iterator self_type; + typedef T value_type; + + iterator(pairIt_t& dataIterator) : mDataIterator(dataIterator) { } + // prefix increment + self_type operator++() { mDataIterator++; return *this; } + // postfix increment + self_type operator++(int unused) {self_type copy(*this); ++*this; return copy;} + // TODO: given the fact that the data which is hold is a pointer, it always needs to be + // valid for dereference + T& operator*() { return *this.operator->();} + T* operator->() { return (*mDataIterator).payload;} + + bool operator==(const self_type& other) { return mDataIterator == other.mDataIterator; } + bool operator!=(const self_type& other) { return mDataIterator != other.mDataIterator; } + + bool operator==(const PayloadMetaData_t& md) { return (*mDataIterator).header == md } + bool operator!=(const PayloadMetaData_t& md) { return (*mDataIterator).header != md } + + /** conversion operator to PayloadMetaData_t struct */ + operator PayloadMetaData_t() {return (*mDataIterator).header;} + /** return size of payload */ + size_t size() {return (*mDataIterator).header.mPayloadSize;} + + private: + pairIt_t mDataIterator; + }; + + /** to be defined + class const_iterator + { + }; + */ + + private: + vector mDataArray; +}; + +template +int messageList::add(T& headerMsg, T& payloadMsg) +{ + // following conversion relies on the conversion operator for complex types + uint* headerData = static_cast(headerMsg); + + DataHeader_t* srcHeader = reinterpret_cast(headerData); + DataHeader_t tgtHeader(*srcHeader); +} + +}; // namespace Format +}; // namespace AliceO2 +#endif From 4cdf46e319fc9c50d9ccccf5e6e775a675253423 Mon Sep 17 00:00:00 2001 From: Matthias Richter Date: Wed, 30 Mar 2016 14:49:09 +0200 Subject: [PATCH 04/13] Extending message list class to allow two template parameters Introducing template parameters for the header message and the payload message. Correcting some typos. --- format/message_list.cxx | 10 +++++ format/message_list.h | 99 ++++++++++++++++++++++++++++------------- 2 files changed, 77 insertions(+), 32 deletions(-) create mode 100644 format/message_list.cxx diff --git a/format/message_list.cxx b/format/message_list.cxx new file mode 100644 index 0000000000000..ffbabc3f93fc6 --- /dev/null +++ b/format/message_list.cxx @@ -0,0 +1,10 @@ +// @file message_list.cxx +// @author Matthias Richter +// @since 2016-02-11 +// @brief Container class for messages in the O2 framework + +#include "message_list.h" +#include "memory-format.h" + +using namespace AliceO2; +using namespace Format; diff --git a/format/message_list.h b/format/message_list.h index 4157a88fcccaf..5dc4f1172fbba 100644 --- a/format/message_list.h +++ b/format/message_list.h @@ -14,11 +14,15 @@ //* any purpose. It is provided "as is" without express or implied warranty. * //**************************************************************************** -// @file messageList.h +// @file message_list.h // @author Matthias Richter // @since 2016-02-11 // @brief Container class for messages in the O2 framework +#include +#include +#include +#include // memset #include "memory-format.h" namespace AliceO2 { @@ -26,16 +30,26 @@ namespace Format { // Ideally it does not matter for the implementation of the container class // whether the type is the message container or the pointer to the payload -template +template class messageList { public: - messageList(); - messageList(const messageList& other); - messageList& operator=(const messageList& other); - ~messageList(); + typedef MsgT message_type; + typedef HdrT header_type; + + messageList() {} + messageList(const messageList& other); // not yet implemented + messageList& operator=(const messageList& other); // not yet implemented + ~messageList() {} /** add data block to list */ - int add(T* headerMsg, T* payloadMsg); + int add(MsgT& headerMsg, MsgT& payloadMsg) { + // conversion relies on the conversion operator for complex types + const uint8_t* headerData = headerMsg; + + const HdrT* srcHeader = reinterpret_cast(headerData); + // TODO: consistency check + mDataArray.push_back(messagePair(*srcHeader, payloadMsg)); + } /** number of data blocks in the list */ size_t size() {return mDataArray.size();} /** clear the list */ @@ -44,44 +58,65 @@ class messageList { bool empty() {mDataArray.empty();} /** - * messagePair describes the two sequential messages for header and payload - * of a data block + * messagePair describes the two sequential message parts for header and payload + * respectively. * * TODO: decide whether to use pointer to message or pointer to payload in the * message. Whith the template approach and appropriate conversion operators in the * message class possibly both ways can be served at the same time */ struct messagePair { - DataHeader_t header; - T* payload; + HdrT mHeader; + MsgT* mPayload; + + messagePair(MsgT& payload) : mHeader(), mPayload(&payload) { + memset(&mHeader, 0, sizeof(HdrT)); + } + + messagePair(const HdrT& header, MsgT& payload) : mHeader(), mPayload(&payload) { + memcpy(&mHeader, &header, sizeof(HdrT)); + } }; - typedef vector::iterator pairIt_t + typedef typename std::vector::iterator pairIt_t; + // TODO: operators inside a class can only have one parameter + // check whether to create a functor class + //bool operator==(const messageList::pairIt_t& first, const messageList::pairIt_t& second) { + // return (first->mHeader == second->mHeader) && (first->mPayload == second->mPayload); + //} + // + //bool operator!=(const messageList::pairIt_t& first, const messageList::pairIt_t& second) { + // return (first->mHeader != second->mHeader) || (first->mPayload != second->mPayload); + //} class iterator { public: typedef iterator self_type; - typedef T value_type; + typedef MsgT value_type; - iterator(pairIt_t& dataIterator) : mDataIterator(dataIterator) { } + iterator(const pairIt_t& dataIterator) : mDataIterator(dataIterator) { } // prefix increment - self_type operator++() { mDataIterator++; return *this; } + self_type operator++() { ++mDataIterator; return *this; } // postfix increment self_type operator++(int unused) {self_type copy(*this); ++*this; return copy;} // TODO: given the fact that the data which is hold is a pointer, it always needs to be // valid for dereference - T& operator*() { return *this.operator->();} - T* operator->() { return (*mDataIterator).payload;} + MsgT& operator*() { return *((*mDataIterator).mPayload);} + MsgT* operator->() { return (*mDataIterator).mPayload;} + + HdrT& getHdr() const {return (*mDataIterator).mHeader;} bool operator==(const self_type& other) { return mDataIterator == other.mDataIterator; } bool operator!=(const self_type& other) { return mDataIterator != other.mDataIterator; } - bool operator==(const PayloadMetaData_t& md) { return (*mDataIterator).header == md } - bool operator!=(const PayloadMetaData_t& md) { return (*mDataIterator).header != md } + /** TODO: comparison object to be used + bool operator==(const PayloadMetaData_t& md) { return (*mDataIterator).mHeader == md; } + bool operator!=(const PayloadMetaData_t& md) { return (*mDataIterator).mHeader != md; } + */ /** conversion operator to PayloadMetaData_t struct */ - operator PayloadMetaData_t() {return (*mDataIterator).header;} + operator HdrT() {return (*mDataIterator).mHeader;} /** return size of payload */ - size_t size() {return (*mDataIterator).header.mPayloadSize;} + size_t size() {return (*mDataIterator).mHeader.mPayloadSize;} private: pairIt_t mDataIterator; @@ -93,19 +128,19 @@ class messageList { }; */ - private: - vector mDataArray; -}; + iterator begin() { + pairIt_t it = mDataArray.begin(); + return iterator(it); + } -template -int messageList::add(T& headerMsg, T& payloadMsg) -{ - // following conversion relies on the conversion operator for complex types - uint* headerData = static_cast(headerMsg); + iterator end() { + pairIt_t it = mDataArray.end(); + return iterator(it); + } - DataHeader_t* srcHeader = reinterpret_cast(headerData); - DataHeader_t tgtHeader(*srcHeader); -} + private: + std::vector mDataArray; +}; }; // namespace Format }; // namespace AliceO2 From 41ea1d63c76f18fed68e486883a45824c9b19d4a Mon Sep 17 00:00:00 2001 From: Matthias Richter Date: Wed, 30 Mar 2016 14:53:24 +0200 Subject: [PATCH 05/13] Adding test executable for message_list template The test is to first extend a compilation check and can be later extended to a unit test --- format/testFormat.cxx | 105 ++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 105 insertions(+) create mode 100644 format/testFormat.cxx diff --git a/format/testFormat.cxx b/format/testFormat.cxx new file mode 100644 index 0000000000000..299add41f99b1 --- /dev/null +++ b/format/testFormat.cxx @@ -0,0 +1,105 @@ +#include "message_list.h" +#include "memory-format.h" + +#include // memset +#include + +using namespace AliceO2; +using namespace Format; + +typedef const uint8_t* SimpleMsg_t; + +struct SimpleHeader_t { + uint32_t id; + uint32_t specification; + + SimpleHeader_t() : id(0), specification(0) {} + SimpleHeader_t(uint32_t _id, uint32_t _spec) : id(_id), specification(_spec) {} +}; + +std::ostream& operator<<(std::ostream& stream, SimpleHeader_t header) { + stream << "Header ID: " << header.id << std::endl; + stream << "Header Specification: " << std::hex << header.specification; +} + +class TestMsg { +public: + TestMsg() : mBuffer(nullptr), mBufferSize(0) {} + TestMsg(const uint8_t* buffer, unsigned size) : mBuffer(new uint8_t[size]), mBufferSize(size) { + memcpy(mBuffer, buffer, size); + } + ~TestMsg() {clear();} + + int alloc(int size) { + } + + uint8_t* get() const { + return mBuffer; + } + + operator uint8_t*() { return get();} + + void clear() { + if (mBuffer) { + delete mBuffer; + } + mBuffer = NULL; + mBufferSize = 0; + } + +private: + uint8_t* mBuffer; + unsigned mBufferSize; +}; + +template +void print_list(ListType& list) { + for (typename ListType::iterator it = list.begin(); + it != list.end(); + ++it) { + // the iterator defines a conversion operator to the header type + std::cout << static_cast(it) << std::endl; + // dereferencing of the iterator gives the payload + std::cout << *it << std::endl; + } +} + +int main(int argc, char** argv) +{ + int iResult = 0; + + typedef messageList SimpleList_t; + SimpleList_t input; + input.size(); + + SimpleHeader_t hdr1(1, 0xdeadbeef); + const char* payload1="payload1"; + SimpleMsg_t headerMsg1 = reinterpret_cast(&hdr1); + SimpleMsg_t payloadMsg1 = reinterpret_cast(payload1); + input.add(headerMsg1, payloadMsg1); + + SimpleHeader_t hdr2(2, 0xf00); + const char* payload2="payload2"; + SimpleMsg_t headerMsg2 = reinterpret_cast(&hdr2); + SimpleMsg_t payloadMsg2 = reinterpret_cast(payload2); + input.add(headerMsg2, payloadMsg2); + + std::cout << "simple list:" << std::endl; + print_list(input); + std::cout << std::endl; + + typedef messageList TestMsgList_t; + TestMsgList_t msglist; + TestMsg testHeaderMsg1((uint8_t*)&hdr1, sizeof(hdr1)); + TestMsg testPayloadMsg1((uint8_t*)payload1, strlen(payload1)+1); + msglist.add(testHeaderMsg1, testPayloadMsg1); + TestMsg testHeaderMsg2((uint8_t*)&hdr2, sizeof(hdr2)); + TestMsg testPayloadMsg2((uint8_t*)payload2, strlen(payload2)+1); + msglist.add(testHeaderMsg2, testPayloadMsg2); + + std::cout << "message class list:" << std::endl; + print_list(msglist); + std::cout << std::endl; + + return 0; +} From df1a68d2519582f7a219e56d0fa05e974db7b3db Mon Sep 17 00:00:00 2001 From: Matthias Richter Date: Wed, 30 Mar 2016 14:54:58 +0200 Subject: [PATCH 06/13] adding format test to build --- CMakeLists.txt | 1 + format/CMakeLists.txt | 63 +++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 64 insertions(+) create mode 100644 format/CMakeLists.txt diff --git a/CMakeLists.txt b/CMakeLists.txt index c0f4191ef9397..73686d21727f4 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -224,6 +224,7 @@ add_subdirectory (macro) add_subdirectory (o2cdb) add_subdirectory (test) add_subdirectory (o2qa) +add_subdirectory (format) Option(BUILD_DOXYGEN "Build Doxygen" OFF) diff --git a/format/CMakeLists.txt b/format/CMakeLists.txt new file mode 100644 index 0000000000000..fcd82768b0289 --- /dev/null +++ b/format/CMakeLists.txt @@ -0,0 +1,63 @@ +# @author Matthias Richter +# @brief cmake setup for the test of format implementation + +set(INCLUDE_DIRECTORIES + ${CMAKE_SOURCE_DIR}/format +) + +set(SYSTEM_INCLUDE_DIRECTORIES + ${BASE_INCLUDE_DIRECTORIES} + ${Boost_INCLUDE_DIR} + ${FAIRROOT_INCLUDE_DIR} +) + +include_directories(${INCLUDE_DIRECTORIES}) +include_directories(SYSTEM ${SYSTEM_INCLUDE_DIRECTORIES}) + +set(LINK_DIRECTORIES + ${FAIRROOT_LIBRARY_DIR} + ${Boost_LIBRARY_DIRS} +) + +link_directories(${LINK_DIRECTORIES}) + +#library source +set(SRCS +) + +set(DEPENDENCIES + ${DEPENDENCIES} + ) + +set(DEPENDENCIES + ${DEPENDENCIES} + ${CMAKE_THREAD_LIBS_INIT} +) + +set(SRCS + message_list.cxx +) + +set(LIBRARY_NAME aliceo2format) + +GENERATE_LIBRARY() + +Set(Exe_Names + testFormat +) + +set(Exe_Source + testFormat.cxx +) + +list(LENGTH Exe_Names _length) +math(EXPR _length ${_length}-1) + +ForEach(_file RANGE 0 ${_length}) + list(GET Exe_Names ${_file} _name) + list(GET Exe_Source ${_file} _src) + set(EXE_NAME ${_name}) + set(SRCS ${_src}) + set(DEPENDENCIES ALICEHLT dl) + GENERATE_EXECUTABLE() +EndForEach(_file RANGE 0 ${_length}) From 54d57492f50f99ef39e3f7b62ee4e000d184b7de Mon Sep 17 00:00:00 2001 From: Matthias Richter Date: Wed, 6 Apr 2016 13:47:49 +0200 Subject: [PATCH 07/13] Introducing conditional search in the list iterator --- format/message_list.h | 66 +++++++++++++++++++++++++++++++++---------- 1 file changed, 51 insertions(+), 15 deletions(-) diff --git a/format/message_list.h b/format/message_list.h index 5dc4f1172fbba..cb5c323c3ee93 100644 --- a/format/message_list.h +++ b/format/message_list.h @@ -23,7 +23,7 @@ #include #include #include // memset -#include "memory-format.h" +#include // std::function namespace AliceO2 { namespace Format { @@ -35,13 +35,20 @@ class messageList { public: typedef MsgT message_type; typedef HdrT header_type; + /// comparison metric for selection of elements + /// an external function can be defined by the caller of begin() to + /// apply a selection of elements + typedef std::function HdrComparison; messageList() {} messageList(const messageList& other); // not yet implemented messageList& operator=(const messageList& other); // not yet implemented ~messageList() {} - /** add data block to list */ + /// add data block to list + /// both header and payload message parts are required to add an entry + /// the actual header of type HdrT is extracted from the header + /// message part. int add(MsgT& headerMsg, MsgT& payloadMsg) { // conversion relies on the conversion operator for complex types const uint8_t* headerData = headerMsg; @@ -88,14 +95,40 @@ class messageList { // return (first->mHeader != second->mHeader) || (first->mPayload != second->mPayload); //} + /** + * @class iterator + * Implementation of navigation through the list and access to elements. + * + * An optional comparison metric @ref HdrComparison can be used to provide + * a selection of elements. + */ class iterator { public: typedef iterator self_type; typedef MsgT value_type; - - iterator(const pairIt_t& dataIterator) : mDataIterator(dataIterator) { } + + iterator(const pairIt_t& dataIterator, const pairIt_t& iteratorRange, + const HdrComparison hdrsel = HdrComparison()) + : mDataIterator(dataIterator) + , mEnd(iteratorRange) + , mHdrSelection(hdrsel) + { } + iterator(const pairIt_t& dataIterator) + : mDataIterator(dataIterator) + , mEnd(dataIterator) + , mHdrSelection(HdrComparison()) + { } // prefix increment - self_type operator++() { ++mDataIterator; return *this; } + self_type& operator++() { + while (++mDataIterator != mEnd) { + // operator bool() of std::function is used to determine whether + // a selector is set or not, the default is not callable. + // if the std::function container has an assigned target, this is + // called with the header as parameter + if (!mHdrSelection || mHdrSelection(mDataIterator->mHeader)) break; + } + return *this; + } // postfix increment self_type operator++(int unused) {self_type copy(*this); ++*this; return copy;} // TODO: given the fact that the data which is hold is a pointer, it always needs to be @@ -103,16 +136,12 @@ class messageList { MsgT& operator*() { return *((*mDataIterator).mPayload);} MsgT* operator->() { return (*mDataIterator).mPayload;} + /** return header at iterator position */ HdrT& getHdr() const {return (*mDataIterator).mHeader;} bool operator==(const self_type& other) { return mDataIterator == other.mDataIterator; } bool operator!=(const self_type& other) { return mDataIterator != other.mDataIterator; } - /** TODO: comparison object to be used - bool operator==(const PayloadMetaData_t& md) { return (*mDataIterator).mHeader == md; } - bool operator!=(const PayloadMetaData_t& md) { return (*mDataIterator).mHeader != md; } - */ - /** conversion operator to PayloadMetaData_t struct */ operator HdrT() {return (*mDataIterator).mHeader;} /** return size of payload */ @@ -120,6 +149,8 @@ class messageList { private: pairIt_t mDataIterator; + pairIt_t mEnd; + HdrComparison mHdrSelection; }; /** to be defined @@ -128,14 +159,19 @@ class messageList { }; */ - iterator begin() { - pairIt_t it = mDataArray.begin(); - return iterator(it); + iterator begin(const HdrComparison hdrsel = HdrComparison()) { + iterator ret(mDataArray.begin(), mDataArray.end(), hdrsel); + // if the std::function container has an assigned target, this is + // is used used to check whether the iterator matches the selection + // further checks are in the increment operator. + // the iterator class implements a type cast operator which allows + // to use it directly in the HdrComparison + if (hdrsel && !hdrsel(ret)) ++ret; + return ret; } iterator end() { - pairIt_t it = mDataArray.end(); - return iterator(it); + return iterator(mDataArray.end()); } private: From 42e76a3f48f261ffbee7269bdc4ad7f0e8e8f649 Mon Sep 17 00:00:00 2001 From: Matthias Richter Date: Wed, 6 Apr 2016 17:13:48 +0200 Subject: [PATCH 08/13] Using a mockup of the list element selector --- format/testFormat.cxx | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/format/testFormat.cxx b/format/testFormat.cxx index 299add41f99b1..c13c1e83450f2 100644 --- a/format/testFormat.cxx +++ b/format/testFormat.cxx @@ -53,8 +53,8 @@ class TestMsg { }; template -void print_list(ListType& list) { - for (typename ListType::iterator it = list.begin(); +void print_list(ListType& list, typename ListType::HdrComparison hdrsel = typename ListType::HdrComparison()) { + for (typename ListType::iterator it = list.begin(hdrsel); it != list.end(); ++it) { // the iterator defines a conversion operator to the header type @@ -98,7 +98,7 @@ int main(int argc, char** argv) msglist.add(testHeaderMsg2, testPayloadMsg2); std::cout << "message class list:" << std::endl; - print_list(msglist); + print_list(msglist, [](const TestMsgList_t::header_type& hdr){return hdr.specification==0xf00;} ); std::cout << std::endl; return 0; From e03f1f7864c333843f5b9c4a98aac8618ee0c28e Mon Sep 17 00:00:00 2001 From: Matthias Richter Date: Wed, 13 Apr 2016 11:56:46 +0200 Subject: [PATCH 09/13] Adjusting new development to the proposed directory structure Creating new module 'DataFormats' on top level. The recent development regarding data formats goes into DataFormats/Generic. --- CMakeLists.txt | 2 +- DataFormats/CMakeLists.txt | 3 +++ DataFormats/Generic/CMakeLists.txt | 3 +++ {format => DataFormats/Generic}/message_list.cxx | 0 {format => DataFormats/Generic}/message_list.h | 0 {format => DataFormats/Generic/test}/CMakeLists.txt | 10 +--------- {format => DataFormats/Generic/test}/header_versions.h | 0 {format => DataFormats/Generic/test}/memory-format.h | 0 {format => DataFormats/Generic/test}/testFormat.cxx | 0 9 files changed, 8 insertions(+), 10 deletions(-) create mode 100644 DataFormats/CMakeLists.txt create mode 100644 DataFormats/Generic/CMakeLists.txt rename {format => DataFormats/Generic}/message_list.cxx (100%) rename {format => DataFormats/Generic}/message_list.h (100%) rename {format => DataFormats/Generic/test}/CMakeLists.txt (82%) rename {format => DataFormats/Generic/test}/header_versions.h (100%) rename {format => DataFormats/Generic/test}/memory-format.h (100%) rename {format => DataFormats/Generic/test}/testFormat.cxx (100%) diff --git a/CMakeLists.txt b/CMakeLists.txt index 73686d21727f4..01ae1f7216365 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -224,7 +224,7 @@ add_subdirectory (macro) add_subdirectory (o2cdb) add_subdirectory (test) add_subdirectory (o2qa) -add_subdirectory (format) +add_subdirectory (DataFormats) Option(BUILD_DOXYGEN "Build Doxygen" OFF) diff --git a/DataFormats/CMakeLists.txt b/DataFormats/CMakeLists.txt new file mode 100644 index 0000000000000..ea3c28753faeb --- /dev/null +++ b/DataFormats/CMakeLists.txt @@ -0,0 +1,3 @@ +# @brief cmake setup for the DataFormats module of AliceO2 + +add_subdirectory (Generic) diff --git a/DataFormats/Generic/CMakeLists.txt b/DataFormats/Generic/CMakeLists.txt new file mode 100644 index 0000000000000..326b52234a511 --- /dev/null +++ b/DataFormats/Generic/CMakeLists.txt @@ -0,0 +1,3 @@ +# @brief cmake setup for DataFormats/Generic of AliceO2 + +add_subdirectory (test) diff --git a/format/message_list.cxx b/DataFormats/Generic/message_list.cxx similarity index 100% rename from format/message_list.cxx rename to DataFormats/Generic/message_list.cxx diff --git a/format/message_list.h b/DataFormats/Generic/message_list.h similarity index 100% rename from format/message_list.h rename to DataFormats/Generic/message_list.h diff --git a/format/CMakeLists.txt b/DataFormats/Generic/test/CMakeLists.txt similarity index 82% rename from format/CMakeLists.txt rename to DataFormats/Generic/test/CMakeLists.txt index fcd82768b0289..5f79e337bd77a 100644 --- a/format/CMakeLists.txt +++ b/DataFormats/Generic/test/CMakeLists.txt @@ -2,20 +2,17 @@ # @brief cmake setup for the test of format implementation set(INCLUDE_DIRECTORIES - ${CMAKE_SOURCE_DIR}/format + ${CMAKE_CURRENT_SOURCE_DIR}/.. ) set(SYSTEM_INCLUDE_DIRECTORIES - ${BASE_INCLUDE_DIRECTORIES} ${Boost_INCLUDE_DIR} - ${FAIRROOT_INCLUDE_DIR} ) include_directories(${INCLUDE_DIRECTORIES}) include_directories(SYSTEM ${SYSTEM_INCLUDE_DIRECTORIES}) set(LINK_DIRECTORIES - ${FAIRROOT_LIBRARY_DIR} ${Boost_LIBRARY_DIRS} ) @@ -35,13 +32,8 @@ set(DEPENDENCIES ) set(SRCS - message_list.cxx ) -set(LIBRARY_NAME aliceo2format) - -GENERATE_LIBRARY() - Set(Exe_Names testFormat ) diff --git a/format/header_versions.h b/DataFormats/Generic/test/header_versions.h similarity index 100% rename from format/header_versions.h rename to DataFormats/Generic/test/header_versions.h diff --git a/format/memory-format.h b/DataFormats/Generic/test/memory-format.h similarity index 100% rename from format/memory-format.h rename to DataFormats/Generic/test/memory-format.h diff --git a/format/testFormat.cxx b/DataFormats/Generic/test/testFormat.cxx similarity index 100% rename from format/testFormat.cxx rename to DataFormats/Generic/test/testFormat.cxx From 64e43d1942d019b8b593cb723acca4cf81836439 Mon Sep 17 00:00:00 2001 From: Matthias Richter Date: Wed, 13 Apr 2016 15:05:44 +0200 Subject: [PATCH 10/13] Rename unit test exe and add as test to cmake configuration --- DataFormats/Generic/test/CMakeLists.txt | 12 ++++++++---- .../test/{testFormat.cxx => testMessageList.cxx} | 0 2 files changed, 8 insertions(+), 4 deletions(-) rename DataFormats/Generic/test/{testFormat.cxx => testMessageList.cxx} (100%) diff --git a/DataFormats/Generic/test/CMakeLists.txt b/DataFormats/Generic/test/CMakeLists.txt index 5f79e337bd77a..2460a5565406e 100644 --- a/DataFormats/Generic/test/CMakeLists.txt +++ b/DataFormats/Generic/test/CMakeLists.txt @@ -1,5 +1,5 @@ # @author Matthias Richter -# @brief cmake setup for the test of format implementation +# @brief cmake setup for the test of generic format implementation set(INCLUDE_DIRECTORIES ${CMAKE_CURRENT_SOURCE_DIR}/.. @@ -35,16 +35,15 @@ set(SRCS ) Set(Exe_Names - testFormat ) set(Exe_Source - testFormat.cxx ) list(LENGTH Exe_Names _length) -math(EXPR _length ${_length}-1) +if(${_length}) +math(EXPR _length ${_length}-1) ForEach(_file RANGE 0 ${_length}) list(GET Exe_Names ${_file} _name) list(GET Exe_Source ${_file} _src) @@ -53,3 +52,8 @@ ForEach(_file RANGE 0 ${_length}) set(DEPENDENCIES ALICEHLT dl) GENERATE_EXECUTABLE() EndForEach(_file RANGE 0 ${_length}) +endif(${_length}) + +# avoid installation of test exe by not using macro GENERATE_EXECUTABLE +add_executable(testMessageList testMessageList.cxx) +add_test(testMessageList ${CMAKE_BINARY_DIR}/bin/testMessageList) diff --git a/DataFormats/Generic/test/testFormat.cxx b/DataFormats/Generic/test/testMessageList.cxx similarity index 100% rename from DataFormats/Generic/test/testFormat.cxx rename to DataFormats/Generic/test/testMessageList.cxx From 59891b26cde40f356f11f4225c74c26e143c70ff Mon Sep 17 00:00:00 2001 From: Matthias Richter Date: Wed, 13 Apr 2016 16:27:41 +0200 Subject: [PATCH 11/13] adding documentation --- DataFormats/Generic/test/testMessageList.cxx | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/DataFormats/Generic/test/testMessageList.cxx b/DataFormats/Generic/test/testMessageList.cxx index c13c1e83450f2..d0c4f510ad90e 100644 --- a/DataFormats/Generic/test/testMessageList.cxx +++ b/DataFormats/Generic/test/testMessageList.cxx @@ -1,3 +1,11 @@ +/// @file testMessageList.cxx +/// @brief Unit test for message_list.h template class + +// TODO: this is for the moment only testing compilation +// ideally, this test program is only compiled when the test +// is going to be executed. By using add_executable in the cmake +// configuartion, the program is always build (though not installed) + #include "message_list.h" #include "memory-format.h" @@ -7,8 +15,10 @@ using namespace AliceO2; using namespace Format; +// a simple message type, just a pointer to some payload typedef const uint8_t* SimpleMsg_t; +// a simple header definition struct SimpleHeader_t { uint32_t id; uint32_t specification; @@ -17,11 +27,15 @@ struct SimpleHeader_t { SimpleHeader_t(uint32_t _id, uint32_t _spec) : id(_id), specification(_spec) {} }; +// print operator for the simple header std::ostream& operator<<(std::ostream& stream, SimpleHeader_t header) { stream << "Header ID: " << header.id << std::endl; stream << "Header Specification: " << std::hex << header.specification; } +// more complex message type, some class which wraps around payload +// implements type conversion operator to return pointer to payload +// buffer class TestMsg { public: TestMsg() : mBuffer(nullptr), mBufferSize(0) {} @@ -52,6 +66,8 @@ class TestMsg { unsigned mBufferSize; }; +// helper function to print entries of the message list by using +// iterator template void print_list(ListType& list, typename ListType::HdrComparison hdrsel = typename ListType::HdrComparison()) { for (typename ListType::iterator it = list.begin(hdrsel); From 11423a912ed5933bc7df7a5f78013b5198880cdb Mon Sep 17 00:00:00 2001 From: Matthias Richter Date: Thu, 14 Apr 2016 10:24:37 +0200 Subject: [PATCH 12/13] Adding notice about usage of the data format header in the unit test The original header definition has served as basis for the discussion, the final definition has been developed elsewhere and is soon to be merged. The very early definition is temporarily kept for the unit test. --- DataFormats/Generic/test/memory-format.h | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/DataFormats/Generic/test/memory-format.h b/DataFormats/Generic/test/memory-format.h index 8a54706ffe847..d216cf5657e43 100644 --- a/DataFormats/Generic/test/memory-format.h +++ b/DataFormats/Generic/test/memory-format.h @@ -12,10 +12,14 @@ //* any purpose. It is provided "as is" without express or implied warranty. * //**************************************************************************** -// @file memory_format.h -// @author Matthias Richter -// @since 2016-01-28 -// @brief Primitives for the ALICE O2 in-memory format +/// @file memory_format.h +/// @author Matthias Richter +/// @since 2016-01-28 +/// @brief Helper structs for the ALICE O2 generic format API test +/// @note DO NOT USE OUTSIDE UNIT TEST OF FORMAT API +/// This definitions have been a first draft during discussion of +/// in memory data formats, the final header file has been placed +/// elsewhere, but this file is temporarily kept for the unit test // use the standard definitions of int variables #include From 2cf7d88a6bff8bd05a63cc6e7b5776c2e51cd7a2 Mon Sep 17 00:00:00 2001 From: Matthias Richter Date: Tue, 19 Apr 2016 13:50:48 +0200 Subject: [PATCH 13/13] Bugfix: missing return statement in custom ostream operator<< added When using clang, writing to output stream caused segmentation violation because the stream object was not returned by the operator which has been used just before. Adding also some other return statements (though irrelevant for the crash). Crash log * thread #1: tid = 0x597d9, 0x0000000100001f7f testMessageList`void print_list >(list=, hdrsel=) + 191 at testMessageList.cxx:77, queue = 'com.apple.main-thread', stop reason = EXC_BAD_INSTRUCTION (code=EXC_I386_INVOP, subcode=0x0) frame #0: 0x0000000100001f7f testMessageList`void print_list >(list=, hdrsel=) + 191 at testMessageList.cxx:77 74 it != list.end(); 75 ++it) { 76 // the iterator defines a conversion operator to the header type -> 77 std::cout << static_cast(it) << std::end; 78 // dereferencing of the iterator gives the payload 79 std::cout << *it << std::end; 80 } --- DataFormats/Generic/message_list.h | 2 ++ DataFormats/Generic/test/testMessageList.cxx | 4 ++++ 2 files changed, 6 insertions(+) diff --git a/DataFormats/Generic/message_list.h b/DataFormats/Generic/message_list.h index cb5c323c3ee93..068cffa1bee47 100644 --- a/DataFormats/Generic/message_list.h +++ b/DataFormats/Generic/message_list.h @@ -56,6 +56,8 @@ class messageList { const HdrT* srcHeader = reinterpret_cast(headerData); // TODO: consistency check mDataArray.push_back(messagePair(*srcHeader, payloadMsg)); + + return mDataArray.size(); } /** number of data blocks in the list */ size_t size() {return mDataArray.size();} diff --git a/DataFormats/Generic/test/testMessageList.cxx b/DataFormats/Generic/test/testMessageList.cxx index d0c4f510ad90e..f7726975c8ac9 100644 --- a/DataFormats/Generic/test/testMessageList.cxx +++ b/DataFormats/Generic/test/testMessageList.cxx @@ -31,6 +31,8 @@ struct SimpleHeader_t { std::ostream& operator<<(std::ostream& stream, SimpleHeader_t header) { stream << "Header ID: " << header.id << std::endl; stream << "Header Specification: " << std::hex << header.specification; + + return stream; } // more complex message type, some class which wraps around payload @@ -45,6 +47,8 @@ class TestMsg { ~TestMsg() {clear();} int alloc(int size) { + // not yet implemented + return 0; } uint8_t* get() const {