controller.update always called twice

[benmccann]

In src/core/core.controller.js: we have some code that updates each controller twice. Both the call to helpers.each and updateDatasets below end up calling controller.update on every controller.

// Can only reset the new controllers after the scales have been updated
helpers.each(newControllers, function(controller) {
	controller.reset();
});

me.updateDatasets();

The source for controller.reset calls controller.update:

reset: function() {
	this.update(true);
},

And updateDatasets calls updateDataset which calls controller.update:

updateDatasets: function() {
	var me = this;

	if (plugins.notify(me, 'beforeDatasetsUpdate') === false) {
		return;
	}

	for (var i = 0, ilen = me.data.datasets.length; i < ilen; ++i) {
		me.updateDataset(i);
	}

	plugins.notify(me, 'afterDatasetsUpdate');
},
updateDataset: function(index) {
	var me = this;
	var meta = me.getDatasetMeta(index);
	var args = {
		meta: meta,
		index: index
	};

	if (plugins.notify(me, 'beforeDatasetUpdate', [args]) === false) {
		return;
	}

	meta.controller.update();

	plugins.notify(me, 'afterDatasetUpdate', [args]);
},

So we end up calling controller.update twice in a row every time we update a chart, which seems like a fairly expensive operation to duplicate. I'm wondering if we can improve performance by removing one of these.

[etimberg]

Part of why this happens is for animations.

// Can only reset the new controllers after the scales have been updated
helpers.each(newControllers, function(controller) {
	controller.reset();
});

this only happens for newly added controllers (aka datasets). If the datasets do not change then this should be a no-op. We need to reset new controllers so that the first animation of those datasets has something to animate from.

When a chart is created, we call update twice for every controller as they are all new. To make this faster, some optimizations are:

1. remove helpers.each calls with regular for loops
2. Have a lighter weight way of resetting for animation though what we have should be fairly light
3. Reset all new controllers in buildOrUpdateControllers while they're being created to avoid an extra loop

[benmccann]

The part I'm worried about is that we calculate the location of every point, bar, etc. on the chart twice to show a static chart. Most of my charts have thousands of data points, so this seems like it could be a pretty substantial amount of extra work?

[etimberg]

@benmccann you're right. We can optimize away the reset if the chart is not animating.

[benmccann]

Could I just do something like this?

if (options.animation.duration) {
	// Can only reset the new controllers after the scales have been updated
	helpers.each(newControllers, function(controller) {
		controller.reset();
	});
}

[etimberg]

yup, that'll work :)