Ref: Adds a Java only implementation in RNSentryMapConverter.convertToWritable #4469

antonis · 2025-01-21T09:22:53Z

📢 Type of change

Bugfix
New feature
Enhancement
Refactoring

Based on #4451

📜 Description

Refactors RNSentryMapConverter.convertToWritable and adds a Java only implementation

⛓️ PR Chain

💡 Motivation and Context

See #4451 (comment)
Part of #3608

💚 How did you test it?

CI, Manual

📝 Checklist

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.
I updated the wizard if needed.
All tests passing
No breaking changes

🔮 Next steps

#skip-changelog

github-actions · 2025-01-21T09:49:55Z

Android (new) Performance metrics 🚀

	Plain	With Sentry	Diff
Startup time	362.35 ms	395.92 ms	33.57 ms
Size	7.15 MiB	8.38 MiB	1.23 MiB

Baseline results on branch: antonis/init-from-json

Startup times

Revision	Plain	With Sentry	Diff
`236a90a`+dirty	384.20 ms	419.90 ms	35.69 ms
`f74989d`+dirty	265.81 ms	320.56 ms	54.75 ms
`ee5a152`+dirty	393.11 ms	458.70 ms	65.59 ms
`69b6731`+dirty	363.50 ms	409.44 ms	45.94 ms
`36893e3`+dirty	380.00 ms	431.02 ms	51.02 ms
`4ea1d1e`+dirty	384.08 ms	403.66 ms	19.58 ms
`1646f54`+dirty	399.30 ms	453.04 ms	53.74 ms

App size

Revision	Plain	With Sentry	Diff
`236a90a`+dirty	7.15 MiB	8.38 MiB	1.23 MiB
`f74989d`+dirty	7.15 MiB	8.38 MiB	1.23 MiB
`ee5a152`+dirty	7.15 MiB	8.38 MiB	1.23 MiB
`69b6731`+dirty	7.15 MiB	8.38 MiB	1.23 MiB
`36893e3`+dirty	7.15 MiB	8.38 MiB	1.23 MiB
`4ea1d1e`+dirty	7.15 MiB	8.38 MiB	1.23 MiB
`1646f54`+dirty	7.15 MiB	8.38 MiB	1.23 MiB

Previous results on branch: antonis/ref-convertToWritable

Startup times

Revision	Plain	With Sentry	Diff
`4ac2cff`+dirty	390.09 ms	401.72 ms	11.64 ms
`865c0d9`+dirty	342.12 ms	366.85 ms	24.73 ms
`6a80f44`+dirty	416.28 ms	487.90 ms	71.62 ms

App size

Revision	Plain	With Sentry	Diff
`4ac2cff`+dirty	7.15 MiB	8.38 MiB	1.23 MiB
`865c0d9`+dirty	7.15 MiB	8.38 MiB	1.23 MiB
`6a80f44`+dirty	7.15 MiB	8.38 MiB	1.23 MiB

github-actions · 2025-01-21T09:53:05Z

iOS (legacy) Performance metrics 🚀

	Plain	With Sentry	Diff
Startup time	1224.04 ms	1229.88 ms	5.84 ms
Size	2.63 MiB	3.69 MiB	1.05 MiB

Baseline results on branch: antonis/init-from-json

Startup times

Revision	Plain	With Sentry	Diff
`236a90a`+dirty	1222.65 ms	1221.98 ms	-0.67 ms
`f74989d`+dirty	1220.98 ms	1219.00 ms	-1.98 ms
`36893e3`+dirty	1211.35 ms	1212.33 ms	0.97 ms
`1646f54`+dirty	1237.86 ms	1238.08 ms	0.22 ms
`69b6731`+dirty	1218.08 ms	1223.96 ms	5.88 ms
`4ea1d1e`+dirty	1231.00 ms	1231.73 ms	0.73 ms
`ee5a152`+dirty	1233.22 ms	1227.27 ms	-5.96 ms

App size

Revision	Plain	With Sentry	Diff
`236a90a`+dirty	2.63 MiB	3.69 MiB	1.05 MiB
`f74989d`+dirty	2.63 MiB	3.68 MiB	1.05 MiB
`36893e3`+dirty	2.63 MiB	3.68 MiB	1.05 MiB
`1646f54`+dirty	2.63 MiB	3.68 MiB	1.05 MiB
`69b6731`+dirty	2.63 MiB	3.69 MiB	1.05 MiB
`4ea1d1e`+dirty	2.63 MiB	3.69 MiB	1.05 MiB
`ee5a152`+dirty	2.63 MiB	3.69 MiB	1.05 MiB

Previous results on branch: antonis/ref-convertToWritable

Startup times

Revision	Plain	With Sentry	Diff
`6a80f44`+dirty	1227.69 ms	1227.96 ms	0.26 ms
`865c0d9`+dirty	1218.47 ms	1220.91 ms	2.45 ms
`4ac2cff`+dirty	1209.27 ms	1211.71 ms	2.44 ms

App size

Revision	Plain	With Sentry	Diff
`6a80f44`+dirty	2.63 MiB	3.69 MiB	1.05 MiB
`865c0d9`+dirty	2.63 MiB	3.69 MiB	1.05 MiB
`4ac2cff`+dirty	2.63 MiB	3.69 MiB	1.05 MiB

github-actions · 2025-01-21T09:57:54Z

iOS (new) Performance metrics 🚀

	Plain	With Sentry	Diff
Startup time	1226.63 ms	1219.08 ms	-7.55 ms
Size	3.19 MiB	4.25 MiB	1.06 MiB

Baseline results on branch: antonis/init-from-json

Startup times

Revision	Plain	With Sentry	Diff
`236a90a`+dirty	1212.67 ms	1214.40 ms	1.73 ms
`f74989d`+dirty	1246.20 ms	1245.12 ms	-1.08 ms
`36893e3`+dirty	1231.54 ms	1231.40 ms	-0.15 ms
`1646f54`+dirty	1217.25 ms	1210.30 ms	-6.95 ms
`69b6731`+dirty	1221.39 ms	1219.30 ms	-2.08 ms
`4ea1d1e`+dirty	1240.67 ms	1246.71 ms	6.04 ms
`ee5a152`+dirty	1209.06 ms	1204.49 ms	-4.57 ms

App size

Revision	Plain	With Sentry	Diff
`236a90a`+dirty	3.19 MiB	4.25 MiB	1.06 MiB
`f74989d`+dirty	3.19 MiB	4.25 MiB	1.06 MiB
`36893e3`+dirty	3.19 MiB	4.25 MiB	1.06 MiB
`1646f54`+dirty	3.19 MiB	4.25 MiB	1.06 MiB
`69b6731`+dirty	3.19 MiB	4.25 MiB	1.06 MiB
`4ea1d1e`+dirty	3.19 MiB	4.25 MiB	1.06 MiB
`ee5a152`+dirty	3.19 MiB	4.25 MiB	1.06 MiB

Previous results on branch: antonis/ref-convertToWritable

Startup times

Revision	Plain	With Sentry	Diff
`6a80f44`+dirty	1229.63 ms	1221.77 ms	-7.86 ms
`865c0d9`+dirty	1229.65 ms	1229.57 ms	-0.08 ms
`4ac2cff`+dirty	1217.83 ms	1228.10 ms	10.27 ms

App size

Revision	Plain	With Sentry	Diff
`6a80f44`+dirty	3.19 MiB	4.25 MiB	1.06 MiB
`865c0d9`+dirty	3.19 MiB	4.25 MiB	1.06 MiB
`4ac2cff`+dirty	3.19 MiB	4.25 MiB	1.06 MiB

github-actions · 2025-01-21T10:12:00Z

Android (legacy) Performance metrics 🚀

	Plain	With Sentry	Diff
Startup time	444.13 ms	432.52 ms	-11.60 ms
Size	17.75 MiB	20.11 MiB	2.37 MiB

Baseline results on branch: antonis/init-from-json

Startup times

Revision	Plain	With Sentry	Diff
`f74989d`	480.30 ms	505.59 ms	25.29 ms
`236a90a`	417.78 ms	432.00 ms	14.22 ms
`ee5a152`	455.06 ms	471.58 ms	16.52 ms
`4ea1d1e`	473.92 ms	473.94 ms	0.02 ms
`36893e3`	416.35 ms	420.88 ms	4.53 ms
`69b6731`	475.62 ms	477.30 ms	1.67 ms
`1646f54`	351.27 ms	335.28 ms	-15.99 ms

App size

Revision	Plain	With Sentry	Diff
`f74989d`	17.75 MiB	20.11 MiB	2.37 MiB
`236a90a`	17.75 MiB	20.11 MiB	2.37 MiB
`ee5a152`	17.75 MiB	20.11 MiB	2.37 MiB
`4ea1d1e`	17.75 MiB	20.11 MiB	2.37 MiB
`36893e3`	17.75 MiB	20.11 MiB	2.37 MiB
`69b6731`	17.75 MiB	20.11 MiB	2.37 MiB
`1646f54`	17.75 MiB	20.11 MiB	2.37 MiB

Previous results on branch: antonis/ref-convertToWritable

Startup times

Revision	Plain	With Sentry	Diff
`865c0d9`	459.00 ms	456.18 ms	-2.82 ms
`4ac2cff`	445.40 ms	431.02 ms	-14.38 ms
`6a80f44`	465.20 ms	460.86 ms	-4.35 ms

App size

Revision	Plain	With Sentry	Diff
`865c0d9`	17.75 MiB	20.11 MiB	2.37 MiB
`4ac2cff`	17.75 MiB	20.11 MiB	2.37 MiB
`6a80f44`	17.75 MiB	20.11 MiB	2.37 MiB

…sdk-init

…ertToWritable

krystofwoldrich · 2025-01-22T14:49:16Z

packages/core/android/src/main/java/io/sentry/react/RNSentryMapConverter.java

+  }
+
+  public static class WritableJavaArrayCreator implements WritableArrayCreator {
+    @Override


Is the annotation needed? We don't override already implemented method here.

Technically it is not needed but it is usually recommended (example style guide) to ensure that the method correctly matches the method signature in the interface.

krystofwoldrich · 2025-01-22T14:57:07Z

packages/core/android/src/main/java/io/sentry/react/RNSentryMapConverter.java

-      keysAndValues[index++] = entry.getValue();
-    }
-    return JavaOnlyMap.of(keysAndValues);
+    return (WritableMap) convertToJavaWritable(map);


I see that base on the current code the return value can't be null, but rather than forcing the compiler to the non-null value, let's check the output of convertToJavaWritable and if null we can return new empty JavaReadableMap.

krystofwoldrich · 2025-01-22T15:02:02Z

I realized we haven't done it in the past, but we should mark all the publics with @internal

antonis · 2025-01-23T08:18:55Z

I realized we haven't done it in the past, but we should mark all the publics with @internal

@krystofwoldrich do you mean a javadoc tag or a custom annotation?
I will also make sure to lower the access level of classes/methods were this is possible.

krystofwoldrich · 2025-01-23T08:41:51Z

I meant access level if possible if not annotation.

antonis · 2025-01-23T14:27:03Z

Closing in favour of a simpler implementation in #4479

antonis added 5 commits January 21, 2025 10:15

Refactor convertToWritable to allow other array and map creators

35a878f

Add convertToJavaWritable method that uses JavaOnlyMap and JavaOnlyArray

8140490

Remove custom mapToReadableMap implementation

e84a97d

Rename native implementation tests for clarity

7cb8507

Add java implementation tests

04adaf2

antonis changed the title ~~Ref: Refactors RNSentryMapConverter.convertToWritable and adds a Java only implementation~~ Ref: Adds a Java only implementation in RNSentryMapConverter.convertToWritable Jan 21, 2025

antonis mentioned this pull request Jan 21, 2025

feat(experimental): Initialize Android SDK from json configuration #4451

Merged

10 tasks

antonis marked this pull request as ready for review January 21, 2025 09:31

antonis requested review from krystofwoldrich and lucas-zimerman as code owners January 21, 2025 09:31

This was referenced Jan 21, 2025

ref(android): Extracts Android native initialization to standalone structures #4445

Merged

Ref: Handle nested json configuration structures #4470

Closed

antonis and others added 5 commits January 21, 2025 13:02

Merge branch 'antonis/init-from-json' into antonis/ref-convertToWritable

ae514a2

Adds java doc and null annotations

0aacc17

Merge branch 'capture-app-start-errors' into antonis/extract-android-…

87b5914

…sdk-init

Update CHANGELOG.md

f35b764

Merge branch 'antonis/extract-android-sdk-init' into antonis/ref-conv…

cc8183b

…ertToWritable

antonis mentioned this pull request Jan 21, 2025

Fail gracefully when the provided json is not valid #4474

Closed

10 tasks

antonis added 2 commits January 22, 2025 09:51

Merge branch 'antonis/init-from-json' into antonis/ref-convertToWritable

60a0d5d

Merge branch 'antonis/init-from-json' into antonis/ref-convertToWritable

290a3ae

krystofwoldrich reviewed Jan 22, 2025

View reviewed changes

krystofwoldrich mentioned this pull request Jan 23, 2025

Adds utility class for converting JsonObject to WritableMap #4479

Merged

10 tasks

antonis closed this Jan 23, 2025

Uh oh!

Ref: Adds a Java only implementation in RNSentryMapConverter.convertToWritable #4469

Ref: Adds a Java only implementation in RNSentryMapConverter.convertToWritable #4469

Uh oh!

Conversation

antonis commented Jan 21, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

📢 Type of change

📜 Description

⛓️ PR Chain

💡 Motivation and Context

💚 How did you test it?

📝 Checklist

🔮 Next steps

Uh oh!

github-actions bot commented Jan 21, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Android (new) Performance metrics 🚀

Baseline results on branch: antonis/init-from-json

Startup times

App size

Previous results on branch: antonis/ref-convertToWritable

Startup times

App size

Uh oh!

github-actions bot commented Jan 21, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

iOS (legacy) Performance metrics 🚀

Baseline results on branch: antonis/init-from-json

Startup times

App size

Previous results on branch: antonis/ref-convertToWritable

Startup times

App size

Uh oh!

github-actions bot commented Jan 21, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

iOS (new) Performance metrics 🚀

Baseline results on branch: antonis/init-from-json

Startup times

App size

Previous results on branch: antonis/ref-convertToWritable

Startup times

App size

Uh oh!

github-actions bot commented Jan 21, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Android (legacy) Performance metrics 🚀

Baseline results on branch: antonis/init-from-json

Startup times

App size

Previous results on branch: antonis/ref-convertToWritable

Startup times

App size

Uh oh!

krystofwoldrich Jan 22, 2025

Choose a reason for hiding this comment

Uh oh!

antonis Jan 22, 2025

Choose a reason for hiding this comment

Uh oh!

krystofwoldrich Jan 22, 2025

Choose a reason for hiding this comment

Uh oh!

krystofwoldrich commented Jan 22, 2025

Uh oh!

antonis commented Jan 23, 2025

Uh oh!

krystofwoldrich commented Jan 23, 2025

Uh oh!

antonis commented Jan 23, 2025

Uh oh!

Uh oh!

antonis commented Jan 21, 2025 •

edited

Loading

github-actions bot commented Jan 21, 2025 •

edited

Loading

github-actions bot commented Jan 21, 2025 •

edited

Loading

github-actions bot commented Jan 21, 2025 •

edited

Loading

github-actions bot commented Jan 21, 2025 •

edited

Loading