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

Integrate value marshalling performance improvement #606

Merged
merged 2 commits into from
Sep 3, 2021

Conversation

Chaoba
Copy link
Contributor

@Chaoba Chaoba commented Sep 2, 2021

PRs must be submitted under the terms of our Contributor License Agreement CLA.
Fixes: < Link to related issues that will be fixed by this pull request, if they exist >

Pull request checklist:

  • Briefly describe the changes in this PR.
  • Include before/after visuals or gifs if this PR includes visual changes.
  • Write tests for all new functionality. If tests were not written, please explain why.
  • Add example if relevant.
  • Document any changes to public APIs.
  • Apply changelog label ('breaking change', 'bug 🪲', 'build', 'docs', 'feature 🍏', 'performance ⚡', 'testing 💯') or use the label 'skip changelog'
  • Add an entry inside this element for inclusion in the mapbox-maps-android changelog: <changelog>Integrate value marshalling performance improvement</changelog>.

Summary of changes

Use fromJson method in Value directly instead of using ValueConverter.fromJson() to improve performance.
Try to load long_route.json on XiaoMi 8

Method Time(nanoseconds)
ValueConverter.fromJson 2945885468
Value.fromJson 873840938

User impact (optional)

@Chaoba Chaoba self-assigned this Sep 2, 2021
@Chaoba Chaoba requested a review from a team September 2, 2021 08:03
@kiryldz
Copy link
Contributor

kiryldz commented Sep 2, 2021

@Chaoba is it possible already to measure time before and after this improvement in our examples?
Or does it help only if working with large json's?

@pengdev
Copy link
Member

pengdev commented Sep 2, 2021

Great cleanup 👍 do we have a benchmark that we can see the performance improvements?

@Chaoba
Copy link
Contributor Author

Chaoba commented Sep 2, 2021

Updated the load time with two different methods in pr description.

@tobrun
Copy link
Member

tobrun commented Sep 2, 2021

Updated the load time with two different methods in pr description.

Those numbers look great!

@Chaoba Chaoba merged commit d9cb73a into main Sep 3, 2021
@Chaoba Chaoba deleted the kl-value-marshaling branch September 3, 2021 02:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants