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

refactor: native FFI for Android and Cocoa SDK #1622

Closed
wants to merge 6 commits into from
Closed

Conversation

vaind
Copy link
Collaborator

@vaind vaind commented Sep 1, 2023

📜 Description

This is an evaluation/PoC for integration of the iOS/macOS (a.k.a. Cocoa) SDK as well as Android (Java) SDK via direct FFI invocation, avoiding method channels.

Tracking issue: #1444

Currently, this PR only ads the configuration & code generated to make the necessary change & I've tested a full call roundtrip for both SDKs.

💡 Motivation and Context

💚 How did you test it?

📝 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

Decide whether we want to go ahead and implement this fully - replacing the functionality currently implemented in platform-specific method channels.

@github-actions
Copy link
Contributor

github-actions bot commented Sep 1, 2023

Android Performance metrics 🚀

  Plain With Sentry Diff
Startup time 346.02 ms 421.12 ms 75.10 ms
Size 6.16 MiB 7.18 MiB 1.02 MiB

Baseline results on branch: main

Startup times

Revision Plain With Sentry Diff
fcd1ee4 298.96 ms 376.04 ms 77.09 ms
870f5eb 329.45 ms 369.29 ms 39.84 ms
fe4aa56 356.06 ms 428.67 ms 72.61 ms
0db91cc 327.85 ms 387.31 ms 59.46 ms
7ade5af 341.04 ms 386.84 ms 45.80 ms
d7758e8 300.12 ms 349.88 ms 49.76 ms
457a85b 312.37 ms 376.67 ms 64.31 ms
bf4aed7 311.24 ms 365.66 ms 54.42 ms
b728df4 390.98 ms 445.81 ms 54.83 ms
f2db4ec 372.46 ms 469.72 ms 97.26 ms

App size

Revision Plain With Sentry Diff
fcd1ee4 6.16 MiB 7.13 MiB 1000.85 KiB
870f5eb 5.94 MiB 6.92 MiB 1005.77 KiB
fe4aa56 6.06 MiB 7.10 MiB 1.04 MiB
0db91cc 5.94 MiB 6.95 MiB 1.01 MiB
7ade5af 5.94 MiB 6.95 MiB 1.01 MiB
d7758e8 5.94 MiB 6.95 MiB 1.01 MiB
457a85b 6.06 MiB 7.09 MiB 1.03 MiB
bf4aed7 6.06 MiB 7.03 MiB 997.04 KiB
b728df4 5.94 MiB 6.95 MiB 1.01 MiB
f2db4ec 6.06 MiB 7.03 MiB 990.27 KiB

Previous results on branch: refactor/native-ffi

Startup times

Revision Plain With Sentry Diff
84d609c 339.49 ms 409.26 ms 69.77 ms

App size

Revision Plain With Sentry Diff
84d609c 6.16 MiB 7.18 MiB 1.02 MiB

@github-actions
Copy link
Contributor

github-actions bot commented Sep 1, 2023

Fails
🚫 Please consider adding a changelog entry for the next release.

Instructions and example for changelog

Please add an entry to CHANGELOG.md to the "Unreleased" section. Make sure the entry includes this PR's number.

Example:

## Unreleased

- native FFI for Android and Cocoa SDK ([#1622](https://github.com/getsentry/sentry-dart/pull/1622))

If none of the above apply, you can opt out of this check by adding #skip-changelog to the PR description.

Generated by 🚫 dangerJS against ae34a42

@github-actions
Copy link
Contributor

github-actions bot commented Sep 1, 2023

iOS Performance metrics 🚀

  Plain With Sentry Diff
Startup time 1245.24 ms 1247.15 ms 1.91 ms
Size 8.29 MiB 9.38 MiB 1.09 MiB

Baseline results on branch: main

Startup times

Revision Plain With Sentry Diff
211a7aa 1258.02 ms 1275.65 ms 17.63 ms
f0fcbe1 1238.08 ms 1243.44 ms 5.36 ms
55cc6ef 1271.92 ms 1281.04 ms 9.12 ms
ddc97ad 1228.02 ms 1232.61 ms 4.59 ms
6aab859 1245.14 ms 1247.59 ms 2.45 ms
e893df5 1247.90 ms 1262.31 ms 14.41 ms
6572f8d 1242.16 ms 1246.63 ms 4.47 ms
6d7a391 1265.65 ms 1289.98 ms 24.33 ms
56810ff 1267.59 ms 1293.48 ms 25.89 ms
1e781fc 1257.96 ms 1281.49 ms 23.53 ms

App size

Revision Plain With Sentry Diff
211a7aa 8.10 MiB 9.18 MiB 1.08 MiB
f0fcbe1 8.29 MiB 9.38 MiB 1.09 MiB
55cc6ef 8.10 MiB 9.18 MiB 1.08 MiB
ddc97ad 8.29 MiB 9.37 MiB 1.08 MiB
6aab859 8.29 MiB 9.36 MiB 1.07 MiB
e893df5 8.09 MiB 9.07 MiB 1001.04 KiB
6572f8d 8.29 MiB 9.36 MiB 1.07 MiB
6d7a391 8.16 MiB 9.16 MiB 1.01 MiB
56810ff 8.15 MiB 9.12 MiB 987.35 KiB
1e781fc 8.10 MiB 9.18 MiB 1.08 MiB

Previous results on branch: refactor/native-ffi

Startup times

Revision Plain With Sentry Diff
84d609c 1229.24 ms 1234.31 ms 5.07 ms

App size

Revision Plain With Sentry Diff
84d609c 8.29 MiB 9.38 MiB 1.09 MiB

@codecov
Copy link

codecov bot commented Sep 1, 2023

Codecov Report

Patch has no changes to coverable lines.

❗ Current head ae34a42 differs from pull request most recent head 8367da2. Consider uploading reports for the commit 8367da2 to get more accurate results

Files Changed Coverage
...r/lib/src/integrations/native_sdk_integration.dart ø

📢 Thoughts on this report? Let us know!.

@vaind vaind mentioned this pull request Sep 7, 2023
6 tasks
@getsantry
Copy link

getsantry bot commented Oct 18, 2023

This issue has gone three weeks without activity. In another week, I will close it.

But! If you comment or otherwise update it, I will reset the clock, and if you remove the label Waiting for: Community, I will leave it alone ... forever!


"A weed is but an unloved flower." ― Ella Wheeler Wilcox 🥀

@getsantry getsantry bot added the Stale label Oct 18, 2023
@vaind
Copy link
Collaborator Author

vaind commented Apr 23, 2024

closing because

  • cocoa FFI has already been in use for profiling for a while
  • java FFI is coming for replay in another PR

@vaind vaind closed this Apr 23, 2024
@vaind vaind deleted the refactor/native-ffi branch April 23, 2024 11:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

1 participant