-
Notifications
You must be signed in to change notification settings - Fork 6k
dart:ui conversion from native to FfiNative #29607
Conversation
|
Does this yield better performance? |
Please note that this is a draft PR, for changes still being worked on. But yes, in general, FFI tends to have less overhead compared to the old native functions. For some quick benchmark numbers to get an idea of the difference, see: That said, end users are unlikely to see a noticeable impact on app performance from this improvement since the overhead of calling native functions in |
|
@cskau-g Are you still working on this PR? |
Yes. Though I've been out on leave for a while. |
|
It looks like this pull request may not have tests. Please make sure to add tests before merging. If you need an exemption to this rule, contact Hixie on the #hackers channel in Chat (don't just cc him here, he won't see it! He's on Discord!). If you are not sure if you need tests, consider this rule of thumb: the purpose of a test is to make sure someone doesn't accidentally revert the fix. Ask yourself, is there anything in your PR that you feel it is important we not accidentally revert back to how it was before your fix? Reviewers: Read the Tree Hygiene page and make sure this patch meets those guidelines before LGTMing. |
|
@gspencergoog are you a good candidate to review this change? If not can you help nominate someone who might be? |
|
@jason-simmons or @aam might be good reviewers |
|
I think the iOS failure is because of CI getting confused about where this applies to. Fixing that probably requires a rebase. Rebasing this patch would be painful. I think we should ignore that failure on presubmit. |
|
Wacky, it passed on a rerun. |
|
Note for when we reland this: |
|
Looks like we're missing test coverage of those two methods in the engine. |
This change converts
dart:uinative functions into FFI native functions.For example:
Becomes:
This makes use of the newly introduced FFI binding Tonic templates from c797abf
The benefit of using FFI native function calls instead of the old native function calls, is that FFI in many cases is faster, as seen in the benchmarks on https://dart-review.googlesource.com/c/sdk/+/218920.
It is however worth noting that there are certain cases where FFI native function calls (currently) have a larger overhead, due to known issues.
But in
dart:uithere is only one function that falls in that category (namelySemanticsUpdateBuilder::updateNodewith its 36 parameters). And benchmarking suggests that this has no measurable impact on actual app performance.