-
Notifications
You must be signed in to change notification settings - Fork 120
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
Low level GeoJson implementation #710
Comments
Thank you @tobrun for framing the issue and proposing some solutions. By looking at your list, looks like at list the following items would change the public API of the library:
Am I reading it right? If so, we need to 1) either wait until the next MAS SEMVER release (4.x) or 2) develop this new library as a new product with a different namespace. |
Notes in OP were mostly about the internal management so probably we can keep the same public api though this is something that needs to be determined when scoping out. As first steps I would recommend writing up a test suite to tracks memory allocation / cpu usage for 1 feature up to a million features and compare that with the 2.2.9 version. After, we can scoop out what the impact would be to bring the notes from OP into play and use that same test suite as reference during implementation. |
I did a bit of testing on how much memory Point takes in 2.x.x based code vs 3.0 (with AutoValue).
|
Also tried comparing loading of sample json with FeatureCollection: { Here is Heap dump for 2.0 and 3.0 based mapbox-java |
@osana Thanks for kicking off this work - what's your take on the results in #710 (comment) and #710 (comment) and what are the next steps? |
Is this still in progress? |
@zugaldia @tobrun I think the next steps could be as @tobrun points out
|
Thank you @osana. Let's start by having an implementation that doesn't rely on AutoValue to avoid the overhead. This could be based of the MAS v2.x code to speed things up. If SEMVER is a concern, let's build the prototype under a new namespace (e.g. |
@zugaldia good idea to look at v2.x code and I agree that having two packages next to each other is a good way to compare. |
Capturing from @Mordag in mapbox/mapbox-gl-native#14555 to look into replacing GSON with Jackson as parser which makes a lot of sense as our data model doesn't change (the geojson RFC makes sure of that). |
Android JSON Parsers Comparison: Looks like |
This issue has been automatically detected as stale because it has not had recent activity and will be archived. Thank you for your contributions. |
1 similar comment
This issue has been automatically detected as stale because it has not had recent activity and will be archived. Thank you for your contributions. |
This is a request for either a rewrite or alternative implementation of the current GeoJson module found here. We need a high performance implementation that has a minimal memory footprint. The reasoning behind that is that geojson models should be scalable (eg. downstream we query the map for 1000 of features, the path currently taken to create those and the intermediate objects created for doing that isn't optimal atm).
Couple of thoughts to improve this:
More information on performance tips here
RFC of GeoJSON can be found here.
cc @mapbox/android
The text was updated successfully, but these errors were encountered: