Create a new hook to enable data decimation

[etimberg]

The beforeElementsUpdate hook can be used to decimate data. The chart elements will not be created until after this hook has fired ensuring that if decimation occurs, only the needed elements will be created.

This is a simpler take on #8251 that does not need to reorder any passes. Instead, the decimation plugin uses a heuristic to determine the chart width.

Results

Test 1 (uPlot Benchmark, 100 runs)

	Master	PR
Average	53ms	20 ms
Min	37ms	13 ms
Max	175ms	137ms
Variation	138ms	124ms
Heap	36425.8 kB	21191.4 kB

Test 2 (10M data points, since run)

	Master	PR
Render Time	2954.39 ms	153.13 ms

Decimation Plugin

{
	id: 'decimate-data',
	beforeElementUpdate: (chart, args) => {
		// Use an approximation since this is before scales are parsed
		const verticalAxisCount = Object.values(chart.scales).reduce((acc, scale) => acc += scale.axis === 'y' ? 1 : 0, 0);

		// assume ~50px for now. If the axis is actually wider then we have more points than needed
		// TODO: Take display settings into account and compute a more accurate guess for each axis
		const availableWidth = chart.width - (verticalAxisCount * 50);

		chart.data.datasets.forEach((dataset, datasetIndex) => {
			let i, point, x, y, prevX, minIndex, maxIndex, minY, maxY;
			const {data} = dataset;
			const meta = chart.getDatasetMeta(datasetIndex);

			if (meta.type !== 'line') {
				// Only line datasets are supported
				return;
			}

			const xAxis = chart.scales[meta.xAxisID];
			if (xAxis.type !== 'linear' && xAxis.type != 'time') {
				// Only linear interpolation is supported
				return;
			}

			if (!!chart.options.parsing) {
				// Plugin only supports data that does not need parsing
				return;
			}

			const decimated = [];

			const xMin = data[0].x;
			const xMax = data[data.length - 1].x;
			const dx = xMax - xMin;

			for (i = 0; i < data.length; ++i) {
				point = data[i]
				x = (point.x - xMin) / dx * availableWidth;
				y = point.y;
				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(data[minIndex], data[maxIndex]);
					}
					if (i > 0) {
						// Last point in the previous interval
						decimated.push(data[i - 1]);
					}
					decimated.push(point);
					prevX = truncX;
					minY = maxY = y;
					minIndex = maxIndex = i;
				}
			}

			dataset._data = dataset.data;
			dataset.data = decimated;
		});
	}
};

[benmccann]

I'm not quite sure how to read the results. Does that mean it went from 2086ms to 1487ms + 153ms or just 153ms? Good improvement either way, but if the latter then wow!!

[benmccann]

I'm assuming if we had this new plugin, then we wouldn't get benefit from the existing line controller decimation anymore? I wonder if we removed the existing line controller decimation and added this general decimation plugin if we'd stay roughly equal in size, but come out ahead on speed and so maybe we should include this plugin in core in that case

[etimberg]

I've updated that to be clearer. It's two different tests. On the main benchmark it went, on average over 100 runs, from 53ms to 20ms. Rendering 10M points, rendering time dropped from 2954ms to 153ms

[etimberg]

I'm assuming if we had this new plugin, then we wouldn't get benefit from the existing line controller decimation anymore? I wonder if we removed the existing line controller decimation and added this general decimation plugin if we'd stay roughly equal in size, but come out ahead on speed and so maybe we should include this plugin in core in that case

@kurkle and I bounced that around too. It might be worth exploring since this also reduces memory overhead. That being said, without the plugin this is quite small.

Size Change: +152 B (0%)

Total Size: 217 kB

Filename	Size	Change
dist/chart.esm.js	63.1 kB	+56 B (0%)
dist/chart.js	79 kB	+59 B (0%)
dist/chart.min.js	57.3 kB	+37 B (0%)

[leeoniya]

+1 for having this behavior in the core and enabled via an opt, or better yet on by default.

i'm already not a fan of HighCharts needing the additional "boost" module either. at some point i'm going to add ** callouts for libs that require excessive and precise config to attain the shown bench results.

[etimberg]

That's some good feedback @leeoniya. I agree that if we can find a way to make it on by default, it'd be great. In my ideal world, I'd try to make it enabled even when we have to parse and sort the data as it would be a good improvement for everyone. With that said, I did look at flot and they have the hook for decimation before parsing.

Some things I'd want to do on the plugin before we consider shipping it:

• Support more axis types (log, timeseries)
• Bail out early if data.length < 4 * width
• Testing for heuristics and decide if fancier axis measuring is needed
• Disable when the primary axis is y
• Find a way to make this work better with editable data. Right now replaced dataset.data is disruptive

[kurkle]

I think these new hook(s) could be valuable even if we do the decimation in core.

If doing in core (+1), it might be good to just have that new hook for controllers too, and implement it in the line controller. Does not make sense to have it in 2 levels, so moving from LineElement to LineController works for me.

Note: After this, the addElements only adds the dataset element, which is not what you'd except from the function name.

[kurkle]

Suggested change

	let i, parsed, cur, prev;
	if (start > 0) {
	if (start > 0 || (start === 0 && count < data.length)) {
	sorted = meta._sorted;
	prev = meta._parsed[start - 1];
	prev = start > 0 ? meta._parsed[start - 1] : undefined;
	}
	let sorted = start === 0 && count === data.length ? true : meta._sorted;
	let prev = start > 0 && meta._parsed[start - 1];
	let i, cur, parsed;

[etimberg]

Good simplification. With fresh eyes, I thought of some cases where this method could incorrectly detect the sorted status of the data array.

Incorrectly Mark Data as not Sorted

const data = [0, 2, 1, 3, 4];
parse(0, 5); // not sorted

data[1] = 1;
data[2] = 2; // data == [0, 1, 2, 3, 4]

parse(0, 3); // not sorted

Incorrect Mark Data as Sorted

const data = [0, 1, 2, 3, 4];
parse(0, 5); // sorted

data[2] = 4; // data == [0, 1, 4, 3, 4]
parse(0, 3); // sorted

[kurkle]

Good points. We should set the meta._sorted to false when data is modified (and parsing is enabled).
And add tests for these cases, to make sure it works :)

[etimberg]

I think these new hook(s) could be valuable even if we do the decimation in core.

If doing in core (+1), it might be good to just have that new hook for controllers too, and implement it in the line controller. Does not make sense to have it in 2 levels, so moving from LineElement to LineController works for me.

Note: After this, the addElements only adds the dataset element, which is not what you'd except from the function name.

Can you expand a bit on what you're thinking for this? Do you mean having a beforeElementUpdate method on the controller that gets called first? Or do you mean a beforeElementUpdate hook that the dataset controller fires?

[kurkle]

Can you expand a bit on what you're thinking for this? Do you mean having a beforeElementUpdate method on the controller that gets called first? Or do you mean a beforeElementUpdate hook that the dataset controller fires?

The first one, having a beforeElementsUpdate on the controller. I did not walk the code through, so not sure if that makes sense.

[etimberg]

It might be possible to put it there though not sure if the controller should be modifying the data.

[etimberg]

I made some progress. I added the plugin into the core (not yet enabled by default). I also added a new feature, dataset.dataKey that allows pointing the data stream away from dataset.data. My thinking is that users don't want their original array modified, so the decimation plugin stores the decimated data as dataset._decimated and then we tell the controller to treat datset._decimated as the array to listen to (for updates, etc). That should resolve a lot of the complexity with updates & user interactions.

Work Still To-Do

• Document decimation plugin options
• Test all controllers with dataKey feature
• Decide on some initial options for decimation. Allow code to support configurable decimation algorithms in the future

[benmccann]

dataKey seems a little risky to me because every plugin, etc. would have to understand to get data from dataKey instead of data which would add complication. I'd probably just stick the original data on another field called _originalData and replace data with the decimated data. The original array isn't be modified (just moved), so the user can easily maintain a reference to it outside the chart to maintain access to it without having to know about the _originalData field.

[kurkle]

We could create a getter for the data that uses dataKey. Would need to rename the original data property then to something else and default dataKey to that.

[etimberg]

I thought about the getter solution too. The reason I didn't implement it was what happens if someone did something like #7340 and passed the data in a class instance

[benmccann]

I'm not sure I understand how the class instance relates to having a getter. They seem orthogonal to me, but I might be missing something

[etimberg]

I meant if the user passes in a class where data is already a getter, we may not be able to redefine it. This is also likely a problem for frozen data objects (hence why we moved the metas to chart._metasets). Perhaps we can put the data key there, or as you said, just modify the data array and tell the user that that's how the plugin works.

@kurkle @simonbrunel any thoughts?

[kurkle]

The use passed data object can be stored in the config. we can build the chart.data.datasets[i] and have getter for that objects data property.
so chart.config.data.datasets[0].data would be the original data object. when decimated we lose any additional properties or prototype a data point has. What are we thinking differently?

[kurkle]

Some clarification to what I was suggesting:

currently:

chart {
  get data { return this.config.data; }
  set data(data) { this.config.data = data; }
}

proposal:

Chart {
  constructor() {
    this.data = { labels: [], datasets: [] };
  }
  set data(data) { 
    this.config.data = data;
    // update('data'), or continue requiring user to call update()
  }
  buildOrUpdateControllers() {
    const datasets = this.data.datasets;
    const configDatasets = me.config.data.datasets;
    for (let i = 0, ilen = configDatasets.length; i < ilen; i++) {
      // simplified, update logic should remain
      datasets[i] = new DatasetController(me, i, configDatasets[i]);
    }
  }
}

DatasetController {
  constructor(chart, index, config) {
    this.config = config;
    this._data = config.data || [];
    this.dataKey = '_data';
  }
  get data {
    return this[this.dataKey];
  }
  set data(value) {
    this.config.data = value;
    // update('data') or continue requiring user to call update()
  }
}

So chart.data.datasets array would contain the controllers, but setting chart.data would equal to setting chart.config.data (for compatibility, not sure if its needed).

Could also combine controller and its meta for reduced complexity and overhead, but I'm not sure if it would be good in all aspects.

[etimberg]

I'm trying to think more about this. It could be very surprising if chart.data.datasets was the controllers and many people may be using chart.data and assuming it is the data object they passed in. In v3 we did a lot of work to stop modify the data object passed by the user and this feels like a regression backwards.

Maybe I should just update the plugin to modify the dataset and keep the changes minimal force it to be opt-in. Something like this. When decimating in future updates, the plugin would need to look for _data first since that indicates whether or not the dataset has been mutated.

dataset._data = dataset.data;
dataset._decimated = dataset.decimated;

Object.defineProperty(dataset, 'data', {
  configurable: true,
  enumerable: true,
  writable: true,
  get: function() {
    return this._decimated;
  },
  set: function(data) {
    this._data = data;
  }
});

We'd probably want something to clean this up on chart delete and put the object back to what it was.

[benmccann]

I'm trying to think more about this. It could be very surprising if chart.data.datasets was the controllers and many people may be using chart.data and assuming it is the data object they passed in. In v3 we did a lot of work to stop modify the data object passed by the user and this feels like a regression backwards.

I agree with all of this.

Maybe I should just update the plugin to modify the dataset and keep the changes minimal force it to be opt-in.

I still think we might be able to do better than this

Here's another idea - what if we made meta.data hold the decimated data and chart.data.datasets.data could remain the original data? We'd have to be a little careful of which one we're using in the scriptable context and plugins, but hopefully it should mostly be pretty straightforward

[etimberg]

I thought about something like that. meta.data is currently the elements, but meta.parsed is the parsed data (that would be decimated). The big downside that I thought of is that most of our examples would break subtly when decimating. We show the callbacks for the tooltips using dataset.data[index] and without decimation the index will match up, but as soon as there is any decimation, the indexes will be off and user tooltips could show the wrong data. I can see this leading to a lot of issues as users migrate over to v3.

[benmccann]

Callbacks for tooltips are much simpler if they just use dataPoint or formattedValue (docs). Perhaps we should remove dataset from the context to avoid the possibility of users subtly tripping up as you mentioned and to encourage them to write it in the simpler way. I took a look at this and the one thing we'd need to change is that dataPoint is only the parsed value. We should probably add a rawPoint as well (#8318)

[kurkle]

@etimberg
leeoniya/uPlot#455 (comment)

[etimberg]

I think we might need to account for that (0.5, 0.5) translation that is applied