Tracking Surface for Athena MT. ATLASRECTS-4623
The main purpose is to eventually ATLAS_CHECK_THREAD_SAFETY
in Tracking/TrkDetDescr/TrkSurfaces
I will use as an example the Surface.h
class .
Lazy Init
- Move to cachedUniquePtr for the things that are
lazily initialized
. Examples :
CxxUtils::CachedUniquePtrT<Amg::Transform3D> m_transform; //!< Transform3D to orient surface w.r.t to global frame
CxxUtils::CachedUniquePtrT<Amg::Vector3D> m_center; //!< center position of the surface
CxxUtils::CachedUniquePtrT<Amg::Vector3D> m_normal; //!< normal vector of the surface
A method that instantiates a member at 1st use and then returns the existing instance (does not modify afterwards) and is logically const changes from:
inline const Amg::Vector3D& Surface::center() const
{
if (m_transform && !m_center) m_center = new Amg::Vector3D(m_transform->translation());
if (m_center) return (*m_center);
.....
to :
if(m_transform && !m_center){
return *(m_center.set(
std::make_unique<Amg::Vector3D>(m_transform->translation())
));
}
if(m_center) return (*m_center);
......
const setters
- This class (and her descendants) have certain
const setter
, things like
void setOwner(SurfaceOwner x) const
allowing modification of const object.
-
Unfortunatelly this feature is used/propagated in many places so will take a bit of time to migrate all depended code
-
In this case (although we could silence the warnings with atomic etc) unlike the above a setter can not be really be logically const!
-
So for now I try to provide this kind of way for client migration to const correctness:
/** Set ownership
* Athena MT note : This should not be really used
* as it modifies a const object.
* The non-const overload below is fine.
*/
void setOwner ATLAS_NOT_THREAD_SAFE (SurfaceOwner x) const
{ const_cast<SurfaceOwner&> (m_owner) = x; }
/* set Ownership */
void setOwner(SurfaceOwner x)
{ m_owner = x; }
/** return ownership */
SurfaceOwner owner() const
{ return m_owner; }
i.e
- A. Provide non-const setter methods, as they should have been to start with.
- B. Leave the const setters for now there as to not break too much code.
- C. Mark the const setters explicitly as non-thread safe
- D. The idea is to migrate the clients in a staged manner and then remove the non Thread safe ones... obviously new code using setter on const objects will get warnings from the checker.
- E see ATLASRECTS-4957 for such a case
Client packages (e.g TrkGeometry)
-
Changes due to using CxxUtils::CachedUniquePtrT
-
They include methods like
move
,resize
which are marked as const , but at least some of them are effectivelysetters
. -
For now I added comments on those to motivate a bit of a closer look. One package at a time I guess .... this is just the 1st step...
Mentioning @ssnyder.