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

Use efficient operations when copying bytes between Dart and Java #1019

Merged
merged 4 commits into from
Sep 14, 2023

Conversation

brianquinlan
Copy link
Collaborator

@brianquinlan brianquinlan commented Sep 13, 2023

  • Use the efficiency methods introduced in package:jni 0.7.0 to copy bytes between Dart and Java.
  • Reduces the time to copy a 1MB buffer from 400ms to 400µs on Dart 3.1.0

  • I’ve reviewed the contributor guide and applied the relevant portions to this PR.
Contribution guidelines:

Note that many Dart repos have a weekly cadence for reviewing PRs - please allow for some latency before initial review feedback.

@HosseinYousefi
Copy link
Member

HosseinYousefi commented Sep 13, 2023

Not directly related to this PR:

  1. If you add implementation "com.google.android.gms:play-services-cronet:18.0.1" to the example's app/build.gradle as well (even though it's not necessary – as the plugin has it), jnigen can correctly generate the bindings after running flutter build apk in example/.

  2. Consider adding a consumer-rules.pro to cronet and configure it in build.gradle. This will you can keep the cronet classes after tree shaking (in release mode). This way, users don't have to manually add a proguard-rules.pro.

@brianquinlan
Copy link
Collaborator Author

Not directly related to this PR:

  1. If you add implementation "com.google.android.gms:play-services-cronet:18.0.1" to the example's app/build.gradle as well (even though it's not necessary – as the plugin has it), jnigen can correctly generate the bindings after running flutter build apk in example/.

Done.

  1. Consider adding a consumer-rules.pro to cronet and configure it in build.gradle. This will you can keep the cronet classes after tree shaking (in release mode). This way, users don't have to manually add a proguard-rules.pro.

Done.

@dcharkes
Copy link

DBC: Could you post some ball-park perf improvements here?

Copy link
Member

@HosseinYousefi HosseinYousefi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM! Thanks @brianquinlan!

@brianquinlan
Copy link
Collaborator Author

DBC: Could you post some ball-park perf improvements here?

Done in the commit message.

@brianquinlan brianquinlan merged commit e19094a into dart-lang:master Sep 14, 2023
15 checks passed
@dcharkes
Copy link

  • Reduces the time to copy a 1MB buffer from 400ms to 400µs

Sweet!

@dcharkes
Copy link

@brianquinlan what Dart/Flutter version was this?

cc @sstrickl our downstream users benefitting from your recent stack of improvements to setRange! 🚀

@brianquinlan
Copy link
Collaborator Author

3.1.0 - I updated the commit message

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

Successfully merging this pull request may close these issues.

3 participants