Skip to content

Commit

Permalink
Batch mode for efficiently mutating Style instances
Browse files Browse the repository at this point in the history
Mutation operations on a Style instance are designed to be atomic.
Depending on the operation, there is a significant amount of work that
must be preformed to flush and recreate state. This work is wasted when
multiple mutation methods are invokes sequentially.

Batching these mutations allows deferring this work until the batch is
complete and deduping the work.

    style.batch(function() {
        style.addLayer(layer1);
        style.addLayer(layer2);
        ...
        style.addLayer(layerN);
    });

Internally, calls to four methods are deferred:
- style._groupLayers (will only fire once)
- style._broadcastLayers (will only fire once)
- style._reloadSource (will fire once per source)
- style.fire (each call will fire)

A casual benchmark shows the time to add 80 layers dropping from ~800 ms
to ~20 ms when run as a batch.

Issue: #1341
  • Loading branch information
scothis committed Jun 29, 2015
1 parent d33c899 commit 819de3e
Show file tree
Hide file tree
Showing 3 changed files with 184 additions and 3 deletions.
79 changes: 76 additions & 3 deletions js/style/style.js
Original file line number Diff line number Diff line change
Expand Up @@ -216,6 +216,79 @@ Style.prototype = util.inherit(Evented, {
zh.lastZoom = z;
},

_reloadSource: function (sourceId) {
this.sources[sourceId].reload();
},

/**
* Apply multiple style mutations effciently
* @param {function} work function which performs mutations
* @private
*/
batch: function(work) {
if (!this._loaded) {
throw new Error('Style is not done loading');
}

var hoistedMethods = {};
var buffer = {};

var restoreMethods = function() {
Object.keys(hoistedMethods).forEach(function(func) {
this[func] = hoistedMethods[func];
}, this);
}.bind(this);

// buffer fired events
buffer.fire = [];
hoistedMethods.fire = this.fire;
this.fire = function() {
buffer.fire.push(arguments);
};

// capture _groupLayers calls
hoistedMethods._groupLayers = this._groupLayers;
this._groupLayers = function() {
buffer._groupLayers = true;
};

// capture _broadcastLayers calls
hoistedMethods._broadcastLayers = this._broadcastLayers;
this._broadcastLayers = function() {
buffer._broadcastLayers = true;
};

// capture _reloadSource calls, per source
buffer._reloadSource = {};
hoistedMethods._reloadSource = this._reloadSource;
this._reloadSource = function(sourceId) {
buffer._reloadSource[sourceId] = true;
};

try {
work(this);

restoreMethods();

// call once if called
if (buffer._groupLayers) this._groupLayers();
if (buffer._broadcastLayers) this._broadcastLayers();

// reload sources
Object.keys(buffer._reloadSource).forEach(function(sourceId) {
this._reloadSource(sourceId);
}, this);

// re-fire events
buffer.fire.forEach(function(args) {
this.fire.apply(this, args);
}, this);
} catch (e) {
restoreMethods();
throw e;
}
},

addSource: function(id, source) {
if (!this._loaded) {
throw new Error('Style is not done loading');
Expand Down Expand Up @@ -311,7 +384,7 @@ Style.prototype = util.inherit(Evented, {
this._groupLayers();
this._broadcastLayers();
if (layer.source) {
this.sources[layer.source].reload();
this._reloadSource(layer.source);
}
this.fire('layer.add', {layer: layer});
return this;
Expand Down Expand Up @@ -381,7 +454,7 @@ Style.prototype = util.inherit(Evented, {
layer.filter = filter;
this.stylesheet.layers[this._order.indexOf(layer.id)].filter = filter;
this._broadcastLayers();
this.sources[layer.source].reload();
this._reloadSource(layer.source);
this.fire('change');
},

Expand All @@ -406,7 +479,7 @@ Style.prototype = util.inherit(Evented, {
styleLayer.layout[name] = value;
this._broadcastLayers();
if (layer.source) {
this.sources[layer.source].reload();
this._reloadSource(layer.source);
}
this.fire('change');
},
Expand Down
1 change: 1 addition & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@
"mapbox-gl-test-suite": "git+https://github.com/mapbox/mapbox-gl-test-suite.git#632163906f90883c611d09f57a5c5b988d1e923f",
"mkdirp": "^0.5.1",
"prova": "^2.1.2",
"sinon": "^1.15.4",
"st": "^0.5.4",
"through": "^2.3.7",
"watchify": "^3.2.2"
Expand Down
107 changes: 107 additions & 0 deletions test/js/style/style.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ var test = require('prova');
var st = require('st');
var http = require('http');
var path = require('path');
var sinon = require('sinon');
var Style = require('../../../js/style/style');
var VectorTileSource = require('../../../js/source/vector_tile_source');
var LayoutProperties = require('../../../js/style/layout_properties');
Expand Down Expand Up @@ -1097,3 +1098,109 @@ test('Style#featuresAt', function(t) {
t.end();
});
});

test('Style#batch', function(t) {
t.test('hoists and replaces methods', function(t) {
var style = new Style(createStyleJSON());
var methods = {
fire: style.fire,
_groupLayers: style._groupLayers,
_broadcastLayers: style._broadcastLayers,
_reloadSource: style._reloadSource
};
style.on('load', function() {
style.batch(function() {
t.notEqual(style.fire, methods.fire, 'surrogate method');
t.notEqual(style._groupLayers, methods._groupLayers, 'surrogate method');
t.notEqual(style._broadcastLayers, methods._broadcastLayers, 'surrogate method');
t.notEqual(style._reloadSource, methods._reloadSource, 'surrogate method');
});

t.equal(style.fire, methods.fire, 'original method');
t.equal(style._groupLayers, methods._groupLayers, 'original method');
t.equal(style._broadcastLayers, methods._broadcastLayers, 'original method');
t.equal(style._reloadSource, methods._reloadSource, 'original method');

t.end();
});
});

t.test('hoists and replaces methods after error', function(t) {
var style = new Style(createStyleJSON());
var methods = {
fire: style.fire,
_groupLayers: style._groupLayers,
_broadcastLayers: style._broadcastLayers,
_reloadSource: style._reloadSource
};
style.on('load', function() {
var error = new Error('Must recover');

t.throws(function() {
style.batch(function() {
throw error;
});
}, error, 'same error as thrown');

t.equal(style.fire, methods.fire, 'original method');
t.equal(style._groupLayers, methods._groupLayers, 'original method');
t.equal(style._broadcastLayers, methods._broadcastLayers, 'original method');
t.equal(style._reloadSource, methods._reloadSource, 'original method');

t.end();
});
});

t.test('defers expensive methods', function(t) {
var style = new Style(createStyleJSON({
"sources": {
"streets": createGeoJSONSourceJSON(),
"terrain": createGeoJSONSourceJSON()
}
}));

style.on('load', function() {
// spies to track defered methods
sinon.spy(style, 'fire');
sinon.spy(style, '_reloadSource');
sinon.spy(style, '_broadcastLayers');
sinon.spy(style, '_groupLayers');

style.batch(function(s) {
s.addLayer({ id: 'first', type: 'symbol', source: 'streets' });
s.addLayer({ id: 'second', type: 'symbol', source: 'streets' });
s.addLayer({ id: 'third', type: 'symbol', source: 'terrain' });

t.notOk(style.fire.called, 'fire is deferred');
t.notOk(style._reloadSource.called, '_reloadSource is deferred');
t.notOk(style._broadcastLayers.called, '_broadcastLayers is deferred');
t.notOk(style._groupLayers.called, '_groupLayers is deferred');
});

// called per added layer
t.ok(style.fire.calledThrice, 'fire is called per action');
t.ok(style.fire.calledWith('layer.add'), 'fire was called with layer.add');

// called per source
t.ok(style._reloadSource.calledTwice, '_reloadSource is called per source');
t.ok(style._reloadSource.calledWith('streets'), '_reloadSource is called for streets');
t.ok(style._reloadSource.calledWith('terrain'), '_reloadSource is called for terrain');

// called once
t.ok(style._broadcastLayers.calledOnce, '_broadcastLayers is called once');
t.ok(style._groupLayers.calledOnce, '_groupLayers is called once');

t.end();
});
});

t.test('throw before loaded', function(t) {
var style = new Style(createStyleJSON());
t.throws(function() {
style.batch(function() {});
}, Error, /load/i);
style.on('load', function() {
t.end();
});
});
});

0 comments on commit 819de3e

Please sign in to comment.