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

Keep sources in memory #928

Closed
tobrun opened this issue May 9, 2018 · 7 comments
Closed

Keep sources in memory #928

tobrun opened this issue May 9, 2018 · 7 comments

Comments

@tobrun
Copy link
Member

tobrun commented May 9, 2018

When looking into mapbox/mapbox-gl-native#11843. I'm noticing the following in MapUtils:

  public static void updateMapSourceFromFeatureCollection(@NonNull MapboxMap mapboxMap,
                                                          @Nullable FeatureCollection collection,
                                                          @NonNull String sourceId) {
    if (collection == null) {
      collection = FeatureCollection.fromFeatures(new Feature[] {});
    }

    GeoJsonSource source = mapboxMap.getSourceAs(sourceId);
    if (source == null) {
      GeoJsonOptions routeGeoJsonOptions = new GeoJsonOptions().withMaxZoom(16);
      GeoJsonSource routeSource = new GeoJsonSource(sourceId, collection, routeGeoJsonOptions);
      mapboxMap.addSource(routeSource);
    } else {
      source.setGeoJson(collection);
    }
  }

With every update we are getting the source from MapboxMap, this is an expensive task which requires to go through JNI to C++ an back. Instead of creating new instances with getSourceAs we should cache the value of the source.

@tobrun tobrun added the bug Defect to be fixed. label May 9, 2018
@danesfeder
Copy link
Contributor

danesfeder commented May 14, 2018

@Guardiola31337 maybe this can be part of refactoring NavigationMapRoute? The better approach @tobrun is explaining here is the same as the new LLP code that I was showing you.

@tobrun
Copy link
Member Author

tobrun commented May 15, 2018

The idea here is to keep sources/layers in memory and reload them when a new style has been loaded (map change event).

@rishabhsri20
Copy link

Any solution for this freeze? Happens with me when starting a navigation.

@danesfeder
Copy link
Contributor

Hey @rishabhsri20, this ticket is "next up" for our pipeline of work - stay tuned for updates as we update this ticket once work begins. Thank you for your patience!

@danesfeder
Copy link
Contributor

danesfeder commented Jan 14, 2019

@LukasPaczos I recall having a brief discussion about this during #1387 - we would get some performance wins by not using the MapUtil as Tobrun mentioned?

@tobrun
Copy link
Member Author

tobrun commented Jan 14, 2019

@danesfeder since v7.0.0 this is optimised under the hood, if you can Style#getSource(id) it will return a cached source if available (same goes for layers).

  @Nullable
  public Source getSource(String id) {
    validateState("getSource");
    Source source = sources.get(id);
    if (source == null) {
      source = nativeMapView.getSource(id);
    }
    return source;
  }

Don't think this issue is actionable anymore.

@danesfeder
Copy link
Contributor

Awesome thanks for the clarification here @tobrun, closing with #1615

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

No branches or pull requests

4 participants