From 198fb97a4971052308bc90d8217f0a087e1be5f6 Mon Sep 17 00:00:00 2001 From: Anand Thakker Date: Thu, 2 Jun 2016 14:23:15 -0400 Subject: [PATCH 1/2] Add workerCount option Closes #2022 --- js/style/style.js | 4 ++-- js/ui/map.js | 12 ++++++++++-- test/js/style/style.test.js | 5 ++--- test/js/ui/map.test.js | 16 ++++++++++++++++ 4 files changed, 30 insertions(+), 7 deletions(-) diff --git a/js/style/style.js b/js/style/style.js index fa6ec4fc9a5..97422ad8b2b 100644 --- a/js/style/style.js +++ b/js/style/style.js @@ -19,9 +19,9 @@ var StyleFunction = require('./style_function'); module.exports = Style; -function Style(stylesheet, animationLoop) { +function Style(stylesheet, animationLoop, workerCount) { this.animationLoop = animationLoop || new AnimationLoop(); - this.dispatcher = new Dispatcher(Math.max(browser.hardwareConcurrency - 1, 1), this); + this.dispatcher = new Dispatcher(workerCount || 1, this); this.spriteAtlas = new SpriteAtlas(512, 512); this.lineAtlas = new LineAtlas(256, 512); diff --git a/js/ui/map.js b/js/ui/map.js index 730ad15c36e..b18180f2e26 100755 --- a/js/ui/map.js +++ b/js/ui/map.js @@ -52,7 +52,8 @@ var defaultOptions = { failIfMajorPerformanceCaveat: false, preserveDrawingBuffer: false, - trackResize: true + trackResize: true, + workerCount: Math.max(browser.hardwareConcurrency - 1, 1) }; /** @@ -97,6 +98,7 @@ var defaultOptions = { * @param {number} [options.zoom] The zoom level of the map's initial viewport. * @param {number} [options.bearing] The bearing (rotation) of the map's initial viewport measured in degrees counter-clockwise from north. * @param {number} [options.pitch] The pitch of the map's initial viewport measured in degrees. + * @param {number} [options.workerCount=navigator.hardwareConcurrency] The number of WebWorkers the map should use to process vector tile data. * @example * var map = new mapboxgl.Map({ * container: 'map', @@ -109,10 +111,16 @@ var defaultOptions = { 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; if (typeof options.container === 'string') { this._container = document.getElementById(options.container); @@ -586,7 +594,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.style = new Style(style, this.animationLoop, this._workerCount); } this.style diff --git a/test/js/style/style.test.js b/test/js/style/style.test.js index 26920b3d333..3aab22335b0 100644 --- a/test/js/style/style.test.js +++ b/test/js/style/style.test.js @@ -9,7 +9,6 @@ var Style = require('../../../js/style/style'); var VectorTileSource = require('../../../js/source/vector_tile_source'); var StyleLayer = require('../../../js/style/style_layer'); var util = require('../../../js/util/util'); -var browser = require('../../../js/util/browser'); function createStyleJSON(properties) { return util.extend({ @@ -1123,8 +1122,8 @@ test('Style#query*Features', function(t) { }); test('Style creates correct number of workers', function(t) { - var style = new Style(createStyleJSON()); - t.equal(style.dispatcher.actors.length, browser.hardwareConcurrency - 1); + 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 244f6b71e5c..6307e46b228 100755 --- a/test/js/ui/map.test.js +++ b/test/js/ui/map.test.js @@ -6,6 +6,7 @@ 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'); @@ -930,6 +931,21 @@ test('Map', function(t) { t.end(); }); + t.test('workerCount option', function(t) { + var map = createMap({}); + // TODO: it's not great to check this private member here; better would + // be to mock Style and then just check that the correct workerCount + // param is being passed. + t.equal(map._workerCount, browser.hardwareConcurrency - 1, 'workerCount defaults to hardwareConcurrency - 1'); + map = createMap({ workerCount: 3 }); + t.equal(map._workerCount, 3, 'workerCount option is used'); + t.throws(function () { + createMap({ workerCount: 0 }); + }); + t.end(); + }); + + t.end(); }); From a97a10cc33dfa3363a71b9653623f0b009d26d85 Mon Sep 17 00:00:00 2001 From: Anand Thakker Date: Tue, 7 Jun 2016 07:56:00 -0400 Subject: [PATCH 2/2] Address PR comments --- js/ui/map.js | 2 +- test/js/ui/map.test.js | 11 ++++------- 2 files changed, 5 insertions(+), 8 deletions(-) diff --git a/js/ui/map.js b/js/ui/map.js index b18180f2e26..4fed846a9e3 100755 --- a/js/ui/map.js +++ b/js/ui/map.js @@ -98,7 +98,7 @@ var defaultOptions = { * @param {number} [options.zoom] The zoom level of the map's initial viewport. * @param {number} [options.bearing] The bearing (rotation) of the map's initial viewport measured in degrees counter-clockwise from north. * @param {number} [options.pitch] The pitch of the map's initial viewport measured in degrees. - * @param {number} [options.workerCount=navigator.hardwareConcurrency] The number of WebWorkers the map should use to process vector tile data. + * @param {number} [options.workerCount=navigator.hardwareConcurrency - 1] The number of WebWorkers the map should use to process vector tile data. * @example * var map = new mapboxgl.Map({ * container: 'map', diff --git a/test/js/ui/map.test.js b/test/js/ui/map.test.js index 6307e46b228..345a320999c 100755 --- a/test/js/ui/map.test.js +++ b/test/js/ui/map.test.js @@ -932,13 +932,10 @@ test('Map', function(t) { }); t.test('workerCount option', function(t) { - var map = createMap({}); - // TODO: it's not great to check this private member here; better would - // be to mock Style and then just check that the correct workerCount - // param is being passed. - t.equal(map._workerCount, browser.hardwareConcurrency - 1, 'workerCount defaults to hardwareConcurrency - 1'); - map = createMap({ workerCount: 3 }); - t.equal(map._workerCount, 3, 'workerCount option is used'); + 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 }); });