search

openMVG / openMVG

open Multiple View Geometry library. Basis for 3D computer vision and Structure from Motion.
Mozilla Public License 2.0
5.58k stars 1.66k forks source link

Eigen Alignment #1347

Open NikolausDemmel opened 6 years ago

NikolausDemmel commented 6 years ago

When working with the code I was wondering about how Eigen alignment is dealt with in OpenMVG.

For example, in https://github.com/openMVG/openMVG/blob/master/src/openMVG/types.hpp in the definition of Hash_Map the variant using std::map uses Eigen::aligned_allocator, but the variant using std::unordered_map does not. Briefly checking the usages of Hash_Map, it seems there might actually not be any instances where the alignment would actually be needed.

It seems the only types where EIGEN_MAKE_ALIGNED_OPERATOR_NEW is used are TrifocalTensorModel and PointFeature (+sub classes). The former does not appear in a hash map. The latter does, but it seems to me the alignment is actually not needed for PointFeature, since it has a single Vec2f member, which does not seem to be a vectorizable fixed-size Eigen type (size is 8 byte, which is no multiple of 16, see https://eigen.tuxfamily.org/dox/group__TopicFixedSizeVectorizable.html).

So is the aligned_allocator needed for std::map? And if so, why is it not used for std::unordered_map?

pmoulon commented 6 years ago

Thx for your feedback.

From what I remember when I look in detail to this I think this is related to the memory allocation model of the c++11 object of std::unordered_map.

So adding the aligned_allocator should not hurt.

BTW, any help you can provide to provide the best usage of Eigen is more than welcome. -> As you said perhaps I made mistake in some place like with the PointFeature struct.

pmoulon commented 6 years ago

@NikolausDemmel Any new comment?

NikolausDemmel commented 5 years ago

Sorry for the late reply.

BTW, any help you can provide to provide the best usage of Eigen is more than welcome. -> As you said perhaps I made mistake in some place like with the PointFeature struct.

Unfortunately, I'm not expert in this and still trying to find out myself all the intricacies of alignment.

IIUC, if missing alignment is an issue, it should always result in a segfault / crash. I.e. hopefully it should not result in silent failures.

Since there seem to be none of those, maybe there is no need to take action.

I could still add the aligned allocator to unordered_map or remove it from map in a PR, if you want.