Skip to content

Shallow Copy Fixes, master branch (2021.07.02.)

While trying to understand why the xAODChecker executable is reporting some false positives (ATEAM-764), I had to realise that a much deeper problem is at foot than I thought. It turns out that xAOD::ShallowAuxContainer was hiding a bug all these years. 😱

When reading shallow copies from a file, the implementation for the SG::IAuxStoreIO interface on xAOD::ShallowAuxContainer was flawed. When trying to ask for the "I/O pointer" of a variable that the object reported as having, the function would print an error for not finding the variable, and then return nullptr. This was because of a very naive, and erroneous handling of the xAOD::ShallowAuxContainer::m_parentIO variable.

The proper way of handling this would've been to make xAOD::ShallowAuxContainer::m_parentIO mutable, and implement a thread-safe way of initialising it to something based on xAOD::ShallowAuxContainer::m_parentLink lazily, if necessary. But I didn't feel like coming up with a complicated logic for these functions. Since this issue is anyway not something that we would ever hit during reconstruction. And even during analysis the two affected functions (xAOD::ShallowAuxContainer::getIOData() and xAOD::ShallowAuxContainer::getIOType()) see so little traffic, that a sub-optimal solution should not have any measurable effect on analysis code performance. (Apart from obviously no analysis code using these functions in practice.)

While doing all this, I also found that 2 transient variables of xAOD::ShallowAuxContainer were not marked as such. 😦 So our current validation AODs are a tiny bit bigger than they need to be. This should be fixed by this MR as well.

Finally, I removed some archaic code from xAODChecker that was only put in it to hide some (back then) known issues, a long-long time ago.

Pinging @ssnyder, @jcatmore, @emoyse, @elmsheus.

Edited by Attila Krasznahorkay

Merge request reports