Skip to content

Conversation

@buenaflor
Copy link
Contributor

@buenaflor buenaflor commented Sep 1, 2025

📜 Description

Refactor loadContexts and loadDebugImages to 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

  • iOS (FFI): shows to be on average ~2-3x faster than channels
  • Android (JNI): shows to be on average ~2x faster than channels

💚 How did you test it?

Integration test, manual tests

📝 Checklist

  • I reviewed submitted code
  • I added tests to verify changes
  • No new PII added or SDK only sends newly added PII if sendDefaultPii is enabled
  • I updated the docs if needed
  • All tests passing
  • No breaking changes

🔮 Next steps

@buenaflor buenaflor marked this pull request as ready for review September 1, 2025 14:58
cursor[bot]

This comment was marked as outdated.

Copy link
Collaborator

@denrase denrase left a 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.

@buenaflor
Copy link
Contributor Author

@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)

@buenaflor
Copy link
Contributor Author

@denrase changed up the benchmarks a bit and it's about ~2-3x faster than method channels (measured with average over 300 iterations)

cursor[bot]

This comment was marked as outdated.

Copy link
Collaborator

@denrase denrase left a 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.

@buenaflor
Copy link
Contributor Author

buenaflor commented Sep 2, 2025

LGTM. Pls check why CI is failing and ignore if it's unrelated to this PR.

  • sdk metrics and integration test ci is flaky as ever
  • package analysis fails because the binding generates a function that's unused, I guess we have to add a step to the script to generate an ignore comment

will update it

@buenaflor buenaflor merged commit 4110ac1 into main Sep 2, 2025
135 of 137 checks passed
@buenaflor buenaflor deleted the ffi-jni/load-contexts-load-debug-images branch September 2, 2025 15:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[FFI/JNI Refactor]: Contexts and Debug Images

4 participants