Skip to content

Conversation

SoilRos
Copy link
Member

@SoilRos SoilRos commented Jul 4, 2025

In this PR, the idea is that the geometry holds all the corners of the cell. This because in most of the cases, the geometry is constructed in place and the compiler theoretically could see through the things that are actually needed and only generate code for that. In other words, it would avoid making the copy if those values are not used.

Here is a benchmark for that hypothesis: https://quick-bench.com/q/IPXA-MuhVrdQ_9DSbgbcAyRW9ek

  • Indeed, this seems to be the case in GCC and there is no difference between storing the values or the pointers to the values.

8ek-EyAlGhyT2wGlEONCBPsEi3g

  • Sadly, the picture changes for Clang and storing the values is 5 times more expensive even if only the center of the geometry is being used.

IPXA-MuhVrdQ_9DSbgbcAyRW9ek

Note that this PR is based on #885 which fixes all the lifetime issues of the geometry objects.

@SoilRos SoilRos marked this pull request as draft July 4, 2025 14:01
@blattms blattms added the manual:enhancement This is an enhancement/improvent that needs to be documented in the manual label Jul 9, 2025
@blattms
Copy link
Member

blattms commented Jul 9, 2025

jenkjns build this please

@blattms
Copy link
Member

blattms commented Jul 9, 2025

jenkins build this please

@aritorto aritorto mentioned this pull request Jul 14, 2025
@SoilRos SoilRos force-pushed the bugfix/return-geometry-by-value-3 branch 2 times, most recently from 9a8a719 to 9799277 Compare July 31, 2025 14:49
@SoilRos
Copy link
Member Author

SoilRos commented Jul 31, 2025

jenkins build this serial please

SoilRos and others added 3 commits August 1, 2025 12:38
This is to conform with the dune interface
To avoid copying a shared pointer, which is rather costly, the geometry now stores the corners holding either the ownership of the entity variables.
@SoilRos SoilRos force-pushed the bugfix/return-geometry-by-value-3 branch from 9799277 to deba814 Compare August 1, 2025 10:38
@SoilRos
Copy link
Member Author

SoilRos commented Aug 1, 2025

jenkins build this serial please

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

manual:enhancement This is an enhancement/improvent that needs to be documented in the manual

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants