Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Move the internal FEValuesViews functions into a new file. #15895

Merged
merged 3 commits into from Aug 21, 2023

Conversation

drwells
Copy link
Member

@drwells drwells commented Aug 19, 2023

See also #13949 and #15829.

Most of the compilation work for FEValues is actually the inner functions used by FEValuesViews objects. Explicitly instantiating these functions in their own translation unit significantly lowers compilation time. Here are the numbers for release mode:

master:

  • fe_values_views: 7:58 wall time, 4.5 GB max RSS

this branch:

  • fe_values_views: 0:32 wall time, 1.6 GB max RSS
  • fe_values_views_internal: 2:58 wall time, 2.8 GB max RSS

I had to adjust the type signatures (on the first ArrayView argument) to avoid instantiating things for 'const float' rather than float et al.

Most of the compilation work for FEValues is actually the inner functions used
by FEValuesViews objects. Explicitly instantiating these functions in their own
translation unit significantly lowers compilation time. Here are the numbers for
release mode:

master:
- fe_values_views: 7:58 wall time, 4.5 GB max RSS

feature:
- fe_values_views: 0:32 wall time, 1.6 GB max RSS
- fe_values_views_internal: 2:58 wall time, 2.8 GB max RSS

I had to adjust the type signatures (on the first ArrayView argument) to avoid
instantiating things for 'const float' rather than float et al.
Copy link
Member

@kronbichler kronbichler left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a nice improvement in memory usage and times, thank you! I am in full support of this. I have some unrelated notes of idioms that were present before but are inefficient - not sure if we care enough about performance to change them, because these functions are having the right data structure to do things efficiently anyway.

source/fe/fe_values_views_internal.cc Outdated Show resolved Hide resolved
source/fe/fe_values_views_internal.cc Outdated Show resolved Hide resolved
@kronbichler kronbichler merged commit b9c13a2 into dealii:master Aug 21, 2023
15 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants