Use less allocations on UGGrid intersection geometries

Currently, UGGrid intersection geometries do many allocations for very small objects. In simple simulations I get millions (!) of allocation calls due to this, a few orders of magnitude higher than any other allocation in a simulation. This happens because

• The geometry object is stored as a shared pointer. While this is handy when having several geometries referring to the same intersection, this is barely of any use as we mostly expect people to have one geometry and get it from the entity. An optional type is more appropriate here.
• The coordinates are a std::vector of field vectors that need to be created for every geometry. This MR uses the suggestion by @oliver.sander by storing the coordinates wither with std::array or Dune::ReservedVector depending on the dimension of the geometry.
• Every new intersection stores a std::vector of faces. While this is needed for the non-conforming case, it penalizes the common case where there is one or no faces neighboring the intersection. This can be solved by properly using a union type, i.e. std::variant

The current MR mitigates the first issue by using std::optional, the second by storing coordinates on std::array or Dune::ReservedVector, and the third one by using std::variant. All of them optimized to not allocate anything on the hot-path but still be flexible to allocate if needed.

added feature label

marked this merge request as draft

added 1 commit

• 4d673cd7 - Use optional & move coordinates into geometry

Compare with previous version

marked this merge request as ready

assigned to @santiago.ospina

requested review from @oliver.sander

@oliver.sander I think we can merge this as it is since this should be straight forward and uncontroversial, and we can discuss the second problem elsewhere.

changed the description

added 1 commit

• 0b8f3e01 - Add wrapper that generates coordinates on the fly

Compare with previous version

added 1 commit

• 49d8bed2 - Add wrapper that generates coordinates on the fly

Compare with previous version

marked this merge request as draft

added 1 commit

• 675cccfd - Add wrapper that generates coordinates on the fly

Compare with previous version

changed the description

changed the description

added 3 commits

• b4eaafa3 - Add wrapper that generates coordinates on the fly
• 6f23ef77 - Avoid allocating a vector of faces when intersection has only one face
• 82bc3962 - Reduce size of functors to avoid allocations

Compare with previous version

changed the description

added 1 commit

• ebc64415 - Reduce size of functors to avoid allocations

Compare with previous version

added 1 commit

• a2995756 - Reduce size of functors to avoid allocations

Compare with previous version

added 1 commit

• 9cc211bd - Add CHANGELOG entry

Compare with previous version

Since the whole point of this MR is performance, are there any benchmarks?

Santiago, thanks for working on this. It would be helpful, though, if we could split the MR into two parts. Replacing smart pointers by std::optional looks convincing to me, and we could probably merge that quickly. The rest of your changes is less obvious, and would need more time.

added 11 commits

• 9cc211bd...1c7d360a - 6 commits from branch master
• ef739516 - Use optional & move coordinates into geometry
• 7d272afe - Add wrapper that generates coordinates on the fly
• 34a0b3a9 - Avoid allocating a vector of faces when intersection has only one face
• 35a0de4a - Reduce size of functors to avoid allocations
• 41921dd0 - Add CHANGELOG entry

Compare with previous version

added 3 commits

• 12d7c9c4 - Use std::optional to store multilinear geometries & move coordinates
• 4786d835 - Use std::function to (maybe) generate coordinate geometry on the fly
• e4c7ac9a - Gather UGGrid performance improvements in CHANGELOG

Compare with previous version

added 3 commits

• b0b7ebe5 - Use std::function to (maybe) generate coordinate geometry on the fly
• c6fc552c - Use std::variant in intersection to store a face, a face vector, or none
• 2be2fc82 - Gather UGGrid performance improvements in CHANGELOG

Compare with previous version

changed the description

added 2 commits

• 00285afb - Use std::variant in intersection to store a face, a face vector, or none
• 24f369c3 - Gather UGGrid performance improvements in CHANGELOG

Compare with previous version

added 5 commits

• 24f369c3...287f7fe7 - 2 commits from branch master
• 008a3826 - Use std::optional to store multilinear geometries & move coordinates
• a790c5a9 - Use std::variant in intersection to store a face, a face vector, or none
• feff7a8b - Store UGGeometry corner coordinates on the stack

Compare with previous version

changed the description

Thank you Santiago. Please fix the three very minor issues that I found, and then feel free to merge yourself.

added 2 commits

• 75e259d2 - Use std::variant in intersection to store a face, a face vector, or none
• 22c468b1 - Store UGGeometry corner coordinates on the stack

Compare with previous version

marked this merge request as ready

enabled an automatic merge when all merge checks for 22c468b1 pass

resolved all threads

merged

mentioned in commit 5a4a50f8

Hi @santiago.ospina , this merge request produced new signed vs. unsigned warnings. Can you please fix those? Thank you!

mentioned in merge request !759 (closed)

resolved all threads