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

Optimize code and string allocations #55

Merged
merged 10 commits into from
Mar 1, 2024

Conversation

rubensousa
Copy link
Collaborator

@rubensousa rubensousa commented Feb 29, 2024

  • Used StringBuilder to construct json strings instead of keeping N copies of the entire string in memory. This makes JsonObjectWrapper and PayloadObject a lot faster and use less memory
  • Load JS code from assets instead of using static strings in memory

Benchmark test on my Pixel 6:

JsonObjectWrapper.collectionToJsonString, 1000 elements:

Before: 2964 ms, After: 106 ms

JsonObjectWrapper.mapToJsonString, 1000 properties:

Before: 1014 ms, After: 55 ms

@TheRishka
Copy link
Contributor

@bwalter @wkarl we would need some support here :)

@bwalter089
Copy link

The re-format (which makes sense) and the move to truth (I am not sure about the benefit here ;)) make it a little bit hard to review the real changes but the optimizations are very welcome additions, thanks a lot for your work! Looks good to me,

Copy link
Contributor

@bwalter bwalter left a comment

Choose a reason for hiding this comment

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

Thanks for the good change!

@rubensousa
Copy link
Collaborator Author

The re-format (which makes sense) and the move to truth (I am not sure about the benefit here ;)) make it a little bit hard to review the real changes but the optimizations are very welcome additions, thanks a lot for your work! Looks good to me,

I added Truth because the expected and actual were reversed from the junit assertions, which made it harder for me to analyse why the tests were not green in the beginning

@bwalter
Copy link
Contributor

bwalter commented Feb 29, 2024

The re-format (which makes sense) and the move to truth (I am not sure about the benefit here ;)) make it a little bit hard to review the real changes but the optimizations are very welcome additions, thanks a lot for your work! Looks good to me,

I added Truth because the expected and actual were reversed from the junit assertions, which made it harder for me to analyse why the tests were not green in the beginning

Fair point. Rust std lib uses left/right instead of expected/actual to avoid the issues ;)

@rubensousa rubensousa merged commit b23f072 into p7s1digital:master Mar 1, 2024
5 checks passed
Eridana pushed a commit that referenced this pull request Mar 1, 2024
Optimize code and string allocations
# Conflicts:
#	jsbridge/src/main/kotlin/de/prosiebensat1digital/oasisjsbridge/JsBridge.kt
#	jsbridge/src/main/kotlin/de/prosiebensat1digital/oasisjsbridge/extensions/PromiseExtension.kt
#	jsbridge/src/main/kotlin/de/prosiebensat1digital/oasisjsbridge/extensions/XMLHttpRequestExtension.kt
rubensousa added a commit to rubensousa/oasis-jsbridge-android that referenced this pull request Mar 1, 2024
…locations

Optimize code and string allocations
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.

6 participants