From 4bfa34281efa8a6bf4ba5e452c2923f9e11ec420 Mon Sep 17 00:00:00 2001 From: tobrun Date: Wed, 11 Jul 2018 16:28:47 +0200 Subject: [android] - cleanup ExecutorService with activity LifeCycle event onStop(), allow setTileData to be called fron any thread --- .../style/sources/CustomGeometrySource.java | 90 +++++++++++++++------- .../testapp/activity/style/GridSourceActivity.java | 26 +++---- 2 files changed, 71 insertions(+), 45 deletions(-) diff --git a/platform/android/MapboxGLAndroidSDK/src/main/java/com/mapbox/mapboxsdk/style/sources/CustomGeometrySource.java b/platform/android/MapboxGLAndroidSDK/src/main/java/com/mapbox/mapboxsdk/style/sources/CustomGeometrySource.java index 469bfa8f39..4522f118a8 100644 --- a/platform/android/MapboxGLAndroidSDK/src/main/java/com/mapbox/mapboxsdk/style/sources/CustomGeometrySource.java +++ b/platform/android/MapboxGLAndroidSDK/src/main/java/com/mapbox/mapboxsdk/style/sources/CustomGeometrySource.java @@ -1,10 +1,10 @@ package com.mapbox.mapboxsdk.style.sources; +import android.support.annotation.AnyThread; import android.support.annotation.NonNull; import android.support.annotation.Nullable; import android.support.annotation.UiThread; import android.support.annotation.WorkerThread; - import com.mapbox.geojson.Feature; import com.mapbox.geojson.FeatureCollection; import com.mapbox.mapboxsdk.geometry.LatLngBounds; @@ -18,22 +18,25 @@ import java.util.Map; import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.ExecutorService; import java.util.concurrent.Executors; +import java.util.concurrent.ThreadFactory; import java.util.concurrent.atomic.AtomicBoolean; +import java.util.concurrent.atomic.AtomicInteger; /** * Custom Vector Source, allows using FeatureCollections. - * */ @UiThread public class CustomGeometrySource extends Source { - private ExecutorService executor; - private GeometryTileProvider provider; + + private static final AtomicInteger poolCount = new AtomicInteger(); private final Map cancelledTileRequests = new ConcurrentHashMap<>(); + private ExecutorService executorService; + private GeometryTileProvider provider; /** * Create a CustomGeometrySource * - * @param id The source id. + * @param id The source id. * @param provider The tile provider that returns geometry data for this source. */ public CustomGeometrySource(String id, GeometryTileProvider provider) { @@ -42,28 +45,28 @@ public class CustomGeometrySource extends Source { /** * Create a CustomGeometrySource with non-default CustomGeometrySourceOptions. - *

Supported options are minZoom, maxZoom, buffer, and tolerance.

+ *

+ * Supported options are minZoom, maxZoom, buffer, and tolerance. + *

* - * @param id The source id. + * @param id The source id. * @param provider The tile provider that returns geometry data for this source. - * @param options CustomGeometrySourceOptions. + * @param options CustomGeometrySourceOptions. */ public CustomGeometrySource(String id, GeometryTileProvider provider, CustomGeometrySourceOptions options) { super(); this.provider = provider; - executor = Executors.newFixedThreadPool(4); initialize(id, options); } /** - * Invalidate previously provided features within a given bounds at all zoom levels. - * Invoking this method will result in new requests to `GeometryTileProvider` for regions - * that contain, include, or intersect with the provided bounds. + * Invalidate previously provided features within a given bounds at all zoom levels. + * Invoking this method will result in new requests to `GeometryTileProvider` for regions + * that contain, include, or intersect with the provided bounds. * * @param bounds The region in which features should be invalidated at all zoom levels */ public void invalidateRegion(LatLngBounds bounds) { - checkThread(); nativeInvalidateBounds(bounds); } @@ -72,11 +75,10 @@ public class CustomGeometrySource extends Source { * in new requests to `GeometryTileProvider` for visible tiles. * * @param zoomLevel Tile zoom level. - * @param x Tile X coordinate. - * @param y Tile Y coordinate. + * @param x Tile X coordinate. + * @param y Tile Y coordinate. */ public void invalidateTile(int zoomLevel, int x, int y) { - checkThread(); nativeInvalidateTile(zoomLevel, x, y); } @@ -86,12 +88,12 @@ public class CustomGeometrySource extends Source { * background threads. * * @param zoomLevel Tile zoom level. - * @param x Tile X coordinate. - * @param y Tile Y coordinate. - * @param data Feature collection for the tile. + * @param x Tile X coordinate. + * @param y Tile Y coordinate. + * @param data Feature collection for the tile. */ + @AnyThread public void setTileData(int zoomLevel, int x, int y, FeatureCollection data) { - checkThread(); nativeSetTileData(zoomLevel, x, y, data); } @@ -103,15 +105,15 @@ public class CustomGeometrySource extends Source { */ @NonNull public List querySourceFeatures(@Nullable Expression filter) { - checkThread(); Feature[] features = querySourceFeatures(filter != null ? filter.toArray() : null); - return features != null ? Arrays.asList(features) : new ArrayList(); + return features != null ? Arrays.asList(features) : new ArrayList<>(); } protected native void initialize(String sourceId, Object options); private native Feature[] querySourceFeatures(Object[] filter); + @AnyThread private native void nativeSetTileData(int z, int x, int y, FeatureCollection data); private native void nativeInvalidateTile(int z, int x, int y); @@ -132,7 +134,7 @@ public class CustomGeometrySource extends Source { TileID tileID = new TileID(z, x, y); cancelledTileRequests.put(tileID, cancelFlag); GeometryTileRequest request = new GeometryTileRequest(tileID, provider, this, cancelFlag); - executor.execute(request); + obtainExecutorService().execute(request); } @WorkerThread @@ -143,19 +145,49 @@ public class CustomGeometrySource extends Source { } } + @AnyThread + private ExecutorService obtainExecutorService() { + if (executorService == null || executorService.isShutdown()) { + createExecutorService(); + } + return executorService; + } + + @AnyThread + private void createExecutorService() { + executorService = Executors.newFixedThreadPool(4, new ThreadFactory() { + final AtomicInteger threadCount = new AtomicInteger(); + final int poolId = poolCount.incrementAndGet(); + + @Override + public Thread newThread(@NonNull Runnable r) { + return new Thread(r, "CustomGeom-" + poolId + "-" + threadCount.incrementAndGet()); + } + }); + } + + /** + * Clear out the used ExecutorService and remove the sleeping threads. + */ + public void onStop() { + if (executorService != null) { + executorService.shutdownNow(); + } + } + private static class TileID { public int z; public int x; public int y; - public TileID(int _z, int _x, int _y) { + TileID(int _z, int _x, int _y) { z = _z; x = _x; y = _y; } public int hashCode() { - return Arrays.hashCode(new int[]{z, x, y}); + return Arrays.hashCode(new int[] {z, x, y}); } public boolean equals(Object object) { @@ -168,7 +200,7 @@ public class CustomGeometrySource extends Source { } if (object instanceof TileID) { - TileID other = (TileID)object; + TileID other = (TileID) object; return this.z == other.z && this.x == other.x && this.y == other.y; } return false; @@ -181,8 +213,8 @@ public class CustomGeometrySource extends Source { private WeakReference sourceRef; private AtomicBoolean cancelled; - public GeometryTileRequest(TileID _id, GeometryTileProvider p, - CustomGeometrySource _source, AtomicBoolean _cancelled) { + GeometryTileRequest(TileID _id, GeometryTileProvider p, + CustomGeometrySource _source, AtomicBoolean _cancelled) { id = _id; provider = p; sourceRef = new WeakReference<>(_source); @@ -196,7 +228,7 @@ public class CustomGeometrySource extends Source { FeatureCollection data = provider.getFeaturesForBounds(LatLngBounds.from(id.z, id.x, id.y), id.z); CustomGeometrySource source = sourceRef.get(); - if (!isCancelled() && source != null && data != null) { + if (!isCancelled() && source != null && data != null) { source.setTileData(id, data); } } diff --git a/platform/android/MapboxGLAndroidSDKTestApp/src/main/java/com/mapbox/mapboxsdk/testapp/activity/style/GridSourceActivity.java b/platform/android/MapboxGLAndroidSDKTestApp/src/main/java/com/mapbox/mapboxsdk/testapp/activity/style/GridSourceActivity.java index fdc3826fb1..fb2f4185e4 100644 --- a/platform/android/MapboxGLAndroidSDKTestApp/src/main/java/com/mapbox/mapboxsdk/testapp/activity/style/GridSourceActivity.java +++ b/platform/android/MapboxGLAndroidSDKTestApp/src/main/java/com/mapbox/mapboxsdk/testapp/activity/style/GridSourceActivity.java @@ -4,7 +4,6 @@ import android.graphics.Color; import android.os.Bundle; import android.support.annotation.NonNull; import android.support.v7.app.AppCompatActivity; - import com.mapbox.geojson.Feature; import com.mapbox.geojson.FeatureCollection; import com.mapbox.geojson.MultiLineString; @@ -18,7 +17,6 @@ import com.mapbox.mapboxsdk.style.sources.CustomGeometrySource; import com.mapbox.mapboxsdk.style.sources.GeometryTileProvider; import com.mapbox.mapboxsdk.testapp.R; - import java.util.ArrayList; import java.util.Arrays; import java.util.List; @@ -34,7 +32,7 @@ public class GridSourceActivity extends AppCompatActivity implements OnMapReadyC private static final String ID_GRID_LAYER = "grid_layer"; private MapView mapView; - private MapboxMap mapboxMap; + private CustomGeometrySource source; /** * Implementation of GeometryTileProvider that returns features representing a zoom-dependent @@ -66,7 +64,7 @@ public class GridSourceActivity extends AppCompatActivity implements OnMapReadyC gridSpacing = 20; } - List gridLines = new ArrayList(); + List> gridLines = new ArrayList<>(); for (double y = Math.ceil(bounds.getLatNorth() / gridSpacing) * gridSpacing; y >= Math.floor(bounds.getLatSouth() / gridSpacing) * gridSpacing; y -= gridSpacing) { gridLines.add(Arrays.asList(Point.fromLngLat(bounds.getLonWest(), y), @@ -74,7 +72,7 @@ public class GridSourceActivity extends AppCompatActivity implements OnMapReadyC } features.add(Feature.fromGeometry(MultiLineString.fromLngLats(gridLines))); - gridLines = new ArrayList(); + gridLines = new ArrayList<>(); for (double x = Math.floor(bounds.getLonWest() / gridSpacing) * gridSpacing; x <= Math.ceil(bounds.getLonEast() / gridSpacing) * gridSpacing; x += gridSpacing) { gridLines.add(Arrays.asList(Point.fromLngLat(x, bounds.getLatSouth()), @@ -91,26 +89,20 @@ public class GridSourceActivity extends AppCompatActivity implements OnMapReadyC super.onCreate(savedInstanceState); setContentView(R.layout.activity_grid_source); - mapView = (MapView) findViewById(R.id.mapView); + mapView = findViewById(R.id.mapView); mapView.onCreate(savedInstanceState); mapView.getMapAsync(this); } @Override public void onMapReady(@NonNull final MapboxMap map) { - mapboxMap = map; - // add source - CustomGeometrySource source = new CustomGeometrySource(ID_GRID_SOURCE, new GridProvider()); - mapboxMap.addSource(source); + map.addSource(source = new CustomGeometrySource(ID_GRID_SOURCE, new GridProvider())); // add layer - LineLayer layer = new LineLayer(ID_GRID_LAYER, ID_GRID_SOURCE); - layer.setProperties( - lineColor(Color.parseColor("#000000")) + map.addLayer(new LineLayer(ID_GRID_LAYER, ID_GRID_SOURCE) + .withProperties(lineColor(Color.parseColor("#000000"))) ); - - mapboxMap.addLayer(layer); } @Override @@ -134,6 +126,9 @@ public class GridSourceActivity extends AppCompatActivity implements OnMapReadyC @Override protected void onStop() { super.onStop(); + if (source != null) { + source.onStop(); + } mapView.onStop(); } @@ -148,5 +143,4 @@ public class GridSourceActivity extends AppCompatActivity implements OnMapReadyC super.onSaveInstanceState(outState); mapView.onSaveInstanceState(outState); } - } -- cgit v1.2.1