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

Call fftw_cleanup to deallocate all memory allocated by FFTW #4289

Closed

Conversation

WeiqunZhang
Copy link
Member

FFTW caches a small amount of persistent memory. The memory "leak" is benign. Nevertheless, we call fftw_cleanup to deallocate all memory allocated by FFTW for sanitizers.

FFTW caches a small amount of persistent memory. The memory "leak" is
benign. Nevertheless, we call fftw_cleanup to deallocate all memory
allocated by FFTW for sanitizers.
@WeiqunZhang
Copy link
Member Author

This is for testing only. I am not sure we want to do that because

After calling fftw_cleanup, all existing plans become undefined, and you should not attempt to execute them nor to destroy them. You can however create and execute/destroy new plans, in which case FFTW starts accumulating wisdom information again.

@ax3l ax3l self-requested a review January 9, 2025 18:50
@ax3l
Copy link
Member

ax3l commented Jan 9, 2025

Yeah, it's a bit with the 🔨 because it might influence frameworks that use AMReX for part of their model. On the other hand, I see rocfft_cleanup and that does the same - so both cleanups (and inits) we might need to make conditional / able to reuse / keep existing contexts in the future to play nice when integrated in larger projects that persist after AMReX finalize.

The leak interestingly does not show in serial runs, only in those with MPI enabled. I also verified it with valgrind now (originally seen with Clang address sanitizer).
ECP-WarpX/impactx#790 (comment)

Testing this patch right now.

@ax3l
Copy link
Member

ax3l commented Jan 9, 2025

To try to make it more robust: For FFTW (and ROCfft), we could keep track if someone else initialized FFTW already (as we do with MPI). In that case, as we do for other libs, we should not finalize the context. At least for threaded FFTW, there is a global status var for that.

@ax3l
Copy link
Member

ax3l commented Jan 9, 2025

I see still the same issue with fftw_plan_many_dft_r2c and fftw_plan_many_dft_c2r even when using this patch.

Could be an FFTW issue with calling the plan creation multiple times?
https://stackoverflow.com/questions/22471184/checking-fftw3-with-valgrind

@WeiqunZhang
Copy link
Member Author

The memory leak should be fixed in #4290.

@WeiqunZhang WeiqunZhang closed this Jan 9, 2025
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.

2 participants