windows-drivers-rs
windows-drivers-rs copied to clipboard
Eliminate the need for `call_unsafe_wdf_function_binding` via helper lib
This supercedes #50 if you decide to go for this one.
Context: All Wdf* functions are simply inline stubs that look up the corresponding function from WdfFunctions and call it (with an additional WdfDriverGlobals parameter).
Normally, bindgen will not generate bindings to these functions because they are marked as inline (and thus have no implementation that Rust can link to).
However, we can create a helper object via a C compiler that implements these functions (by hackily disabling the FORCEINLINE macro 😉) and then use that as the link target for the Rust declarations.
All said and done, this allows us to avoid having to generate code via macro to call the Wdf* functions.
This makes for a much nicer end-user experience.
Interesting PR. We are actually planning on using the "cc in build.rs" approach for solving some issues with several things in WDF relying on global variables being declared in headers. These variables wont generate something we can link against, unless they are included in a .c file and compiled. An example of this is WdfMinimumVersionRequired. Right now, windows-drivers-rs works around this by manually redefining some of the symbols in rust.
However, I'm wary about doing that for all the inlined WDF functions. The way it's done now with the macro, it mirrors exactly how the calling the function in C/C++ would behave. The change in this PR would add an extra layer of indirection by making a call to the compiled helper lib, which would not be possible for the compiler to optimize away. The bindgen docs specifically call out this perf issue:
Note that these functions and methods are usually marked inline for a reason: they tend to be hot. The above workaround makes them an out-of-line call, which might not provide acceptable performance.
It also seems that bindgen now has built-in support to doing something like this automatically via wrap_static_fns. According to this discussion, it seems that LTO may resolve the extra overhead I mentioned above. But extra care is also needed to make sure a change like this doesn't break cross-compilation (which I am fixing currently in #35 )
We are actually planning on using the "cc in build.rs" approach for solving some issues with several things in WDF relying on global variables being declared in headers. These variables wont generate something we can link against, unless they are included in a .c file and compiled. An example of this is WdfMinimumVersionRequired. Right now, windows-drivers-rs works around this by manually redefining some of the symbols in rust.
Ah neat - I had actually accidentally encountered this fix as well but disabled it because my assumption was that you wanted it the other way around :)
However, I'm wary about doing that for all the inlined WDF functions. The way it's done now with the macro, it mirrors exactly how the calling the function in C/C++ would behave. The change in this PR would add an extra layer of indirection by making a call to the compiled helper lib, which would not be possible for the compiler to optimize away. The bindgen docs specifically call out this perf issue:
Indeed, I'm aware of the extra layer of indirection. I had hoped that this wouldn't be a concern for you, given that:
- I had assumed that perf wouldn't be a huge concern at this (what I assume is an) early stage of development.
- I can't imagine this indirection is at all significant compared to the dynamic dispatch via
WdfFunctions. - As you mentioned, Rust has the LTO stage available to it that could probably mitigate this concern. I'm not sure about the
wrap_static_fnsfeature you mentioned above - the WDF functions are not static so I don't think this will apply?
Lots of assumptions there :)
While that isn't too ideal, IMO, the tradeoff for perf here is absolutely worth it given the improved ergonomics: rust-analyzer now has full analysis available to it for WDF functions, the function calls in user code are 1:1 with actual functions, etc.
Additionally, this performance tradeoff doesn't have to be permanent.
This PR is mostly intended to be a stepping stone - ideally these wrapper functions would be fully implemented in Rust (with #[inline(always)]) and otherwise automatically generated bindgen-style.
I was going to generate these wrappers at first, but saw that it would duplicate a lot of functionality from bindgen or otherwise wouldn't play nice.
Maybe once wdkmetadata is in a better state?