Skip to content

Commit

Permalink
Improve cancellation of tile loads (#8633)
Browse files Browse the repository at this point in the history
* use Actor objects directly rather than associating it with a workerID

* make web worker requests cancelable, and cancel loadTile messages when abortTile is called

* determine whether sources are loaded in the source cache
  • Loading branch information
kkaefer authored and mourner committed Aug 21, 2019
1 parent 9ac2b38 commit 5f8f1c2
Show file tree
Hide file tree
Showing 19 changed files with 290 additions and 139 deletions.
1 change: 1 addition & 0 deletions src/source/canvas_source.js
Original file line number Diff line number Diff line change
Expand Up @@ -111,6 +111,7 @@ class CanvasSource extends ImageSource {
*/

load() {
this._loaded = true;
if (!this.canvas) {
this.canvas = (this.options.canvas instanceof window.HTMLCanvasElement) ?
this.options.canvas :
Expand Down
41 changes: 26 additions & 15 deletions src/source/geojson_source.js
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import type {Source} from './source';
import type Map from '../ui/map';
import type Dispatcher from '../util/dispatcher';
import type Tile from './tile';
import type Actor from '../util/actor';
import type {Callback} from '../types/callback';
import type {GeoJSON, GeoJSONFeature} from '@mapbox/geojson-types';
import type {GeoJSONSourceSpecification} from '../style-spec/types';
Expand Down Expand Up @@ -74,9 +75,8 @@ class GeoJSONSource extends Evented implements Source {
_data: GeoJSON | string;
_options: any;
workerOptions: any;
dispatcher: Dispatcher;
map: Map;
workerID: number;
actor: Actor;
_loaded: boolean;
_collectResourceTiming: boolean;
_resourceTiming: Array<PerformanceResourceTiming>;
Expand All @@ -100,8 +100,9 @@ class GeoJSONSource extends Evented implements Source {
this.isTileClipped = true;
this.reparseOverscaled = true;
this._removed = false;
this._loaded = false;

this.dispatcher = dispatcher;
this.actor = dispatcher.getActor();
this.setEventedParent(eventedParent);

this._data = (options.data: any);
Expand Down Expand Up @@ -202,7 +203,7 @@ class GeoJSONSource extends Evented implements Source {
* @returns {GeoJSONSource} this
*/
getClusterExpansionZoom(clusterId: number, callback: Callback<number>) {
this.dispatcher.send('geojson.getClusterExpansionZoom', { clusterId, source: this.id }, callback, this.workerID);
this.actor.send('geojson.getClusterExpansionZoom', { clusterId, source: this.id }, callback);
return this;
}

Expand All @@ -214,7 +215,7 @@ class GeoJSONSource extends Evented implements Source {
* @returns {GeoJSONSource} this
*/
getClusterChildren(clusterId: number, callback: Callback<Array<GeoJSONFeature>>) {
this.dispatcher.send('geojson.getClusterChildren', { clusterId, source: this.id }, callback, this.workerID);
this.actor.send('geojson.getClusterChildren', { clusterId, source: this.id }, callback);
return this;
}

Expand All @@ -228,12 +229,12 @@ class GeoJSONSource extends Evented implements Source {
* @returns {GeoJSONSource} this
*/
getClusterLeaves(clusterId: number, limit: number, offset: number, callback: Callback<Array<GeoJSONFeature>>) {
this.dispatcher.send('geojson.getClusterLeaves', {
this.actor.send('geojson.getClusterLeaves', {
source: this.id,
clusterId,
limit,
offset
}, callback, this.workerID);
}, callback);
return this;
}

Expand All @@ -243,6 +244,7 @@ class GeoJSONSource extends Evented implements Source {
* using geojson-vt or supercluster as appropriate.
*/
_updateWorkerData(callback: Callback<void>) {
this._loaded = false;
const options = extend({}, this.workerOptions);
const data = this._data;
if (typeof data === 'string') {
Expand All @@ -255,7 +257,7 @@ class GeoJSONSource extends Evented implements Source {
// target {this.type}.loadData rather than literally geojson.loadData,
// so that other geojson-like source types can easily reuse this
// implementation
this.workerID = this.dispatcher.send(`${this.type}.loadData`, options, (err, result) => {
this.actor.send(`${this.type}.loadData`, options, (err, result) => {
if (this._removed || (result && result.abandoned)) {
return;
}
Expand All @@ -271,14 +273,18 @@ class GeoJSONSource extends Evented implements Source {
// message queue. Waiting instead for the 'coalesce' to round-trip
// through the foreground just means we're throttling the worker
// to run at a little less than full-throttle.
this.dispatcher.send(`${this.type}.coalesce`, { source: options.source }, null, this.workerID);
this.actor.send(`${this.type}.coalesce`, { source: options.source }, null);
callback(err);
});
}

}, this.workerID);
loaded(): boolean {
return this._loaded;
}

loadTile(tile: Tile, callback: Callback<void>) {
const message = tile.workerID === undefined ? 'loadTile' : 'reloadTile';
const message = !tile.actor ? 'loadTile' : 'reloadTile';
tile.actor = this.actor;
const params = {
type: this.type,
uid: tile.uid,
Expand All @@ -291,7 +297,8 @@ class GeoJSONSource extends Evented implements Source {
showCollisionBoxes: this.map.showCollisionBoxes
};

tile.workerID = this.dispatcher.send(message, params, (err, data) => {
tile.request = this.actor.send(message, params, (err, data) => {
delete tile.request;
tile.unloadVectorData();

if (tile.aborted) {
Expand All @@ -305,21 +312,25 @@ class GeoJSONSource extends Evented implements Source {
tile.loadVectorData(data, this.map.painter, message === 'reloadTile');

return callback(null);
}, this.workerID);
});
}

abortTile(tile: Tile) {
if (tile.request) {
tile.request.cancel();
delete tile.request;
}
tile.aborted = true;
}

unloadTile(tile: Tile) {
tile.unloadVectorData();
this.dispatcher.send('removeTile', { uid: tile.uid, type: this.type, source: this.id }, null, tile.workerID);
this.actor.send('removeTile', { uid: tile.uid, type: this.type, source: this.id });
}

onRemove() {
this._removed = true;
this.dispatcher.send('removeSource', { type: this.type, source: this.id }, null, this.workerID);
this.actor.send('removeSource', { type: this.type, source: this.id });
}

serialize() {
Expand Down
8 changes: 8 additions & 0 deletions src/source/image_source.js
Original file line number Diff line number Diff line change
Expand Up @@ -83,6 +83,7 @@ class ImageSource extends Evented implements Source {
_boundsArray: RasterBoundsArray;
boundsBuffer: VertexBuffer;
boundsSegments: SegmentVector;
_loaded: boolean;

/**
* @private
Expand All @@ -98,18 +99,21 @@ class ImageSource extends Evented implements Source {
this.maxzoom = 22;
this.tileSize = 512;
this.tiles = {};
this._loaded = false;

this.setEventedParent(eventedParent);

this.options = options;
}

load(newCoordinates?: Coordinates, successCallback?: () => void) {
this._loaded = false;
this.fire(new Event('dataloading', {dataType: 'source'}));

this.url = this.options.url;

getImage(this.map._requestManager.transformRequest(this.url, ResourceType.Image), (err, image) => {
this._loaded = true;
if (err) {
this.fire(new ErrorEvent(err));
} else if (image) {
Expand All @@ -125,6 +129,10 @@ class ImageSource extends Evented implements Source {
});
}

loaded(): boolean {
return this._loaded;
}

/**
* Updates the image URL and, optionally, the coordinates. To avoid having the image flash after changing,
* set the `raster-fade-duration` paint property on the raster layer to 0.
Expand Down
9 changes: 6 additions & 3 deletions src/source/raster_dem_tile_source.js
Original file line number Diff line number Diff line change
Expand Up @@ -64,8 +64,9 @@ class RasterDEMTileSource extends RasterTileSource implements Source {
encoding: this.encoding
};

if (!tile.workerID || tile.state === 'expired') {
tile.workerID = this.dispatcher.send('loadDEMTile', params, done.bind(this));
if (!tile.actor || tile.state === 'expired') {
tile.actor = this.dispatcher.getActor();
tile.actor.send('loadDEMTile', params, done.bind(this));
}
}
}
Expand Down Expand Up @@ -125,7 +126,9 @@ class RasterDEMTileSource extends RasterTileSource implements Source {
delete tile.neighboringTiles;

tile.state = 'unloaded';
this.dispatcher.send('removeDEMTile', { uid: tile.uid, source: this.id }, undefined, tile.workerID);
if (tile.actor) {
tile.actor.send('removeDEMTile', { uid: tile.uid, source: this.id });
}
}

}
Expand Down
6 changes: 6 additions & 0 deletions src/source/raster_tile_source.js
Original file line number Diff line number Diff line change
Expand Up @@ -62,9 +62,11 @@ class RasterTileSource extends Evented implements Source {
}

load() {
this._loaded = false;
this.fire(new Event('dataloading', {dataType: 'source'}));
this._tileJSONRequest = loadTileJSON(this._options, this.map._requestManager, (err, tileJSON) => {
this._tileJSONRequest = null;
this._loaded = true;
if (err) {
this.fire(new ErrorEvent(err));
} else if (tileJSON) {
Expand All @@ -83,6 +85,10 @@ class RasterTileSource extends Evented implements Source {
});
}

loaded(): boolean {
return this._loaded;
}

onAdd(map: Map) {
this.map = map;
this.load();
Expand Down
1 change: 1 addition & 0 deletions src/source/source.js
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,7 @@ export interface Source {
vectorLayerIds?: Array<string>,

hasTransition(): boolean;
loaded(): boolean;

fire(event: Event): mixed;

Expand Down
1 change: 1 addition & 0 deletions src/source/source_cache.js
Original file line number Diff line number Diff line change
Expand Up @@ -119,6 +119,7 @@ class SourceCache extends Evented {
loaded(): boolean {
if (this._sourceErrored) { return true; }
if (!this._sourceLoaded) { return false; }
if (!this._source.loaded()) { return false; }
for (const t in this._tiles) {
const tile = this._tiles[t];
if (tile.state !== 'loaded' && tile.state !== 'errored')
Expand Down
3 changes: 2 additions & 1 deletion src/source/tile.js
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ const CLOCK_SKEW_RETRY_TIMEOUT = 30000;
import type {Bucket} from '../data/bucket';
import type StyleLayer from '../style/style_layer';
import type {WorkerTileResult} from './worker_source';
import type Actor from '../util/actor';
import type DEMData from '../data/dem_data';
import type {AlphaImage} from '../util/image';
import type ImageAtlas from '../render/image_atlas';
Expand Down Expand Up @@ -73,7 +74,7 @@ class Tile {
redoWhenDone: boolean;
showCollisionBoxes: boolean;
placementSource: any;
workerID: number | void;
actor: ?Actor;
vtLayers: {[string]: VectorTileLayer};
mask: Mask;

Expand Down
29 changes: 24 additions & 5 deletions src/source/vector_tile_source.js
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@ class VectorTileSource extends Evented implements Source {
reparseOverscaled: boolean;
isTileClipped: boolean;
_tileJSONRequest: ?Cancelable;
_loaded: boolean;

constructor(id: string, options: VectorSourceSpecification & {collectResourceTiming: boolean}, dispatcher: Dispatcher, eventedParent: Evented) {
super();
Expand All @@ -51,6 +52,7 @@ class VectorTileSource extends Evented implements Source {
this.tileSize = 512;
this.reparseOverscaled = true;
this.isTileClipped = true;
this._loaded = false;

extend(this, pick(options, ['url', 'scheme', 'tileSize']));
this._options = extend({ type: 'vector' }, options);
Expand All @@ -65,9 +67,11 @@ class VectorTileSource extends Evented implements Source {
}

load() {
this._loaded = false;
this.fire(new Event('dataloading', {dataType: 'source'}));
this._tileJSONRequest = loadTileJSON(this._options, this.map._requestManager, (err, tileJSON) => {
this._tileJSONRequest = null;
this._loaded = true;
if (err) {
this.fire(new ErrorEvent(err));
} else if (tileJSON) {
Expand All @@ -85,6 +89,10 @@ class VectorTileSource extends Evented implements Source {
});
}

loaded(): boolean {
return this._loaded;
}

hasTile(tileID: OverscaledTileID) {
return !this.tileBounds || this.tileBounds.contains(tileID.canonical);
}
Expand Down Expand Up @@ -120,16 +128,19 @@ class VectorTileSource extends Evented implements Source {
};
params.request.collectResourceTiming = this._collectResourceTiming;

if (tile.workerID === undefined || tile.state === 'expired') {
tile.workerID = this.dispatcher.send('loadTile', params, done.bind(this));
if (!tile.actor || tile.state === 'expired') {
tile.actor = this.dispatcher.getActor();
tile.request = tile.actor.send('loadTile', params, done.bind(this));
} else if (tile.state === 'loading') {
// schedule tile reloading after it has been loaded
tile.reloadCallback = callback;
} else {
this.dispatcher.send('reloadTile', params, done.bind(this), tile.workerID);
tile.request = tile.actor.send('reloadTile', params, done.bind(this));
}

function done(err, data) {
delete tile.request;

if (tile.aborted)
return callback(null);

Expand All @@ -155,12 +166,20 @@ class VectorTileSource extends Evented implements Source {
}

abortTile(tile: Tile) {
this.dispatcher.send('abortTile', { uid: tile.uid, type: this.type, source: this.id }, undefined, tile.workerID);
if (tile.request) {
tile.request.cancel();
delete tile.request;
}
if (tile.actor) {
tile.actor.send('abortTile', { uid: tile.uid, type: this.type, source: this.id }, undefined);
}
}

unloadTile(tile: Tile) {
tile.unloadVectorData();
this.dispatcher.send('removeTile', { uid: tile.uid, type: this.type, source: this.id }, undefined, tile.workerID);
if (tile.actor) {
tile.actor.send('removeTile', { uid: tile.uid, type: this.type, source: this.id }, undefined);
}
}

hasTransition() {
Expand Down
2 changes: 2 additions & 0 deletions src/source/video_source.js
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,7 @@ class VideoSource extends ImageSource {
}

load() {
this._loaded = false;
const options = this.options;

this.urls = [];
Expand All @@ -71,6 +72,7 @@ class VideoSource extends ImageSource {
}

getVideo(this.urls, (err, video) => {
this._loaded = true;
if (err) {
this.fire(new ErrorEvent(err));
} else if (video) {
Expand Down
2 changes: 1 addition & 1 deletion src/source/worker_source.js
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ export type WorkerDEMTileCallback = (err: ?Error, result: ?DEMData) => void;
* the WebWorkers. In addition to providing a custom
* {@link WorkerSource#loadTile}, any other methods attached to a `WorkerSource`
* implementation may also be targeted by the {@link Source} via
* `dispatcher.send('source-type.methodname', params, callback)`.
* `dispatcher.getActor().send('source-type.methodname', params, callback)`.
*
* @see {@link Map#addSourceType}
* @private
Expand Down
Loading

0 comments on commit 5f8f1c2

Please sign in to comment.