Skip to content

Add memory efficient polyline decoder #1518

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

Closed
wants to merge 2 commits into from
Closed

Conversation

kmadsen
Copy link
Contributor

@kmadsen kmadsen commented Nov 21, 2022

I'm making a more memory efficient replayer mapbox/mapbox-navigation-android#6636

The first large memory task is to decode a geometry string. This pull requests makes it possible to pass in a string as an InputStream, so that it can come from a file. It will then decode the polyline upon request.

It is true that the geometry string is inside a DirectionsResponse so it is already memory inefficient. The reason I'm going ahead with it now is because it simpler for streaming objects to consume downstream.

@kmadsen kmadsen requested a review from a team as a code owner November 21, 2022 22:23
@kmadsen kmadsen force-pushed the km-add-polyline-decoder branch from 9b67c33 to 35f667b Compare November 21, 2022 22:30
@codecov
Copy link

codecov bot commented Nov 21, 2022

Codecov Report

Attention: Patch coverage is 91.11111% with 4 lines in your changes missing coverage. Please review.

Project coverage is 76.97%. Comparing base (6b63f16) to head (7a6c048).
Report is 50 commits behind head on main.

Files with missing lines Patch % Lines
...java/com/mapbox/geojson/utils/PolylineDecoder.java 90.24% 4 Missing ⚠️

Impacted file tree graph

@@             Coverage Diff              @@
##               main    #1518      +/-   ##
============================================
+ Coverage     76.94%   76.97%   +0.03%     
- Complexity      938      946       +8     
============================================
  Files           129      130       +1     
  Lines          4025     4048      +23     
  Branches        582      582              
============================================
+ Hits           3097     3116      +19     
- Misses          678      682       +4     
  Partials        250      250              
Files with missing lines Coverage Δ
...n/java/com/mapbox/geojson/utils/PolylineUtils.java 73.56% <100.00%> (-4.54%) ⬇️
...java/com/mapbox/geojson/utils/PolylineDecoder.java 90.24% <90.24%> (ø)
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@kmadsen kmadsen force-pushed the km-add-polyline-decoder branch from 35f667b to 4576fb4 Compare November 23, 2022 00:32
* @param inputStream InputStream that reads a String as bytes
* @param precision OSRMv4 uses 6, OSRMv5 and Google uses 5
*/
public PolylineDecoder(InputStream inputStream, int precision) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
public PolylineDecoder(InputStream inputStream, int precision) {
public PolylineDecoder(@NonNull InputStream inputStream, int precision) {

Comment on lines +67 to +68
@Override
public Point next() throws NoSuchElementException {
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
@Override
public Point next() throws NoSuchElementException {
@Override
@NonNull
public Point next() throws NoSuchElementException {

I don't remember if you can change the annotation of an overridden method though 🙃

@Zayankovsky Zayankovsky deleted the km-add-polyline-decoder branch May 19, 2025 11:51
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.

4 participants