Adds Tonic templates for Dart FFI bindings.#29405
Conversation
There was a problem hiding this comment.
First round of comments, I'll take a deeper dive later.
High level comment: The commit message should have a detailed explanation of why and how:
- What is the goal of the FfiNatives, why do we want to start using them in Tonic.
- How do the FfiNatives (roughly) work compared to the old natives.
- (Which then explains the motivation for adding these to the
Convertors.) - Motivate the serialization.
We'd want to sent this PR to a Flutter dev as well to review. And that description would help them (and me).
dcharkes
left a comment
There was a problem hiding this comment.
lgtm with comments.
Assuming that the 'why' is:
- Replace existing
DartDispatcherwithFfiDispatchercalls later. - Generate bindings from all
FfiDispatchertemplates.
For 2. I would expect also a test or some code showing how that is done. (To prevent that we land something here that turns out to not work for the use case of generating code.)
After the comments are addressed, I'd like someone who understands the workings and/or use of the current DartDispatcher to take a look as well.
|
@chinmaygarde, @iskakaushik would the two of you like to review this change as well, or are you fine with Daco's review? |
|
I'll take a stab at this today but won't be able to fully review it. It requires a fair bit of context to be paged in. |
b1a8771 to
2962099
Compare
|
@chinmaygarde I'd be happy to set up a VC for us to discuss the changes, if that would be helpful? |
iskakaushik
left a comment
There was a problem hiding this comment.
minor nits, but LGTM overall.
|
@mkustermann Since @cskau-g is on leave, is there anyone else who can take this to completion? Since this has an LGTM already, perhaps we should land this? |
|
This pull request executed golden file tests, but it has not been updated in a while (20+ days). Test results from Gold expire after as many days, so this pull request will need to be updated with a fresh commit in order to get results from Gold. |
Apologies for the late reply. I think @cskau-g can land it when he's back (~ 2 weeks) |
This change adds Tonic templates for automatically generating bindings for Dart FFI, as well as serialisation of the bindings.
FfiDispathertemplates to the existingDartDispatcherused for (old) native bindings.DartConverters with conversions to and from the FFI transport types.ffi_native_unittest.cc) for the above.This will allow us to replace the existing native functions in e.g.
dart:uiwith new Dart FFI native functions.The benefit of FFI calls over the old native calls is that FFI automatically handles data conversion for a number of primitive types like int, double and bool, and thus does not require the overhead of calling Dart API within the native code to deal with Handles.
Additionally, for code that does not use Handles, we can avoid further overhead of setting up frames and transitioning VM state.
With
FfiDispatchers in place to set up all the new entry points, the remaining steps to convert the existing native functions to FFI are: 1) setting up the native function resolver, and 2) converting the functions like:The serialisation of the bindings will additionally allow us to automatically generate the above conversions, and subsequently validate all bindings in Dart code match their native counterparts.
Issues: