WIP: xAOD::EventFormat Creation Upgrade, master branch (2020.06.16.)
After @keli's very useful reminder a few days ago, I now tried to look into how to create the xAOD::EventFormat
metadata for (D)xAOD files safely in multi-threaded jobs.
First off, while looking at the code and trying to figure out how to do all this, I had to realise that a unit test in xAODEventFormat has been laying dormant ever since our CMT->CMake transition. xAOD::EventFormat
would behave as expected. It may be worth mentioning that it was this unit test that triggered my inquiry on atlas-sw-core
about comparing lines with hexadecimal numbers in unit tests. (Ping @fwinkl.
As a reminder: Right now xAODMaker::EventFormatSvc
operates completely on its own, making use of IncidentSvc
to receive callbacks at the end of each event, and when the metadata would need to be written. Since IncidentSvc
is not an AthenaMT friendly construct, I was trying to implement the code without relying on it at all.
To do this, I now ended up with 3 components.
-
xAODMaker::EventFormatSvc
became a thread-safe service that doesn't do anything by itself, it acts sort of as a store for the metadata during the job. It's a fair question though whether it is really still needed. But more about it in a bit. -
xAODMaker::EventFormatStreamHelperTool
is a tool that is supposed to be added as a "helper tool" (https://gitlab.cern.ch/atlas/athena/-/blob/master/Control/AthenaServices/src/AthenaOutputStream.cxx#L184) to every output stream that is writing an xAOD format file. Its only purpose is to tellxAODMaker::EventFormatSvc
to collect all the metadata that it needs, from theStoreGateSvc
instance that is being "written from". -
xAODMaker::EventFormatMetaDataMakerTool
is the "metadata tool" in this whole setup. It is notified by MetaDataSvc at the end of the job when metadata is supposed to be written to the metadata store. It grabs the metadata collected byxAODMaker::EventFormatSvc
at that point, and records it into the metadata store.- Note that
xAODMaker::EventFormatSvc
returns a copy of thexAOD::EventFormat
object that it created, so that there would definitely be no clash between a call coming from anxAODMaker::EventFormatStreamHelperTool
instance, and one coming fromxAODMaker::EventFormatMetaDataMakerTool
. This is probably an unnecessary overhead, but a small one at that. (This is the reason why I added the additional tests in xAODEventFormat.)
- Note that
Now... Much of this is probably a bit of over-engineering. But since in the future output streams may actually be writing events "somehow" at the same time, it seemed logical to set up a single point (xAODMaker::EventFormatSvc
) that all the streams would be "talking to". (Through xAODMaker::EventFormatStreamHelperTool
instances.) It is possible that one could get rid of xAODMaker::EventFormatSvc
completely, and have the xAODMaker::EventFormatStreamHelperTool
instances all interact with MetaDataStore
directly. But I wasn't sure if that would be safe, or whether we would even want to make that thread-safe. (Multiple clients grabbing non-const pointers to an xAOD::EventFormat
object at the same time.)
I still want to extend this MR a bit with some more thorough testing, but at this point I wanted to get some feedback. @mnowak, @gemmeren, @tsulaia, @ssnyder, what do you guys think about such a setup?
Note that I had to realise while writing my tests, that SG::ReadMetaHandle is not functional yet. I was hoping to use it in my xAODMakerTest::EventFormatPrinterAlg
algorithm, but it didn't work. (And I didn't want to include the fix of that class in this MR.) The problem is that that handle at the moment always tries to retrieve a non-const pointer to an object held by MetaDataStore
. While in my case I would need to retrieve a const pointer to an object in InputMetaDataStore
.
So we'll need to fix up that code as well in the near future...