From 01d310050c8bb9f685ada0b105dc5e43153de4aa Mon Sep 17 00:00:00 2001 From: Anand Thakker Date: Sat, 6 Aug 2016 00:41:27 -0400 Subject: [PATCH 01/10] Enable Worker to do work for multiple map instances --- js/source/worker.js | 85 +++++++++++++++++++++++++------------------ js/style/style.js | 6 +-- js/util/actor.js | 14 ++++--- js/util/dispatcher.js | 3 +- 4 files changed, 64 insertions(+), 44 deletions(-) diff --git a/js/source/worker.js b/js/source/worker.js index b79a30747a9..09e0afbea32 100644 --- a/js/source/worker.js +++ b/js/source/worker.js @@ -15,29 +15,28 @@ function Worker(self) { this.self = self; this.actor = new Actor(self, this); - // simple accessor object for passing to WorkerSources - var styleLayers = { - getLayers: function () { return this.layers; }.bind(this), - getLayerFamilies: function () { return this.layerFamilies; }.bind(this) - }; + this.layers = {}; + this.layerFamilies = {}; - this.workerSources = { - vector: new VectorTileWorkerSource(this.actor, styleLayers), - geojson: new GeoJSONWorkerSource(this.actor, styleLayers) + this.workerSourceTypes = { + vector: VectorTileWorkerSource, + geojson: GeoJSONWorkerSource }; + // [mapId][sourceType] => worker source instance + this.workerSources = {}; + this.self.registerWorkerSource = function (name, WorkerSource) { - if (this.workerSources[name]) { + if (this.workerSourceTypes[name]) { throw new Error('Worker source with name "' + name + '" already registered.'); } - this.workerSources[name] = new WorkerSource(this.actor, styleLayers); + this.workerSourceTypes[name] = WorkerSource; }.bind(this); } util.extend(Worker.prototype, { - 'set layers': function(layers) { - this.layers = {}; - var that = this; + 'set layers': function(mapId, layers) { + var styleLayers = this.layers[mapId] = {}; // Filter layers and create an id -> layer map var childLayerIndicies = []; @@ -60,20 +59,21 @@ util.extend(Worker.prototype, { function setLayer(serializedLayer) { var styleLayer = StyleLayer.create( serializedLayer, - serializedLayer.ref && that.layers[serializedLayer.ref] + serializedLayer.ref && styleLayers[serializedLayer.ref] ); styleLayer.updatePaintTransitions({}, {transition: false}); - that.layers[styleLayer.id] = styleLayer; + styleLayers[styleLayer.id] = styleLayer; } - this.layerFamilies = createLayerFamilies(this.layers); + this.layerFamilies[mapId] = createLayerFamilies(this.layers[mapId]); }, - 'update layers': function(layers) { - var that = this; + 'update layers': function(mapId, layers) { var id; var layer; + var styleLayers = this.layers[mapId]; + // Update ref parents for (id in layers) { layer = layers[id]; @@ -87,41 +87,41 @@ util.extend(Worker.prototype, { } function updateLayer(layer) { - var refLayer = that.layers[layer.ref]; - if (that.layers[layer.id]) { - that.layers[layer.id].set(layer, refLayer); + var refLayer = styleLayers[layer.ref]; + if (styleLayers[layer.id]) { + styleLayers[layer.id].set(layer, refLayer); } else { - that.layers[layer.id] = StyleLayer.create(layer, refLayer); + styleLayers[layer.id] = StyleLayer.create(layer, refLayer); } - that.layers[layer.id].updatePaintTransitions({}, {transition: false}); + styleLayers[layer.id].updatePaintTransitions({}, {transition: false}); } - this.layerFamilies = createLayerFamilies(this.layers); + this.layerFamilies[mapId] = createLayerFamilies(this.layers[mapId]); }, - 'load tile': function(params, callback) { + 'load tile': function(mapId, params, callback) { var type = params.type || 'vector'; - this.workerSources[type].loadTile(params, callback); + this.getWorkerSource(mapId, type).loadTile(params, callback); }, - 'reload tile': function(params, callback) { + 'reload tile': function(mapId, params, callback) { var type = params.type || 'vector'; - this.workerSources[type].reloadTile(params, callback); + this.getWorkerSource(mapId, type).reloadTile(params, callback); }, - 'abort tile': function(params) { + 'abort tile': function(mapId, params) { var type = params.type || 'vector'; - this.workerSources[type].abortTile(params); + this.getWorkerSource(mapId, type).abortTile(params); }, - 'remove tile': function(params) { + 'remove tile': function(mapId, params) { var type = params.type || 'vector'; - this.workerSources[type].removeTile(params); + this.getWorkerSource(mapId, type).removeTile(params); }, - 'redo placement': function(params, callback) { + 'redo placement': function(mapId, params, callback) { var type = params.type || 'vector'; - this.workerSources[type].redoPlacement(params, callback); + this.getWorkerSource(mapId, type).redoPlacement(params, callback); }, /** @@ -130,13 +130,28 @@ util.extend(Worker.prototype, { * function taking `(name, workerSourceObject)`. * @private */ - 'load worker source': function(params, callback) { + 'load worker source': function(map, params, callback) { try { this.self.importScripts(params.url); callback(); } catch (e) { callback(e); } + }, + + getWorkerSource: function(mapId, type) { + if (!this.workerSources[mapId]) + this.workerSources[mapId] = {}; + if (!this.workerSources[mapId][type]) { + // simple accessor object for passing to WorkerSources + var styleLayers = { + getLayers: function () { return this.layers[mapId]; }.bind(this), + getLayerFamilies: function () { return this.layerFamilies[mapId]; }.bind(this) + }; + this.workerSources[mapId][type] = new this.workerSourceTypes[type](this.actor, styleLayers); + } + + return this.workerSources[mapId][type]; } }); diff --git a/js/style/style.js b/js/style/style.js index 83a1ee638f0..85497c89a83 100644 --- a/js/style/style.js +++ b/js/style/style.js @@ -731,7 +731,7 @@ Style.prototype = util.inherit(Evented, { // Callbacks from web workers - 'get sprite json': function(params, callback) { + 'get sprite json': function(_, params, callback) { var sprite = this.sprite; if (sprite.loaded()) { callback(null, { sprite: sprite.data, retina: sprite.retina }); @@ -742,7 +742,7 @@ Style.prototype = util.inherit(Evented, { } }, - 'get icons': function(params, callback) { + 'get icons': function(_, params, callback) { var sprite = this.sprite; var spriteAtlas = this.spriteAtlas; if (sprite.loaded()) { @@ -756,7 +756,7 @@ Style.prototype = util.inherit(Evented, { } }, - 'get glyphs': function(params, callback) { + 'get glyphs': function(_, params, callback) { var stacks = params.stacks, remaining = Object.keys(stacks).length, allGlyphs = {}; diff --git a/js/util/actor.js b/js/util/actor.js index dc09086cfeb..0c0fd9da40c 100644 --- a/js/util/actor.js +++ b/js/util/actor.js @@ -10,11 +10,13 @@ module.exports = Actor; * * @param {WebWorker} target * @param {WebWorker} parent + * @param {string|number} mapId A unique identifier for the Map instance using this Actor. * @private */ -function Actor(target, parent) { +function Actor(target, parent, mapId) { this.target = target; this.parent = parent; + this.mapId = mapId; this.callbacks = {}; this.callbackID = 0; this.receive = this.receive.bind(this); @@ -32,17 +34,19 @@ Actor.prototype.receive = function(message) { if (callback) callback(data.error || null, data.data); } else if (typeof data.id !== 'undefined' && this.parent[data.type]) { // data.type == 'load tile', 'remove tile', etc. - this.parent[data.type](data.data, done.bind(this)); - } else if (typeof data.id !== 'undefined' && this.parent.workerSources) { + this.parent[data.type](data.mapId, data.data, done.bind(this)); + } else if (typeof data.id !== 'undefined' && this.parent.getWorkerSource) { // data.type == sourcetype.method var keys = data.type.split('.'); - this.parent.workerSources[keys[0]][keys[1]](data.data, done.bind(this)); + var workerSource = this.parent.getWorkerSource(data.mapId, keys[0]); + workerSource[keys[1]](data.data, done.bind(this)); } else { this.parent[data.type](data.data); } function done(err, data, buffers) { this.postMessage({ + mapId: this.mapId, type: '', id: String(id), error: err ? String(err) : null, @@ -54,7 +58,7 @@ Actor.prototype.receive = function(message) { Actor.prototype.send = function(type, data, callback, buffers) { var id = null; if (callback) this.callbacks[id = this.callbackID++] = callback; - this.postMessage({ type: type, id: String(id), data: data }, buffers); + this.postMessage({ mapId: this.mapId, type: type, id: String(id), data: data }, buffers); }; /** diff --git a/js/util/dispatcher.js b/js/util/dispatcher.js index 7ca9bd7c367..cc7a9206e9b 100644 --- a/js/util/dispatcher.js +++ b/js/util/dispatcher.js @@ -16,9 +16,10 @@ module.exports = Dispatcher; function Dispatcher(length, parent) { this.actors = []; this.currentActor = 0; + this.id = util.uniqueId(); for (var i = 0; i < length; i++) { var worker = new WebWorker(); - var actor = new Actor(worker, parent); + var actor = new Actor(worker, parent, this.id); actor.name = "Worker " + i; this.actors.push(actor); } From 1db70897f8740db99a9217507325451ffc944f68 Mon Sep 17 00:00:00 2001 From: Anand Thakker Date: Sat, 6 Aug 2016 01:20:08 -0400 Subject: [PATCH 02/10] Add minimal worker pool --- debug/shared_workers.html | 122 ++++++++++++++++++++++++++++++++++ js/util/dispatcher.js | 11 +-- js/util/worker_pool.js | 35 ++++++++++ test/js/source/worker.test.js | 75 ++++++++++++++++----- 4 files changed, 220 insertions(+), 23 deletions(-) create mode 100644 debug/shared_workers.html create mode 100644 js/util/worker_pool.js diff --git a/debug/shared_workers.html b/debug/shared_workers.html new file mode 100644 index 00000000000..e36330e7fa7 --- /dev/null +++ b/debug/shared_workers.html @@ -0,0 +1,122 @@ + + + + Mapbox GL JS debug page + + + + + + + + +
+
+
+
+ +
+
+
+
+ +
+ + + + + + + + + + diff --git a/js/util/dispatcher.js b/js/util/dispatcher.js index cc7a9206e9b..51e9fa3b41f 100644 --- a/js/util/dispatcher.js +++ b/js/util/dispatcher.js @@ -1,10 +1,11 @@ 'use strict'; +var WorkerPool = require('./worker_pool'); var util = require('./util'); var Actor = require('./actor'); -var WebWorker = require('./web_worker'); module.exports = Dispatcher; +Dispatcher.workerPool = new WorkerPool(); /** * Responsible for sending messages from a {@link Source} to an associated @@ -17,8 +18,9 @@ function Dispatcher(length, parent) { this.actors = []; this.currentActor = 0; this.id = util.uniqueId(); + var workers = Dispatcher.workerPool.acquire(this.id, length); for (var i = 0; i < length; i++) { - var worker = new WebWorker(); + var worker = workers[i]; var actor = new Actor(worker, parent, this.id); actor.name = "Worker " + i; this.actors.push(actor); @@ -66,9 +68,8 @@ Dispatcher.prototype = { }, remove: function() { - for (var i = 0; i < this.actors.length; i++) { - this.actors[i].target.terminate(); - } + Dispatcher.workerPool.release(this.id); this.actors = []; } }; + diff --git a/js/util/worker_pool.js b/js/util/worker_pool.js new file mode 100644 index 00000000000..246cf80b1b7 --- /dev/null +++ b/js/util/worker_pool.js @@ -0,0 +1,35 @@ +'use strict'; + +var assert = require('assert'); +var WebWorker = require('./web_worker'); + +module.exports = WorkerPool; + +function WorkerPool() { + this.workers = []; + this.active = {}; +} + +WorkerPool.prototype = { + acquire: function (mapId, workerCount) { + this._resize(workerCount); + this.active[mapId] = workerCount; + return this.workers.slice(0, workerCount); + }, + + release: function (mapId) { + delete this.active[mapId]; + if (Object.keys(this.active).length === 0) { + this.workers.forEach(function (w) { w.terminate(); }); + this.workers = []; + } + }, + + _resize: function (len) { + assert(typeof len === 'number'); + while (this.workers.length < len) { + this.workers.push(new WebWorker()); + } + } +}; + diff --git a/test/js/source/worker.test.js b/test/js/source/worker.test.js index 66d04dcc708..25846da8d25 100644 --- a/test/js/source/worker.test.js +++ b/test/js/source/worker.test.js @@ -26,7 +26,7 @@ test('before', function(t) { test('load tile', function(t) { t.test('calls callback on error', function(t) { var worker = new Worker(_self); - worker['load tile']({ + worker['load tile'](0, { source: 'source', uid: 0, url: 'http://localhost:2900/error' @@ -42,25 +42,25 @@ test('load tile', function(t) { test('set layers', function(t) { var worker = new Worker(_self); - worker['set layers']([ + worker['set layers'](0, [ { id: 'one', type: 'circle', paint: { 'circle-color': 'red' } }, { id: 'two', type: 'circle', paint: { 'circle-color': 'green' } }, { id: 'three', ref: 'two', type: 'circle', paint: { 'circle-color': 'blue' } } ]); - t.equal(worker.layers.one.id, 'one'); - t.equal(worker.layers.two.id, 'two'); - t.equal(worker.layers.three.id, 'three'); + t.equal(worker.layers[0].one.id, 'one'); + t.equal(worker.layers[0].two.id, 'two'); + t.equal(worker.layers[0].three.id, 'three'); - t.equal(worker.layers.one.getPaintProperty('circle-color'), 'red'); - t.equal(worker.layers.two.getPaintProperty('circle-color'), 'green'); - t.equal(worker.layers.three.getPaintProperty('circle-color'), 'blue'); + t.equal(worker.layers[0].one.getPaintProperty('circle-color'), 'red'); + t.equal(worker.layers[0].two.getPaintProperty('circle-color'), 'green'); + t.equal(worker.layers[0].three.getPaintProperty('circle-color'), 'blue'); - t.equal(worker.layerFamilies.one.length, 1); - t.equal(worker.layerFamilies.one[0].id, 'one'); - t.equal(worker.layerFamilies.two.length, 2); - t.equal(worker.layerFamilies.two[0].id, 'two'); - t.equal(worker.layerFamilies.two[1].id, 'three'); + t.equal(worker.layerFamilies[0].one.length, 1); + t.equal(worker.layerFamilies[0].one[0].id, 'one'); + t.equal(worker.layerFamilies[0].two.length, 2); + t.equal(worker.layerFamilies[0].two[0].id, 'two'); + t.equal(worker.layerFamilies[0].two[1].id, 'three'); t.end(); }); @@ -68,21 +68,21 @@ test('set layers', function(t) { test('update layers', function(t) { var worker = new Worker(_self); - worker['set layers']([ + worker['set layers'](0, [ { id: 'one', type: 'circle', paint: { 'circle-color': 'red' } }, { id: 'two', type: 'circle', paint: { 'circle-color': 'green' } }, { id: 'three', ref: 'two', type: 'circle', paint: { 'circle-color': 'blue' } } ]); - worker['update layers']({ + worker['update layers'](0, { one: { id: 'one', type: 'circle', paint: { 'circle-color': 'cyan' } }, two: { id: 'two', type: 'circle', paint: { 'circle-color': 'magenta' } }, three: { id: 'three', ref: 'two', type: 'circle', paint: { 'circle-color': 'yellow' } } }); - t.equal(worker.layers.one.getPaintProperty('circle-color'), 'cyan'); - t.equal(worker.layers.two.getPaintProperty('circle-color'), 'magenta'); - t.equal(worker.layers.three.getPaintProperty('circle-color'), 'yellow'); + t.equal(worker.layers[0].one.getPaintProperty('circle-color'), 'cyan'); + t.equal(worker.layers[0].two.getPaintProperty('circle-color'), 'magenta'); + t.equal(worker.layers[0].three.getPaintProperty('circle-color'), 'yellow'); t.end(); }); @@ -99,6 +99,45 @@ test('redo placement', function(t) { worker['redo placement']({type: 'test', mapbox: true}); }); +test('update layers isolates different instances\' data', function(t) { + var worker = new Worker(_self); + + worker['set layers'](0, [ + { id: 'one', type: 'circle', paint: { 'circle-color': 'red' } }, + { id: 'two', type: 'circle', paint: { 'circle-color': 'green' } }, + { id: 'three', ref: 'two', type: 'circle', paint: { 'circle-color': 'blue' } } + ]); + + worker['set layers'](1, [ + { id: 'one', type: 'circle', paint: { 'circle-color': 'red' } }, + { id: 'two', type: 'circle', paint: { 'circle-color': 'green' } }, + { id: 'three', ref: 'two', type: 'circle', paint: { 'circle-color': 'blue' } } + ]); + + worker['update layers'](1, { + one: { id: 'one', type: 'circle', paint: { 'circle-color': 'cyan' } }, + two: { id: 'two', type: 'circle', paint: { 'circle-color': 'magenta' } }, + three: { id: 'three', ref: 'two', type: 'circle', paint: { 'circle-color': 'yellow' } } + }); + + t.equal(worker.layers[0].one.id, 'one'); + t.equal(worker.layers[0].two.id, 'two'); + t.equal(worker.layers[0].three.id, 'three'); + + t.equal(worker.layers[0].one.getPaintProperty('circle-color'), 'red'); + t.equal(worker.layers[0].two.getPaintProperty('circle-color'), 'green'); + t.equal(worker.layers[0].three.getPaintProperty('circle-color'), 'blue'); + + t.equal(worker.layerFamilies[0].one.length, 1); + t.equal(worker.layerFamilies[0].one[0].id, 'one'); + t.equal(worker.layerFamilies[0].two.length, 2); + t.equal(worker.layerFamilies[0].two[0].id, 'two'); + t.equal(worker.layerFamilies[0].two[1].id, 'three'); + + + t.end(); +}); + test('after', function(t) { server.close(t.end); }); From f9622cf0fdf8766b2bdba3ed840ee6b4bd12d787 Mon Sep 17 00:00:00 2001 From: Anand Thakker Date: Thu, 11 Aug 2016 13:01:56 -0400 Subject: [PATCH 03/10] Include mapId in Actors' callback tracking --- js/util/actor.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/js/util/actor.js b/js/util/actor.js index 0c0fd9da40c..9aefb0298de 100644 --- a/js/util/actor.js +++ b/js/util/actor.js @@ -56,8 +56,8 @@ Actor.prototype.receive = function(message) { }; Actor.prototype.send = function(type, data, callback, buffers) { - var id = null; - if (callback) this.callbacks[id = this.callbackID++] = callback; + var id = callback ? this.mapId + ':' + this.callbackID++ : null; + if (callback) this.callbacks[id] = callback; this.postMessage({ mapId: this.mapId, type: type, id: String(id), data: data }, buffers); }; From afbdafdd0180db3bb46a28c79c728aab4591cdfc Mon Sep 17 00:00:00 2001 From: Anand Thakker Date: Thu, 11 Aug 2016 19:41:10 -0400 Subject: [PATCH 04/10] Unit test Dispatcher's use of WorkerPool --- js/util/dispatcher.js | 2 +- test/js/util/dispatcher.test.js | 39 +++++++++++++++++++++++++++++++++ 2 files changed, 40 insertions(+), 1 deletion(-) create mode 100644 test/js/util/dispatcher.test.js diff --git a/js/util/dispatcher.js b/js/util/dispatcher.js index 51e9fa3b41f..1515734f26b 100644 --- a/js/util/dispatcher.js +++ b/js/util/dispatcher.js @@ -19,7 +19,7 @@ function Dispatcher(length, parent) { this.currentActor = 0; this.id = util.uniqueId(); var workers = Dispatcher.workerPool.acquire(this.id, length); - for (var i = 0; i < length; i++) { + for (var i = 0; i < workers.length; i++) { var worker = workers[i]; var actor = new Actor(worker, parent, this.id); actor.name = "Worker " + i; diff --git a/test/js/util/dispatcher.test.js b/test/js/util/dispatcher.test.js new file mode 100644 index 00000000000..5911061b1b9 --- /dev/null +++ b/test/js/util/dispatcher.test.js @@ -0,0 +1,39 @@ +'use strict'; + +var test = require('tap').test; +var Dispatcher = require('../../../js/util/dispatcher'); +var WebWorker = require('../../../js/util/web_worker'); +var originalWorkerPool = Dispatcher.workerPool; + +test('Dispatcher', function (t) { + t.test('requests and releases workers from pool', function (t) { + var dispatcher; + var workers = [new WebWorker(), new WebWorker()]; + + var releaseCalled = []; + Dispatcher.workerPool = { + acquire: function (id, count) { + t.equal(count, 2); + return workers; + }, + release: function (id) { + releaseCalled.push(id); + } + }; + + dispatcher = new Dispatcher(2, {}); + t.same(dispatcher.actors.map(function (actor) { return actor.target; }), workers); + dispatcher.remove(); + t.equal(dispatcher.actors.length, 0, 'actors discarded'); + t.same(releaseCalled, [dispatcher.id]); + t.end(); + }); + + t.end(); +}); + +test('after', function (t) { + Dispatcher.workerPool = originalWorkerPool; + t.end(); +}); + From 28eee2fa19cee99f3d658bbc50e2cb000a704288 Mon Sep 17 00:00:00 2001 From: Anand Thakker Date: Thu, 11 Aug 2016 19:53:58 -0400 Subject: [PATCH 05/10] Fix benchmark --- bench/benchmarks/buffer.js | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/bench/benchmarks/buffer.js b/bench/benchmarks/buffer.js index 61d0b13ce9c..0b8f1a8fd61 100644 --- a/bench/benchmarks/buffer.js +++ b/bench/benchmarks/buffer.js @@ -91,14 +91,14 @@ function preloadAssets(stylesheet, callback) { style.on('load', function() { function getGlyphs(params, callback) { - style['get glyphs'](params, function(err, glyphs) { + style['get glyphs'](0, params, function(err, glyphs) { assets.glyphs[JSON.stringify(params)] = glyphs; callback(err, glyphs); }); } function getIcons(params, callback) { - style['get icons'](params, function(err, icons) { + style['get icons'](0, params, function(err, icons) { assets.icons[JSON.stringify(params)] = icons; callback(err, icons); }); @@ -185,10 +185,10 @@ var createLayerFamiliesCacheValue; function createLayerFamilies(layers) { if (layers !== createLayerFamiliesCacheKey) { var worker = new Worker({addEventListener: function() {} }); - worker['set layers'](layers); + worker['set layers'](0, layers); createLayerFamiliesCacheKey = layers; - createLayerFamiliesCacheValue = worker.layerFamilies; + createLayerFamiliesCacheValue = worker.layerFamilies[0]; } return createLayerFamiliesCacheValue; } From dfdd10cf2ef75c6c508046caee4efa203a67ed3a Mon Sep 17 00:00:00 2001 From: Anand Thakker Date: Thu, 11 Aug 2016 23:14:22 -0400 Subject: [PATCH 06/10] Add WorkerPool unit tests --- test/js/util/worker_pool.test.js | 46 ++++++++++++++++++++++++++++++++ 1 file changed, 46 insertions(+) create mode 100644 test/js/util/worker_pool.test.js diff --git a/test/js/util/worker_pool.test.js b/test/js/util/worker_pool.test.js new file mode 100644 index 00000000000..f1f085d677c --- /dev/null +++ b/test/js/util/worker_pool.test.js @@ -0,0 +1,46 @@ +'use strict'; + +var test = require('tap').test; +var WorkerPool = require('../../../js/util/worker_pool'); + +test('WorkerPool', function (t) { + t.test('#acquire', function (t) { + var pool = new WorkerPool(); + + t.equal(pool.workers.length, 0); + var workers1 = pool.acquire('map-1', 4); + t.equal(workers1.length, 4); + + var workers2 = pool.acquire('map-2', 8); + t.equal(workers2.length, 8); + t.equal(pool.workers.length, 8); + + // check that the two different dispatchers' workers arrays correspond + workers1.forEach(function (w, i) { t.equal(w, workers2[i]); }); + t.end(); + }); + + t.test('#release', function (t) { + var pool = new WorkerPool(); + pool.acquire('map-1', 1); + var workers = pool.acquire('map-2', 4); + var terminated = 0; + workers.forEach(function (w) { + w.terminate = function () { terminated += 1; }; + }); + + pool.release('map-2'); + t.comment('keeps workers if a dispatcher is still active'); + t.equal(terminated, 0); + t.equal(pool.workers.length, 4); + + t.comment('terminates workers if no dispatchers are active'); + pool.release('map-1'); + t.equal(terminated, 4); + t.equal(pool.workers.length, 0); + + t.end(); + }); + + t.end(); +}); From e65e6b60f377de7775d3f5c4ee49a910ac7b3b3f Mon Sep 17 00:00:00 2001 From: Anand Thakker Date: Thu, 11 Aug 2016 23:53:15 -0400 Subject: [PATCH 07/10] Add a unit test for 2 Actors using the same Worker --- test/js/util/actor.test.js | 36 +++++++++++++++++++++++++++++++++ test/js/util/dispatcher.test.js | 21 ++++++++++++++++++- 2 files changed, 56 insertions(+), 1 deletion(-) create mode 100644 test/js/util/actor.test.js diff --git a/test/js/util/actor.test.js b/test/js/util/actor.test.js new file mode 100644 index 00000000000..df02df69c24 --- /dev/null +++ b/test/js/util/actor.test.js @@ -0,0 +1,36 @@ +'use strict'; + +var test = require('tap').test; +var proxyquire = require('proxyquire'); +var Actor = require('../../../js/util/actor'); + +test('Actor', function (t) { + t.test('forwards resopnses to correct callback', function (t) { + var WebWorker = proxyquire('../../../js/util/web_worker', { + '../source/worker': function Worker(self) { + this.self = self; + this.actor = new Actor(self, this); + this.test = function (mapId, params, callback) { + setTimeout(callback, 0, null, params); + }; + } + }); + + var worker = new WebWorker(); + + var m1 = new Actor(worker, {}, 'map-1'); + var m2 = new Actor(worker, {}, 'map-2'); + + t.plan(4); + m1.send('test', { value: 1729 }, function (err, response) { + t.error(err); + t.same(response, { value: 1729 }); + }); + m2.send('test', { value: 4104 }, function (err, response) { + t.error(err); + t.same(response, { value: 4104 }); + }); + }); + + t.end(); +}); diff --git a/test/js/util/dispatcher.test.js b/test/js/util/dispatcher.test.js index 5911061b1b9..49b603ec759 100644 --- a/test/js/util/dispatcher.test.js +++ b/test/js/util/dispatcher.test.js @@ -1,6 +1,7 @@ 'use strict'; var test = require('tap').test; +var proxyquire = require('proxyquire'); var Dispatcher = require('../../../js/util/dispatcher'); var WebWorker = require('../../../js/util/web_worker'); var originalWorkerPool = Dispatcher.workerPool; @@ -26,6 +27,20 @@ test('Dispatcher', function (t) { dispatcher.remove(); t.equal(dispatcher.actors.length, 0, 'actors discarded'); t.same(releaseCalled, [dispatcher.id]); + + restoreWorkerPool(); + t.end(); + }); + + test('creates Actors with unique map id', function (t) { + var Dispatcher = proxyquire('../../../js/util/dispatcher', { './actor': Actor }); + + var ids = []; + function Actor (target, parent, mapId) { ids.push(mapId); } + + var dispatchers = [new Dispatcher(1, {}), new Dispatcher(1, {})]; + t.same(ids, dispatchers.map(function (d) { return d.id; })); + t.end(); }); @@ -33,7 +48,11 @@ test('Dispatcher', function (t) { }); test('after', function (t) { - Dispatcher.workerPool = originalWorkerPool; + restoreWorkerPool(); t.end(); }); +function restoreWorkerPool () { + Dispatcher.workerPool = originalWorkerPool; +} + From ad3444bc5f5e6575e6294d2fecae20fd2ca14f4c Mon Sep 17 00:00:00 2001 From: Anand Thakker Date: Thu, 11 Aug 2016 23:58:47 -0400 Subject: [PATCH 08/10] Fix parameter in Worker 'redo placement' test --- test/js/source/worker.test.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/js/source/worker.test.js b/test/js/source/worker.test.js index 25846da8d25..45c6c3946c9 100644 --- a/test/js/source/worker.test.js +++ b/test/js/source/worker.test.js @@ -96,7 +96,7 @@ test('redo placement', function(t) { }; }); - worker['redo placement']({type: 'test', mapbox: true}); + worker['redo placement'](0, {type: 'test', mapbox: true}); }); test('update layers isolates different instances\' data', function(t) { From 44d23b59938b4021155b0e029c16c045ac84e2b2 Mon Sep 17 00:00:00 2001 From: Anand Thakker Date: Fri, 12 Aug 2016 00:34:40 -0400 Subject: [PATCH 09/10] Add map load benchmark --- bench/benchmarks/map_load.js | 40 ++++++++++++++++++++++++++++++++++++ bench/index.js | 1 + 2 files changed, 41 insertions(+) create mode 100644 bench/benchmarks/map_load.js diff --git a/bench/benchmarks/map_load.js b/bench/benchmarks/map_load.js new file mode 100644 index 00000000000..01de2faefe4 --- /dev/null +++ b/bench/benchmarks/map_load.js @@ -0,0 +1,40 @@ +'use strict'; + +var Evented = require('../../js/util/evented'); +var util = require('../../js/util/util'); +var formatNumber = require('../lib/format_number'); + +module.exports = function(options) { + var evented = util.extend({}, Evented); + + var mapsOnPage = 6; + + evented.fire('log', { message: 'Creating ' + mapsOnPage + ' maps' }); + + var loaded = 0; + var start = Date.now(); + for (var i = 0; i < mapsOnPage; i++) { + var map = options.createMap({}); + map.on('load', onload.bind(null, map)); + map.on('error', function (err) { + evented.fire('error', err); + }); + } + + function onload () { + if (++loaded >= mapsOnPage) { + var duration = Date.now() - start; + evented.fire('end', { + message: formatNumber(duration) + ' ms, loaded ' + mapsOnPage + ' maps.', + score: duration + }); + done(); + } + } + + function done () { + } + + return evented; +}; + diff --git a/bench/index.js b/bench/index.js index b6d11c43111..40895d79f7d 100644 --- a/bench/index.js +++ b/bench/index.js @@ -198,6 +198,7 @@ var BenchmarksView = React.createClass({ }); var benchmarks = { + 'load-multiple-maps': require('./benchmarks/map_load'), buffer: require('./benchmarks/buffer'), fps: require('./benchmarks/fps'), 'frame-duration': require('./benchmarks/frame_duration'), From 202ad9c97d0be75d0b75d287ed0e20a640a06762 Mon Sep 17 00:00:00 2001 From: Anand Thakker Date: Wed, 17 Aug 2016 18:08:57 -0400 Subject: [PATCH 10/10] Move the static worker pool instance to style.js This is still not great, but it's better than having it in Dispatcher. Ultimately the right place to put it is at the root level, i.e. mapbox-gl.js, and then pass it through, but that will be a noisy change due the number of 'new Map()' and 'new Style()' in the unit tests --- js/style/style.js | 5 ++++- js/util/dispatcher.js | 9 ++++----- test/js/util/dispatcher.test.js | 19 +++++-------------- 3 files changed, 13 insertions(+), 20 deletions(-) diff --git a/js/style/style.js b/js/style/style.js index 85497c89a83..dc9f98f1adc 100644 --- a/js/style/style.js +++ b/js/style/style.js @@ -18,12 +18,15 @@ var QueryFeatures = require('../source/query_features'); var SourceCache = require('../source/source_cache'); var styleSpec = require('./style_spec'); var StyleFunction = require('./style_function'); +var WorkerPool = require('../util/worker_pool'); + +var workerPool = new WorkerPool(); module.exports = Style; function Style(stylesheet, animationLoop, workerCount) { this.animationLoop = animationLoop || new AnimationLoop(); - this.dispatcher = new Dispatcher(workerCount || 1, this); + this.dispatcher = new Dispatcher(workerPool, workerCount || 1, this); this.spriteAtlas = new SpriteAtlas(1024, 1024); this.lineAtlas = new LineAtlas(256, 512); diff --git a/js/util/dispatcher.js b/js/util/dispatcher.js index 1515734f26b..c12a11724ba 100644 --- a/js/util/dispatcher.js +++ b/js/util/dispatcher.js @@ -1,11 +1,9 @@ 'use strict'; -var WorkerPool = require('./worker_pool'); var util = require('./util'); var Actor = require('./actor'); module.exports = Dispatcher; -Dispatcher.workerPool = new WorkerPool(); /** * Responsible for sending messages from a {@link Source} to an associated @@ -14,11 +12,12 @@ Dispatcher.workerPool = new WorkerPool(); * @interface Dispatcher * @private */ -function Dispatcher(length, parent) { +function Dispatcher(workerPool, length, parent) { + this.workerPool = workerPool; this.actors = []; this.currentActor = 0; this.id = util.uniqueId(); - var workers = Dispatcher.workerPool.acquire(this.id, length); + var workers = this.workerPool.acquire(this.id, length); for (var i = 0; i < workers.length; i++) { var worker = workers[i]; var actor = new Actor(worker, parent, this.id); @@ -68,7 +67,7 @@ Dispatcher.prototype = { }, remove: function() { - Dispatcher.workerPool.release(this.id); + this.workerPool.release(this.id); this.actors = []; } }; diff --git a/test/js/util/dispatcher.test.js b/test/js/util/dispatcher.test.js index 49b603ec759..bb71257c661 100644 --- a/test/js/util/dispatcher.test.js +++ b/test/js/util/dispatcher.test.js @@ -4,7 +4,7 @@ var test = require('tap').test; var proxyquire = require('proxyquire'); var Dispatcher = require('../../../js/util/dispatcher'); var WebWorker = require('../../../js/util/web_worker'); -var originalWorkerPool = Dispatcher.workerPool; +var WorkerPool = require('../../../js/util/worker_pool'); test('Dispatcher', function (t) { t.test('requests and releases workers from pool', function (t) { @@ -12,7 +12,7 @@ test('Dispatcher', function (t) { var workers = [new WebWorker(), new WebWorker()]; var releaseCalled = []; - Dispatcher.workerPool = { + var workerPool = { acquire: function (id, count) { t.equal(count, 2); return workers; @@ -22,13 +22,12 @@ test('Dispatcher', function (t) { } }; - dispatcher = new Dispatcher(2, {}); + dispatcher = new Dispatcher(workerPool, 2, {}); t.same(dispatcher.actors.map(function (actor) { return actor.target; }), workers); dispatcher.remove(); t.equal(dispatcher.actors.length, 0, 'actors discarded'); t.same(releaseCalled, [dispatcher.id]); - restoreWorkerPool(); t.end(); }); @@ -38,7 +37,8 @@ test('Dispatcher', function (t) { var ids = []; function Actor (target, parent, mapId) { ids.push(mapId); } - var dispatchers = [new Dispatcher(1, {}), new Dispatcher(1, {})]; + var workerPool = new WorkerPool(); + var dispatchers = [new Dispatcher(workerPool, 1, {}), new Dispatcher(workerPool, 1, {})]; t.same(ids, dispatchers.map(function (d) { return d.id; })); t.end(); @@ -47,12 +47,3 @@ test('Dispatcher', function (t) { t.end(); }); -test('after', function (t) { - restoreWorkerPool(); - t.end(); -}); - -function restoreWorkerPool () { - Dispatcher.workerPool = originalWorkerPool; -} -