Enable data decimation plugins by separating parsed data into two arrays #8251

etimberg · 2020-12-29T22:09:23Z

Enables plugins via a new hook afterDataParse.

To Do

Ensure existing unit tests pass
Add test coverage
Documentation
Typescript definitions

Testing

I tested this on the uPlot benchmark over 100 runs. It performs noticeably better than the mainline code

master

100 runs done in 5717ms. Average: 57ms, min: 41ms, max: 175ms, variation: 134ms.

PR + decimation plugin

100 runs done in 3027ms. Average: 30ms, min: 22ms, max: 177ms, variation: 155ms.

The plugin i used is below.

{
	id: 'decimate-data',
	afterDataParse: (chart, args) => {
		const { indexScale, parsed } = args;
		let decimated = [];
		let i, point, x, y, prevX, minIndex, maxIndex, minY, maxY;

		for (i = 0; i < parsed.length; ++i) {
			point = parsed[i];
			x = indexScale.getPixelForValue(point[indexScale.axis]);
			y = point[indexScale.axis === 'x' ? 'y' : 'x'];
			const truncX = x | 0;

			if (truncX === prevX) {
				// Determine `minY` / `maxY` and `avgX` while we stay within same x-position
				if (y < minY) {
					minY = y;
					minIndex = i;
				} else if (y > maxY) {
					maxY = y;
					maxIndex = i;
				}
			} else {
				// Push up to 4 points, 3 for the last interval and the first point for this interval
				if (minIndex && maxIndex) {
					decimated.push(parsed[minIndex], parsed[maxIndex]);
				}
				if (i > 0) {
					// Last point in the previous interval
					decimated.push(parsed[i - 1]);
				}
				decimated.push(point);
				prevX = truncX;
				minY = maxY = y;
				minIndex = maxIndex = i;
			}
		}

		args.data = decimated;
	}
};

kurkle · 2020-12-29T22:32:42Z

src/core/core.datasetController.js

+		};
+		me.chart.notifyPlugins('afterDataParse', args);
+
+		if (!isNullOrUndef(args.data)) {


I'd reverse the cases to avoid the !

kurkle · 2020-12-29T22:36:35Z

src/core/core.datasetController.js

@@ -591,7 +614,8 @@ export default class DatasetController {
 	getMinMax(scale, canStack) {
 		const me = this;
 		const meta = me._cachedMeta;
-		const _parsed = meta._parsed;
+		// Use filtered parsed elements if the exist, otherwise the full elements


isn't the correct array always available in the meta._parsed?

I'm storing the full array in meta._parsedRaw and the filtered one in meta._parsed

if there is not filtered array, meta._parsed === meta._parsedRaw?

kurkle · 2020-12-29T22:42:23Z

src/core/core.datasetController.js


 		if (numData > numMeta) {
 			me._insertElements(numMeta, numData - numMeta);
 		} else if (numData < numMeta) {
 			me._removeElements(numData, numMeta - numData);
 		}
 		// Re-parse the old elements (new elements are parsed in _insertElements)
-		me.parse(0, Math.min(numData, numMeta));
+		// me.parse(0, Math.min(numData, numMeta));


I think we need to do the parsing first and resync the element counts after that. So compare _data.length to _parsedRaw.length and if that differs parse. then compare _parsed.length to numMeta and add the elements. need to remove the parsing from the _insertElements.

kurkle · 2020-12-29T22:44:44Z

src/core/core.datasetController.js

@@ -998,7 +1023,7 @@ export default class DatasetController {
 		data.splice(start, 0, ...elements);

 		if (me._parsing) {
-			meta._parsed.splice(start, 0, ...new Array(count));
+			meta._parsedRaw.splice(start, 0, ...new Array(count));


can't do the parsing here at all, because start and count refer to elements, no to the raw data.

Yeah, I'm realizing that this is causing a lot of tests to fail. A lot of code assumes these can be updated in sync at the same time.

I wonder if there is a simpler way to achieve this without touching the parsing, maybe even just a good guess at the size for estimating the number of points to keep

kurkle · 2020-12-29T22:45:54Z

src/core/core.datasetController.js

@@ -1014,7 +1039,7 @@ export default class DatasetController {
 		const me = this;
 		const meta = me._cachedMeta;
 		if (me._parsing) {
-			const removed = meta._parsed.splice(start, count);
+			const removed = meta._parsedRaw.splice(start, count);


this might work, but against _parsed

kurkle · 2020-12-29T22:48:35Z

src/controllers/controller.doughnut.js

@@ -60,7 +60,7 @@ export default class DoughnutController extends DatasetController {
 		const meta = this._cachedMeta;
 		let i, ilen;
 		for (i = start, ilen = start + count; i < ilen; ++i) {
-			meta._parsed[i] = +data[i];
+			meta._parsedRaw[i] = meta._parsed[i] = +data[i];


should do only one array and set the other to same instance after loop. we should make sure _parsedRaw is sufficient sized before parsing, so should set to that.

benmccann · 2020-12-30T16:21:55Z

src/core/core.controller.js

+			// to ensure that decimation plugins can take advantage of the
+			// scale dimensions
+			const controller = me.getDatasetMeta(i).controller;
+			controller._notifyAfterParse();


should we call this _notifyAfterLayout?

Edit: I see now why this was called _notifyAfterParse. I think it could be called _notifyAfterLayout if we were not setting data attribute in the args per the suggestion in https://github.com/chartjs/Chart.js/pull/8251/files#r550253989

benmccann · 2020-12-30T16:34:27Z

src/core/core.datasetController.js

+		const {_cachedMeta: meta} = me;
+		const {iScale} = meta;
+
+		// We expect afterDataParse hooks to set the modified data


Maybe it'd be cleaner to always set meta._parsed = meta._parsedRaw and then have the plugin override meta._parsed. We could expose a setParsed method if that's preferable to setting the value of private _parsed variable. We might have to pass chart as an arg so that we can get the meta

etimberg · 2021-01-23T18:56:53Z

Closing this since #8255 went a lot better

Enable data decimation plugins by separating parsed data into two arrays

39f6e75

etimberg added type: enhancement type: performance labels Dec 29, 2020

kurkle reviewed Dec 29, 2020

View reviewed changes

benmccann reviewed Dec 30, 2020

View reviewed changes

etimberg mentioned this pull request Dec 30, 2020

Create a new hook to enable data decimation #8255

Merged

etimberg closed this Jan 23, 2021

etimberg deleted the enable-decimation-plugins branch March 21, 2021 15:17

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Enable data decimation plugins by separating parsed data into two arrays #8251

Enable data decimation plugins by separating parsed data into two arrays #8251

etimberg commented Dec 29, 2020

kurkle Dec 29, 2020

kurkle Dec 29, 2020

etimberg Dec 29, 2020

kurkle Dec 29, 2020

kurkle Dec 29, 2020

kurkle Dec 29, 2020

etimberg Dec 29, 2020

kurkle Dec 29, 2020

kurkle Dec 29, 2020

benmccann Dec 30, 2020 •

edited

Loading

benmccann Dec 30, 2020 •

edited

Loading

etimberg commented Jan 23, 2021

Enable data decimation plugins by separating parsed data into two arrays #8251

Enable data decimation plugins by separating parsed data into two arrays #8251

Conversation

etimberg commented Dec 29, 2020

To Do

Testing

master

PR + decimation plugin

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

benmccann Dec 30, 2020 • edited Loading

Choose a reason for hiding this comment

benmccann Dec 30, 2020 • edited Loading

Choose a reason for hiding this comment

etimberg commented Jan 23, 2021

benmccann Dec 30, 2020 •

edited

Loading

benmccann Dec 30, 2020 •

edited

Loading