Skip to content

Commit

Permalink
Use a fixed worker pool size
Browse files Browse the repository at this point in the history
  • Loading branch information
Anand Thakker committed Aug 19, 2016
1 parent bb84534 commit f21dced
Show file tree
Hide file tree
Showing 10 changed files with 72 additions and 64 deletions.
16 changes: 16 additions & 0 deletions js/global_worker_pool.js
Original file line number Diff line number Diff line change
@@ -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;
};
2 changes: 2 additions & 0 deletions js/mapbox-gl.js
Original file line number Diff line number Diff line change
Expand Up @@ -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');
Expand Down
8 changes: 3 additions & 5 deletions js/style/style.js
Original file line number Diff line number Diff line change
Expand Up @@ -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);

Expand Down
11 changes: 2 additions & 9 deletions js/ui/map.js
Original file line number Diff line number Diff line change
Expand Up @@ -52,8 +52,7 @@ var defaultOptions = {
failIfMajorPerformanceCaveat: false,
preserveDrawingBuffer: false,

trackResize: true,
workerCount: Math.max(browser.hardwareConcurrency - 1, 1)
trackResize: true
};

/**
Expand Down Expand Up @@ -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',
Expand All @@ -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') {
Expand Down Expand Up @@ -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
Expand Down
4 changes: 2 additions & 2 deletions js/util/dispatcher.js
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
29 changes: 16 additions & 13 deletions js/util/worker_pool.js
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
}
};
Expand Down
7 changes: 0 additions & 7 deletions test/js/style/style.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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();
});
12 changes: 0 additions & 12 deletions test/js/ui/map.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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');
Expand Down Expand Up @@ -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 = {
Expand Down
11 changes: 7 additions & 4 deletions test/js/util/dispatcher.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -13,16 +13,15 @@ test('Dispatcher', function (t) {

var releaseCalled = [];
var workerPool = {
acquire: function (id, count) {
t.equal(count, 2);
acquire: function () {
return workers;
},
release: function (id) {
releaseCalled.push(id);
}
};

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');
Expand All @@ -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();
});

Expand Down
36 changes: 24 additions & 12 deletions test/js/util/worker_pool.test.js
Original file line number Diff line number Diff line change
@@ -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]); });
Expand All @@ -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; };
Expand All @@ -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();
});
Expand Down

0 comments on commit f21dced

Please sign in to comment.