From f21dcedf0f2b35e9b05d5e48880e0408c1697a63 Mon Sep 17 00:00:00 2001 From: Anand Thakker Date: Fri, 19 Aug 2016 00:05:52 -0400 Subject: [PATCH] Use a fixed worker pool size --- js/global_worker_pool.js | 16 ++++++++++++++ js/mapbox-gl.js | 2 ++ js/style/style.js | 8 +++---- js/ui/map.js | 11 ++-------- js/util/dispatcher.js | 4 ++-- js/util/worker_pool.js | 29 +++++++++++++------------ test/js/style/style.test.js | 7 ------- test/js/ui/map.test.js | 12 ----------- test/js/util/dispatcher.test.js | 11 ++++++---- test/js/util/worker_pool.test.js | 36 +++++++++++++++++++++----------- 10 files changed, 72 insertions(+), 64 deletions(-) create mode 100644 js/global_worker_pool.js diff --git a/js/global_worker_pool.js b/js/global_worker_pool.js new file mode 100644 index 00000000000..3391b3e74ce --- /dev/null +++ b/js/global_worker_pool.js @@ -0,0 +1,16 @@ +'use strict'; +var WorkerPool = require('./util/worker_pool'); + +var globalWorkerPool; + +/** + * Creates (if necessary) and returns the single, global WorkerPool instance + * to be shared across each Map + * @private + */ +module.exports = function getGlobalWorkerPool () { + if (!globalWorkerPool) { + globalWorkerPool = new WorkerPool(); + } + return globalWorkerPool; +}; diff --git a/js/mapbox-gl.js b/js/mapbox-gl.js index d1533d84343..7618b57e3bd 100644 --- a/js/mapbox-gl.js +++ b/js/mapbox-gl.js @@ -13,6 +13,8 @@ mapboxgl.Attribution = require('./ui/control/attribution'); mapboxgl.Popup = require('./ui/popup'); mapboxgl.Marker = require('./ui/marker'); +mapboxgl.WorkerPool = require('./util/worker_pool'); + mapboxgl.Style = require('./style/style'); mapboxgl.LngLat = require('./geo/lng_lat'); diff --git a/js/style/style.js b/js/style/style.js index dc9f98f1adc..d641b21c5c7 100644 --- a/js/style/style.js +++ b/js/style/style.js @@ -18,15 +18,13 @@ 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(); +var getWorkerPool = require('../global_worker_pool'); module.exports = Style; -function Style(stylesheet, animationLoop, workerCount) { +function Style(stylesheet, animationLoop) { this.animationLoop = animationLoop || new AnimationLoop(); - this.dispatcher = new Dispatcher(workerPool, workerCount || 1, this); + this.dispatcher = new Dispatcher(getWorkerPool(), this); this.spriteAtlas = new SpriteAtlas(1024, 1024); this.lineAtlas = new LineAtlas(256, 512); diff --git a/js/ui/map.js b/js/ui/map.js index 38802322917..1a52ae29e23 100755 --- a/js/ui/map.js +++ b/js/ui/map.js @@ -52,8 +52,7 @@ var defaultOptions = { failIfMajorPerformanceCaveat: false, preserveDrawingBuffer: false, - trackResize: true, - workerCount: Math.max(browser.hardwareConcurrency - 1, 1) + trackResize: true }; /** @@ -114,7 +113,6 @@ var defaultOptions = { * @param {number} [options.zoom=0] The initial zoom level of the map. If `zoom` is not specified in the constructor options, Mapbox GL JS will look for it in the map's style object. If it is not specified in the style, either, it will default to `0`. * @param {number} [options.bearing=0] The initial bearing (rotation) of the map, measured in degrees counter-clockwise from north. If `bearing` is not specified in the constructor options, Mapbox GL JS will look for it in the map's style object. If it is not specified in the style, either, it will default to `0`. * @param {number} [options.pitch=0] The initial pitch (tilt) of the map, measured in degrees away from the plane of the screen (0-60). If `pitch` is not specified in the constructor options, Mapbox GL JS will look for it in the map's style object. If it is not specified in the style, either, it will default to `0`. - * @param {number} [options.workerCount=navigator.hardwareConcurrency - 1] The number of WebWorkers that Mapbox GL JS should use to process vector tile data. * @example * var map = new mapboxgl.Map({ * container: 'map', @@ -128,15 +126,10 @@ var Map = module.exports = function(options) { options = util.extend({}, defaultOptions, options); - if (options.workerCount < 1) { - throw new Error('workerCount must an integer greater than or equal to 1.'); - } - this._interactive = options.interactive; this._failIfMajorPerformanceCaveat = options.failIfMajorPerformanceCaveat; this._preserveDrawingBuffer = options.preserveDrawingBuffer; this._trackResize = options.trackResize; - this._workerCount = options.workerCount; this._bearingSnap = options.bearingSnap; if (typeof options.container === 'string') { @@ -641,7 +634,7 @@ util.extend(Map.prototype, /** @lends Map.prototype */{ } else if (style instanceof Style) { this.style = style; } else { - this.style = new Style(style, this.animationLoop, this._workerCount); + this.style = new Style(style, this.animationLoop); } this.style diff --git a/js/util/dispatcher.js b/js/util/dispatcher.js index c12a11724ba..d45a562acc8 100644 --- a/js/util/dispatcher.js +++ b/js/util/dispatcher.js @@ -12,12 +12,12 @@ module.exports = Dispatcher; * @interface Dispatcher * @private */ -function Dispatcher(workerPool, length, parent) { +function Dispatcher(workerPool, parent) { this.workerPool = workerPool; this.actors = []; this.currentActor = 0; this.id = util.uniqueId(); - var workers = this.workerPool.acquire(this.id, length); + var workers = this.workerPool.acquire(this.id); for (var i = 0; i < workers.length; i++) { var worker = workers[i]; var actor = new Actor(worker, parent, this.id); diff --git a/js/util/worker_pool.js b/js/util/worker_pool.js index 246cf80b1b7..3ac9e1b0e0f 100644 --- a/js/util/worker_pool.js +++ b/js/util/worker_pool.js @@ -2,33 +2,36 @@ var assert = require('assert'); var WebWorker = require('./web_worker'); +var browser = require('./browser'); module.exports = WorkerPool; +WorkerPool.WORKER_COUNT = Math.max(browser.hardwareConcurrency - 1, 1); + function WorkerPool() { - this.workers = []; + this.workerCount = WorkerPool.WORKER_COUNT; this.active = {}; } WorkerPool.prototype = { - acquire: function (mapId, workerCount) { - this._resize(workerCount); - this.active[mapId] = workerCount; - return this.workers.slice(0, workerCount); + acquire: function (mapId) { + if (!this.workers) { + assert(typeof this.workerCount === 'number'); + this.workers = []; + while (this.workers.length < this.workerCount) { + this.workers.push(new WebWorker()); + } + } + + this.active[mapId] = true; + return this.workers.slice(); }, 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()); + this.workers = null; } } }; diff --git a/test/js/style/style.test.js b/test/js/style/style.test.js index d36dfc47e20..cb44c3b1cb2 100644 --- a/test/js/style/style.test.js +++ b/test/js/style/style.test.js @@ -1232,10 +1232,3 @@ test('Style#addSourceType', function (t) { t.end(); }); - -test('Style creates correct number of workers', function(t) { - var style = new Style(createStyleJSON(), null, 3); - t.equal(style.dispatcher.actors.length, 3); - t.ok(style); - t.end(); -}); diff --git a/test/js/ui/map.test.js b/test/js/ui/map.test.js index abc2d49ee2c..407688f2621 100755 --- a/test/js/ui/map.test.js +++ b/test/js/ui/map.test.js @@ -6,7 +6,6 @@ var window = require('../../../js/util/browser').window; var Map = require('../../../js/ui/map'); var Style = require('../../../js/style/style'); var LngLat = require('../../../js/geo/lng_lat'); -var browser = require('../../../js/util/browser'); var sinon = require('sinon'); var fixed = require('../../testutil/fixed'); @@ -993,17 +992,6 @@ test('Map', function(t) { t.end(); }); - t.test('workerCount option', function(t) { - var map = createMap({ style: createStyle() }); - t.equal(map.style.dispatcher.actors.length, browser.hardwareConcurrency - 1, 'workerCount defaults to hardwareConcurrency - 1'); - map = createMap({ style: createStyle(), workerCount: 3 }); - t.equal(map.style.dispatcher.actors.length, 3, 'workerCount option is used'); - t.throws(function () { - createMap({ workerCount: 0 }); - }); - t.end(); - }); - test('render stabilizes', function (t) { var style = createStyle(); style.sources.mapbox = { diff --git a/test/js/util/dispatcher.test.js b/test/js/util/dispatcher.test.js index bb71257c661..328ce02a01a 100644 --- a/test/js/util/dispatcher.test.js +++ b/test/js/util/dispatcher.test.js @@ -13,8 +13,7 @@ test('Dispatcher', function (t) { var releaseCalled = []; var workerPool = { - acquire: function (id, count) { - t.equal(count, 2); + acquire: function () { return workers; }, release: function (id) { @@ -22,7 +21,7 @@ test('Dispatcher', function (t) { } }; - dispatcher = new Dispatcher(workerPool, 2, {}); + dispatcher = new Dispatcher(workerPool, {}); t.same(dispatcher.actors.map(function (actor) { return actor.target; }), workers); dispatcher.remove(); t.equal(dispatcher.actors.length, 0, 'actors discarded'); @@ -37,10 +36,14 @@ test('Dispatcher', function (t) { var ids = []; function Actor (target, parent, mapId) { ids.push(mapId); } + var previousWorkerCount = WorkerPool.WORKER_COUNT; + WorkerPool.WORKER_COUNT = 1; + var workerPool = new WorkerPool(); - var dispatchers = [new Dispatcher(workerPool, 1, {}), new Dispatcher(workerPool, 1, {})]; + var dispatchers = [new Dispatcher(workerPool, {}), new Dispatcher(workerPool, {})]; t.same(ids, dispatchers.map(function (d) { return d.id; })); + WorkerPool.WORKER_COUNT = previousWorkerCount; t.end(); }); diff --git a/test/js/util/worker_pool.test.js b/test/js/util/worker_pool.test.js index f1f085d677c..a1a0a9bb8ee 100644 --- a/test/js/util/worker_pool.test.js +++ b/test/js/util/worker_pool.test.js @@ -1,19 +1,31 @@ 'use strict'; var test = require('tap').test; +var proxyquire = require('proxyquire'); var WorkerPool = require('../../../js/util/worker_pool'); test('WorkerPool', function (t) { + t.test('.WORKER_COUNT', function (t) { + var WorkerPool = proxyquire('../../../js/util/worker_pool', { + './browser': { hardwareConcurrency: 15 } + }); + t.equal(WorkerPool.WORKER_COUNT, 14); + + WorkerPool.WORKER_COUNT = 4; + t.end(); + }); + t.test('#acquire', function (t) { - var pool = new WorkerPool(); + // make sure we're actually creating some workers + t.ok(WorkerPool.WORKER_COUNT > 0); - t.equal(pool.workers.length, 0); - var workers1 = pool.acquire('map-1', 4); - t.equal(workers1.length, 4); + var pool = new WorkerPool(); - var workers2 = pool.acquire('map-2', 8); - t.equal(workers2.length, 8); - t.equal(pool.workers.length, 8); + t.notOk(pool.workers); + var workers1 = pool.acquire('map-1'); + var workers2 = pool.acquire('map-2'); + t.equal(workers1.length, WorkerPool.WORKER_COUNT); + t.equal(workers2.length, WorkerPool.WORKER_COUNT); // check that the two different dispatchers' workers arrays correspond workers1.forEach(function (w, i) { t.equal(w, workers2[i]); }); @@ -22,8 +34,8 @@ test('WorkerPool', function (t) { t.test('#release', function (t) { var pool = new WorkerPool(); - pool.acquire('map-1', 1); - var workers = pool.acquire('map-2', 4); + pool.acquire('map-1'); + var workers = pool.acquire('map-2'); var terminated = 0; workers.forEach(function (w) { w.terminate = function () { terminated += 1; }; @@ -32,12 +44,12 @@ test('WorkerPool', function (t) { 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.ok(pool.workers.length > 0); t.comment('terminates workers if no dispatchers are active'); pool.release('map-1'); - t.equal(terminated, 4); - t.equal(pool.workers.length, 0); + t.equal(terminated, WorkerPool.WORKER_COUNT); + t.notOk(pool.workers); t.end(); });