Skip to content

Remove TE in favor of standard polymorphism in SurfaceArray and Axis

Paul Gessinger-Befurt requested to merge ACTS-439-surfacearray-std-poly into master

So it turns out that in fact, we don't necessarily need the boost type-erasure for the SurfaceArray. If the public interface is really completely independent from the number of dimensions, then a regular pure virtual base class does the job.

This MR does two things:

  1. The surface grid lookup concept goes away and is replaced by a normal ISurfaceGridLookup pure virtual base class.

  2. Back in the original SurfaceArray MR, I also introduced a concept for Axis, which allows the getAxes methods to just
    return a std::vector<concept::AnyAxis<>> instead of a tuple, which is much easier to work with for inspection. For this MR, I also
    changed this to a pure virtual base class IAxis that does the same job, the methods then return a std::vector<const IAxis*>.

Performance wise, for 1) I think this should have a positive effect, since instead of using boost's custom vtable implementation that dispatches method calls to the Any* objects, we now use the regular C++ vtable mechanism.

For 2) this might be a bit different; previously, the concepted AnyAxis was only ever used if you called the getAxes method, all other method calls inside of Grid were done directly on the type.

Now this is still the case, but all Axis instances derive from IAxis, which means C++ has to deal with the inheritance at some point. However, Grid stores the Axes as members through a tuple directly without pointers. Casting to the base class IAxis is only done when calling getAxes. My understanding is, that since the compiler knows what concrete type each Axis has at compile time, and no indirection is needed because it's not a pointer, it will resolve the inheritance at compile time, and thus not have to do vtable lookups when the Grid calls their methods.

Does anyone know if that is the case? (@hgraslan @asalzbur @msmk @jhrdinka @rlangenb @fklimpel)

If it is not the case, I'd suggested keeping the TE there, since then we certainly do not have that issue.

Closes ACTS-439 Fixes ACTS-445

Edited by Paul Gessinger-Befurt

Merge request reports