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

Arrays of buffer sizes lifetime #184

Open
CoffeeExterminator opened this issue Sep 17, 2024 · 5 comments
Open

Arrays of buffer sizes lifetime #184

CoffeeExterminator opened this issue Sep 17, 2024 · 5 comments

Comments

@CoffeeExterminator
Copy link

Hi,
I have found out (the hard way) that the buffer sizes (bufferSize, inputBufferSize, etc.) provided in the configuration as pointers are required even after initialization and thus must be kept alive. Meaning, that if I do something like this

config.bufferSize = (uint64_t *) &size

The size variable must exist even after the initialization is completed, which, at least to me, was not the expected behavior (I understand they are in fact arrays since it's possible to use multiple buffers, but still): my assumption was that I can forget about config entirely after the initialization. Could you please clarify if there are any other parameters passed as pointers required after initialization in a similar manner? What about VkFFTLaunchParams? Are the structure or any of its fields required after VkFFTAppend completion? I have done some tests and it seems like they aren't, but asking just in case anyway.

@fixgoats
Copy link

I was literally just figuring out this was the problem I was having with my code and I feel kind of silly for not having figured it out earlier. I think you just need to apply the normal lifetime heuristics: when you assign any pointer to a field in the config, if the data it points to is stack allocated and you go out of the scope where the allocation occurs then you have a dangling pointer. This kind of stuff is why RAII is not just a buzzword but actually a useful practice that makes life a whole lot easier.

DTolm added a commit that referenced this issue Sep 23, 2024
-Implemented radix codelets up to 47.
-Implemented composite radix codelets for arbitrary composite stage sizes.
-Implemented new register assignment logic, aimed at optimizing shared memory transfers, register usage and warp utilization.
-Performance improvements for all system sizes - please report regressions if they happen (especially for vendors other than Nvidia and AMD).
-All double pointers passed to VkFFT now make local copy of their contents (#184, #185)
-Fixed locale setting for code generator (vincefn/pyvkfft#38)
@DTolm
Copy link
Owner

DTolm commented Sep 23, 2024

Hello,

I made a change in the last update on develop branch, that VkFFT now stores the copy of all data that is passed as a pointer. This should solve the issue and now all contents of the config can be deallocated after completion of initializeVkFFT (unless I messed up something). The underlying structs that are passed (like VkDevice) still must not be released before the call to deleteVkFFT.

Best regards,
Dmitrii

DTolm added a commit that referenced this issue Sep 23, 2024
-Implemented radix codelets up to 47.
-Implemented composite radix codelets for arbitrary composite stage sizes.
-Implemented new register assignment logic, aimed at optimizing shared memory transfers, register usage and warp utilization.
-Performance improvements for all system sizes - please report regressions if they happen (especially for vendors other than Nvidia and AMD).
-All double pointers passed to VkFFT now make local copy of their contents (#184, #185)
-Fixed locale setting for code generator (vincefn/pyvkfft#38)
@CoffeeExterminator
Copy link
Author

CoffeeExterminator commented Oct 9, 2024

Hi,
I have tested the develop branch and the issue seems fixed now, thank you.

The underlying structs that are passed (like VkDevice) still must not be released before the call to deleteVkFFT.

Got a little confused about this. What about VkQueue, VkCommandPool and VkFence used in the config?

@DTolm
Copy link
Owner

DTolm commented Oct 9, 2024

I just checked, VkQueue, VkCommandPool and VkFence can be released after the initialization call as they are only used for internal GPU calculations of LUT. Will update the documentation with that. Thanks!

@CoffeeExterminator
Copy link
Author

Thanks for clarifying!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants