From e4063102b7d13d3f2cc1bd02f2249e3d9deaf6c2 Mon Sep 17 00:00:00 2001 From: Guillaume Beraudo Date: Fri, 5 Dec 2014 14:29:41 +0100 Subject: [PATCH 1/4] Potentialy faster array allocation in rbush --- src/ol/structs/rbush.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/ol/structs/rbush.js b/src/ol/structs/rbush.js index baa0e3a1f1..4de21cbc1f 100644 --- a/src/ol/structs/rbush.js +++ b/src/ol/structs/rbush.js @@ -74,7 +74,7 @@ ol.structs.RBush.prototype.load = function(extents, values) { } goog.asserts.assert(extents.length === values.length); - var items = []; + var items = new Array(values.length); for (var i = 0, l = values.length; i < l; i++) { var extent = extents[i]; var value = values[i]; @@ -86,7 +86,7 @@ ol.structs.RBush.prototype.load = function(extents, values) { extent[3], value ]; - items.push(item); + items[i] = item; goog.object.add(this.items_, goog.getUid(value).toString(), item); } this.rbush_.load(items); From 3e2cc3c2463061e4d543978d4ae34eb8e2c62e57 Mon Sep 17 00:00:00 2001 From: Guillaume Beraudo Date: Wed, 3 Dec 2014 14:15:46 +0100 Subject: [PATCH 2/4] Faster vector source clear Three seconds speed up for clearing 100'000 features. --- src/ol/structs/rbush.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/ol/structs/rbush.js b/src/ol/structs/rbush.js index 4de21cbc1f..a9c5cce5d9 100644 --- a/src/ol/structs/rbush.js +++ b/src/ol/structs/rbush.js @@ -228,7 +228,7 @@ ol.structs.RBush.prototype.isEmpty = function() { */ ol.structs.RBush.prototype.clear = function() { this.rbush_.clear(); - goog.object.clear(this.items_); + this.items_ = {}; }; From 17e56d83573740be6fc78a251b8cff04452f78b7 Mon Sep 17 00:00:00 2001 From: Guillaume Beraudo Date: Fri, 5 Dec 2014 14:55:46 +0100 Subject: [PATCH 3/4] Introduce clear event on vector source Three seconds speed up when clearing 100'000 features. Clearing is now around 350ms. --- src/ol/source/vectorsource.js | 25 ++++++++++++++++++------ test/spec/ol/source/vectorsource.test.js | 8 ++++++-- 2 files changed, 25 insertions(+), 8 deletions(-) diff --git a/src/ol/source/vectorsource.js b/src/ol/source/vectorsource.js index 6b223026a0..7551565165 100644 --- a/src/ol/source/vectorsource.js +++ b/src/ol/source/vectorsource.js @@ -37,7 +37,14 @@ ol.source.VectorEventType = { CHANGEFEATURE: 'changefeature', /** - * Triggered when a feature is removed from the source. + * Triggered when a clear is called on the source. + * @event ol.source.VectorEvent#clear + * @api + */ + CLEAR: 'clear', + + /** + * Triggered when a single feature is removed from the source. * @event ol.source.VectorEvent#removefeature * @api stable */ @@ -230,12 +237,18 @@ ol.source.Vector.prototype.addFeaturesInternal = function(features) { * @api stable */ ol.source.Vector.prototype.clear = function() { - this.rBush_.forEach(this.removeFeatureInternal, this); + for (var featureId in this.featureChangeKeys_) { + var keys = this.featureChangeKeys_[featureId]; + goog.array.forEach(keys, goog.events.unlistenByKey); + } + this.featureChangeKeys_ = {}; + this.idIndex_ = {}; + this.undefIdIndex_ = {}; this.rBush_.clear(); - goog.object.forEach( - this.nullGeometryFeatures_, this.removeFeatureInternal, this); - goog.object.clear(this.nullGeometryFeatures_); - goog.asserts.assert(goog.object.isEmpty(this.featureChangeKeys_)); + this.nullGeometryFeatures_ = {}; + + var clearEvent = new ol.source.VectorEvent(ol.source.VectorEventType.CLEAR); + this.dispatchEvent(clearEvent); this.changed(); }; diff --git a/test/spec/ol/source/vectorsource.test.js b/test/spec/ol/source/vectorsource.test.js index 1ec5803a2c..e632efae11 100644 --- a/test/spec/ol/source/vectorsource.test.js +++ b/test/spec/ol/source/vectorsource.test.js @@ -90,13 +90,17 @@ describe('ol.source.Vector', function() { goog.events.listen(vectorSource, 'change', changeSpy); var removeFeatureSpy = sinon.spy(); goog.events.listen(vectorSource, 'removefeature', removeFeatureSpy); + var clearSourceSpy = sinon.spy(); + goog.events.listen(vectorSource, 'clear', clearSourceSpy); vectorSource.clear(); expect(vectorSource.getFeatures()).to.eql([]); expect(vectorSource.isEmpty()).to.be(true); expect(changeSpy).to.be.called(); expect(changeSpy.callCount).to.be(1); - expect(removeFeatureSpy).to.be.called(); - expect(removeFeatureSpy.callCount).to.be(features.length); + expect(removeFeatureSpy).not.to.be.called(); + expect(removeFeatureSpy.callCount).to.be(0); + expect(clearSourceSpy).to.be.called(); + expect(clearSourceSpy.callCount).to.be(1); }); }); From e3947fb09a95feda36543d0dd3666f73a23402d8 Mon Sep 17 00:00:00 2001 From: Guillaume Beraudo Date: Fri, 5 Dec 2014 16:38:00 +0100 Subject: [PATCH 4/4] Add optional fast parameter for clearing vector source --- src/ol/source/vectorsource.js | 30 +++++++++++++++++------- test/spec/ol/source/vectorsource.test.js | 24 ++++++++++++++++--- 2 files changed, 42 insertions(+), 12 deletions(-) diff --git a/src/ol/source/vectorsource.js b/src/ol/source/vectorsource.js index 7551565165..de1783e130 100644 --- a/src/ol/source/vectorsource.js +++ b/src/ol/source/vectorsource.js @@ -37,14 +37,15 @@ ol.source.VectorEventType = { CHANGEFEATURE: 'changefeature', /** - * Triggered when a clear is called on the source. + * Triggered when the clear method is called on the source. * @event ol.source.VectorEvent#clear * @api */ CLEAR: 'clear', /** - * Triggered when a single feature is removed from the source. + * Triggered when a feature is removed from the source. + * See {@link ol.source.Vector#clear source.clear()} for exceptions. * @event ol.source.VectorEvent#removefeature * @api stable */ @@ -234,16 +235,27 @@ ol.source.Vector.prototype.addFeaturesInternal = function(features) { /** * Remove all features from the source. + * @param {boolean=} opt_fast Skip dispatching of {@link removefeature} events. * @api stable */ -ol.source.Vector.prototype.clear = function() { - for (var featureId in this.featureChangeKeys_) { - var keys = this.featureChangeKeys_[featureId]; - goog.array.forEach(keys, goog.events.unlistenByKey); +ol.source.Vector.prototype.clear = function(opt_fast) { + if (opt_fast) { + for (var featureId in this.featureChangeKeys_) { + var keys = this.featureChangeKeys_[featureId]; + goog.array.forEach(keys, goog.events.unlistenByKey); + } + this.featureChangeKeys_ = {}; + this.idIndex_ = {}; + this.undefIdIndex_ = {}; + } else { + var rmFeatureInternal = this.removeFeatureInternal; + this.rBush_.forEach(rmFeatureInternal, this); + goog.object.forEach(this.nullGeometryFeatures_, rmFeatureInternal, this); + goog.asserts.assert(goog.object.isEmpty(this.featureChangeKeys_)); + goog.asserts.assert(goog.object.isEmpty(this.idIndex_)); + goog.asserts.assert(goog.object.isEmpty(this.undefIdIndex_)); } - this.featureChangeKeys_ = {}; - this.idIndex_ = {}; - this.undefIdIndex_ = {}; + this.rBush_.clear(); this.nullGeometryFeatures_ = {}; diff --git a/test/spec/ol/source/vectorsource.test.js b/test/spec/ol/source/vectorsource.test.js index e632efae11..558cd725e8 100644 --- a/test/spec/ol/source/vectorsource.test.js +++ b/test/spec/ol/source/vectorsource.test.js @@ -85,7 +85,25 @@ describe('ol.source.Vector', function() { describe('#clear', function() { - it('removes all features', function() { + it('removes all features using fast path', function() { + var changeSpy = sinon.spy(); + goog.events.listen(vectorSource, 'change', changeSpy); + var removeFeatureSpy = sinon.spy(); + goog.events.listen(vectorSource, 'removefeature', removeFeatureSpy); + var clearSourceSpy = sinon.spy(); + goog.events.listen(vectorSource, 'clear', clearSourceSpy); + vectorSource.clear(true); + expect(vectorSource.getFeatures()).to.eql([]); + expect(vectorSource.isEmpty()).to.be(true); + expect(changeSpy).to.be.called(); + expect(changeSpy.callCount).to.be(1); + expect(removeFeatureSpy).not.to.be.called(); + expect(removeFeatureSpy.callCount).to.be(0); + expect(clearSourceSpy).to.be.called(); + expect(clearSourceSpy.callCount).to.be(1); + }); + + it('removes all features using slow path', function() { var changeSpy = sinon.spy(); goog.events.listen(vectorSource, 'change', changeSpy); var removeFeatureSpy = sinon.spy(); @@ -97,8 +115,8 @@ describe('ol.source.Vector', function() { expect(vectorSource.isEmpty()).to.be(true); expect(changeSpy).to.be.called(); expect(changeSpy.callCount).to.be(1); - expect(removeFeatureSpy).not.to.be.called(); - expect(removeFeatureSpy.callCount).to.be(0); + expect(removeFeatureSpy).to.be.called(); + expect(removeFeatureSpy.callCount).to.be(features.length); expect(clearSourceSpy).to.be.called(); expect(clearSourceSpy.callCount).to.be(1); });