VkFFT icon indicating copy to clipboard operation
VkFFT copied to clipboard

[Announcement] Rust bindings for VkFFT

Open bmcdorman opened this issue 4 years ago • 5 comments

Hi,

I've put together some Rust bindings for VkFFT. You can find it here. It's certainly incomplete (particularly in documentation, examples, and safety guarantees), but I think it's close enough for other people to get some use from. Hopefully I'll be able to keep giving it some love for a while.

It assumes the Vulkan backend is used. Unfortunately the other backends would probably be best expressed through completely separate bindings. It further assumes vulkano (idiomatic Rust bindings for Vulkan) is also used.

I've appended a snippet of the first kernel FFT from sample_9 to give you a taste of the API.

Thanks for creating VkFFT!

// Configure kernel FFT
let config = Config::builder()
  .physical_device(context.physical)
  .device(context.device.clone())
  .fence(&context.fence)
  .queue(context.queue.clone())
  .buffer(kernel.clone())
  .command_pool(context.pool.clone())
  .kernel_convolution()
  .normalize()
  .coordinate_features(2)
  .batch_count(2)
  .r2c()
  .disable_reorder_four_step()
  .dim(&[32, 32])
  .build()?;

// Allocate a command buffer
let cmd_buffer = context.alloc_primary_cmd_buffer()?;

// Create command buffer handle
let builder =
  unsafe { UnsafeCommandBufferBuilder::new(&cmd_buffer, Kind::primary(), Flags::None)? };

// Configure FFT launch parameters
let mut params = LaunchParams::builder()
  .command_buffer(&builder)
  .build()?;

// Construct FFT "Application"
let mut app = App::new(config)?;

// Run forward FFT
app.forward(&mut params)?;

// Dispatch command buffer and wait for completion
let cmd_buffer = builder.build()?;
context.submit(cmd_buffer)?;

bmcdorman avatar Apr 22 '21 04:04 bmcdorman

Hello,

That's great to hear! Most certainly it will be useful for some people in the future. I will add the link to your repo to the description in the next update. If you have some thoughts/questions about VkFFT you encountered during the development - feel free to share them!

Best regards, Dmitrii Tolmachev

DTolm avatar Apr 22 '21 07:04 DTolm

Great! There were some things that could've made the process easier, but they're all from a code perspective. Functionality and performance are superb.

  • It would be great to split some of the VkFFT implementation into separate files. The implementation (and examples/benchmarks) are a very dense read right now. It took me a few days to wrap my head around it.
  • All of the VkFFT functions are marked static inline, which prevents the symbols from being exported in a library. We have to rewrite the VkFFT.h header as a part of vkfft-rs's build process to remove those keywords to compile a static library Rust can link against. I could probably open a PR to add a #define for this, though. One probably doesn't want those very large functions being marked inline anyway, as it could dramatically increase code size, and as a result, decrease cache hits. I imagine the C++ compiler isn't inlining them, though, as inline is merely a suggestion to the compiler.
  • Currently buffers can be specified in either the Configuration or LaunchParams. If they're present in the LaunchParams, the values in the Configuration are overwritten. This mutability causes issues with Rust's safety guarantees, as a shared pointer to a buffer specified in the LaunchParams will persist in the Configuration. If the user constructs another LaunchParams without the buffer specified the VkFFT implementation could attempt to use a buffer that went out of scope and was deallocated. Under typical use these issues shouldn't arise, and there are workarounds (for example, nulling the "cached" values in the Configuration after every launch operation), but as a result of things like that it's very difficult to make the safety guarantees Rust expects.

I'm still working through some parts of the implementation, so I'll keep you posted as questions arise!

Thanks again!

bmcdorman avatar Apr 22 '21 08:04 bmcdorman

Thank you for your response! I will briefly go over your suggestions:

  • At this moment of development, I would like to keep VkFFT as a header-only library. I will probably split the benchmark into multiple files, though (similar to cuFFT/rocFFT scripts).
  • As far as I know, static keyword is needed if you have multiple inclusions of a header. The inline keyword doesn't really play much of a role and can be removed, that's true. Overall, most time is taken by the shader compiler (glslang, NVRTC, HIPRTC) anyway.
  • That was the point of LaunchParams - to provide a way to modify some values in the configuration after the plan creation. Nulling the cached value will force to update descriptor sets at every launch. A better way can be to force the user to provide the buffers in every LaunchParams - VkFFT already checks if they have changed compared to the cached version and will update if this happened.

Best regards, Dmitrii Tolmachev

DTolm avatar Apr 22 '21 12:04 DTolm

Thanks for the information! I'll probably require the buffers are specified in the LaunchParams. Retaining the original performance characteristics of the core library is a priority for us. In regards to the single header, I understand the appeal... perhaps as a part of the compilation process a single "amalgamated" header could be produced? I think that might strike the right balance between ease-of-use and implementation organization. I understand that's probably not a priority regardless, but thought I'd throw it out there.

In regards to the functions being static, one approach might be something like dr_wav does. They have a preprocessor define that determines if the implementations are included or just the prototypes (e.g., #define DR_WAV_IMPLEMENTATION). That said, the rewriting method we're using will suffice.

bmcdorman avatar Apr 22 '21 16:04 bmcdorman

Code organization improvement is a continuous process in VkFFT. Originally, I had over 100k lines of Vulkan static shaders (version 1.1.0) covering only a fraction of functionality that it is there today. The run-time generation solved this but if I started writing in it from the beginning - I probably would not have finished at all. The performance and features are the main driving force for me - and this is what really matters to the majority of users. Rest will come along with time.

I already have #define VKFFT_H and it will probably suffice to the need of static keyword (it will disable any further inclusions), though I have not tested it much. I just went with the safest way for now.

DTolm avatar Apr 23 '21 16:04 DTolm