-
-
Notifications
You must be signed in to change notification settings - Fork 278
Refactor loadContexts and loadDebugImages to use JNI and FFI
#3224
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
Conversation
…exts-load-debug-images
denrase
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I suspect these speedups happen due to method channels being async and the event loop picking it up later vs FFI/JNI which runs sync
Does that mean we can't reliably say that this is faster, as the benchmark might be skewed through async/sync, with the latter blocking, while the former is doing other work? Any way to reliably benchmark this?
Might also be worth to benchmark the current encode/decode approach vs directly handing over data structures through FFI/JNI.
packages/flutter/android/src/main/kotlin/io/sentry/flutter/SentryFlutterPlugin.kt
Show resolved
Hide resolved
packages/flutter/android/src/main/kotlin/io/sentry/flutter/SentryFlutterPlugin.kt
Show resolved
Hide resolved
packages/flutter/ios/sentry_flutter/Sources/sentry_flutter/SentryFlutterFFI.swift
Outdated
Show resolved
Hide resolved
packages/flutter/android/src/main/kotlin/io/sentry/flutter/SentryFlutterPlugin.kt
Show resolved
Hide resolved
|
@denrase encoding decoding is 4x faster, I did some benchmarks on this Doing the conversion from NSDictionary was actually my first approach but noticed how slow it was (and complex) I'll do the benchmarks again regarding method channels (I'll start measuring inside the async body which should not include event loop time) |
|
@denrase changed up the benchmarks a bit and it's about ~2-3x faster than method channels (measured with average over 300 iterations) |
denrase
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM. Pls check why CI is failing and ignore if it's unrelated to this PR.
will update it |
📜 Description
Refactor
loadContextsandloadDebugImagesto use JNI and FFI for Android and iOS💡 Motivation and Context
Closes #3205
Local Benchmark Results
15 iterations of channel vs ffi/jni and calculating the avg while removing the first and last iteration to prevent extremes
💚 How did you test it?
Integration test, manual tests
📝 Checklist
sendDefaultPiiis enabled🔮 Next steps