DPL: Improving InputRecord iterator

matthiasrichter · matthiasrichter · commit 7ae04ca9dedd · 2020-01-27T13:45:19.000+01:00
- filtering out invalid input slots from the iteration
- nullptr as default values of DataRef members
- fixing const'ness of isValid method
diff --git a/Framework/Core/include/Framework/DataRef.h b/Framework/Core/include/Framework/DataRef.h
@@ -20,9 +20,9 @@ struct InputSpec;
 struct DataRef {
   // FIXME: had to remove the second 'const' in const T* const
   // to allow assignment
-  const InputSpec* spec;
-  const char* header;
-  const char* payload;
+  const InputSpec* spec = nullptr;
+  const char* header = nullptr;
+  const char* payload = nullptr;
 };
 
 } // namespace framework
diff --git a/Framework/Core/include/Framework/InputRecord.h b/Framework/Core/include/Framework/InputRecord.h
@@ -422,14 +422,14 @@ class InputRecord
   }
 
   /// Helper method to be used to check if a given part of the InputRecord is present.
-  bool isValid(std::string const& s)
+  bool isValid(std::string const& s) const
   {
     return isValid(s.c_str());
   }
 
   /// Helper method to be used to check if a given part of the InputRecord is present.
-  bool isValid(char const* s);
-  bool isValid(int pos);
+  bool isValid(char const* s) const;
+  bool isValid(int pos) const;
 
   size_t size() const
   {
@@ -456,7 +456,11 @@ class InputRecord
       : mParent(parent), mPosition(position), mSize(size > position ? size : position), mElement{nullptr, nullptr, nullptr}
     {
       if (mPosition < mSize) {
-        mElement = mParent->getByPos(mPosition);
+        if (mParent->isValid(mPosition)) {
+          mElement = mParent->getByPos(mPosition);
+        } else {
+          (*this)++;
+        }
       }
     }
 
@@ -465,8 +469,16 @@ class InputRecord
     // prefix increment
     SelfType& operator++()
     {
-      if (mPosition < mSize && ++mPosition < mSize) {
+      while (mPosition < mSize && ++mPosition < mSize) {
+        if (!mParent->isValid(mPosition)) {
+          continue;
+        }
         mElement = mParent->getByPos(mPosition);
+        break;
+      }
+      if (mPosition >= mSize) {
+        // reset the element to the default value of the type
+        mElement = ElementType{};
       }
       return *this;
     }
@@ -493,6 +505,33 @@ class InputRecord
       return mPosition != rh.mPosition;
     }
 
+    bool matches(o2::header::DataHeader matcher) const
+    {
+      if (mPosition >= mSize || mElement.header == nullptr) {
+        return false;
+      }
+      // at this point there must be a DataHeader, this has been checked by the DPL
+      // input cache
+      const auto* dh = DataRefUtils::getHeader<o2::header::DataHeader*>(mElement);
+      return *dh == matcher;
+    }
+
+    bool matches(o2::header::DataOrigin origin, o2::header::DataDescription description = o2::header::gDataDescriptionInvalid) const
+    {
+      if (mPosition >= mSize || mElement.header == nullptr) {
+        return false;
+      }
+      // at this point there must be a DataHeader, this has been checked by the DPL
+      // input cache
+      const auto* dh = DataRefUtils::getHeader<o2::header::DataHeader*>(mElement);
+      return dh->dataOrigin == origin && (description == o2::header::gDataDescriptionInvalid || dh->dataDescription == description);
+    }
+
+    bool matches(o2::header::DataOrigin origin, o2::header::DataDescription description, o2::header::DataHeader::SubSpecificationType subspec) const
+    {
+      return matches(o2::header::DataHeader{description, origin, subspec});
+    }
+
    private:
     size_t mPosition;
     size_t mSize;
diff --git a/Framework/Core/src/InputRecord.cxx b/Framework/Core/src/InputRecord.cxx
@@ -61,7 +61,7 @@ int InputRecord::getPos(std::string const& binding) const
   return -1;
 }
 
-bool InputRecord::isValid(char const* s)
+bool InputRecord::isValid(char const* s) const
 {
   DataRef ref = get(s);
   if (ref.header == nullptr || ref.payload == nullptr) {
@@ -70,7 +70,7 @@ bool InputRecord::isValid(char const* s)
   return true;
 }
 
-bool InputRecord::isValid(int s)
+bool InputRecord::isValid(int s) const
 {
   DataRef ref = getByPos(s);
   if (ref.header == nullptr || ref.payload == nullptr) {
diff --git a/Framework/Core/test/test_InputRecord.cxx b/Framework/Core/test/test_InputRecord.cxx
@@ -127,6 +127,23 @@ BOOST_AUTO_TEST_CASE(TestInputRecord)
   // A few more time just to make sure we are not stateful..
   BOOST_CHECK_EQUAL(record.get<int>("x"), 1);
   BOOST_CHECK_EQUAL(record.get<int>("x"), 1);
+
+  // test the iterator
+  int position = 0;
+  for (auto input = record.begin(), end = record.end(); input != end; input++, position++) {
+    if (position == 0) {
+      BOOST_CHECK(input.matches("TPC") == true);
+      BOOST_CHECK(input.matches("TPC", "CLUSTERS") == true);
+      BOOST_CHECK(input.matches("ITS", "CLUSTERS") == false);
+    }
+    if (position == 1) {
+      BOOST_CHECK(input.matches("ITS") == true);
+      BOOST_CHECK(input.matches("ITS", "CLUSTERS") == true);
+      BOOST_CHECK(input.matches("TPC", "CLUSTERS") == false);
+    }
+    // check if invalid slots are filtered out by the iterator
+    BOOST_CHECK(position != 2);
+  }
 }
 
 // TODO:

Original file line number	Diff line number	Diff line change
`@@ -61,7 +61,7 @@ int InputRecord::getPos(std::string const& binding) const`
`61`	`61`	`return -1;`
`62`	`62`	`}`
`63`	`63`
`64`		`-bool InputRecord::isValid(char const* s)`
	`64`	`+bool InputRecord::isValid(char const* s) const`
`65`	`65`	`{`
`66`	`66`	`DataRef ref = get(s);`
`67`	`67`	`if (ref.header == nullptr \|\| ref.payload == nullptr) {`
`@@ -70,7 +70,7 @@ bool InputRecord::isValid(char const* s)`
`70`	`70`	`return true;`
`71`	`71`	`}`
`72`	`72`
`73`		`-bool InputRecord::isValid(int s)`
	`73`	`+bool InputRecord::isValid(int s) const`
`74`	`74`	`{`
`75`	`75`	`DataRef ref = getByPos(s);`
`76`	`76`	`if (ref.header == nullptr \|\| ref.payload == nullptr) {`