From 6234f5635ae8a97c3ef44599f8b28db5ada9c3c7 Mon Sep 17 00:00:00 2001 From: Ben McCann <322311+benmccann@users.noreply.github.com> Date: Sun, 26 May 2019 21:27:29 -0700 Subject: [PATCH 1/3] [performance] replace helpers.each calls with for-loops --- src/core/core.helpers.js | 19 +++++++--- src/scales/scale.linear.js | 66 ++++++++++++++++----------------- src/scales/scale.logarithmic.js | 53 +++++++++++++------------- 3 files changed, 72 insertions(+), 66 deletions(-) diff --git a/src/core/core.helpers.js b/src/core/core.helpers.js index 52c8aea68a8..31748758090 100644 --- a/src/core/core.helpers.js +++ b/src/core/core.helpers.js @@ -553,25 +553,30 @@ module.exports = function() { ctx.font = font; var longest = 0; - helpers.each(arrayOfThings, function(thing) { + var ilen = arrayOfThings.length; + var i, j, jlen, thing, nestedThing; + for (i = 0; i < ilen; i++) { + thing = arrayOfThings[i]; + // Undefined strings and arrays should not be measured if (thing !== undefined && thing !== null && helpers.isArray(thing) !== true) { longest = helpers.measureText(ctx, data, gc, longest, thing); } else if (helpers.isArray(thing)) { // if it is an array lets measure each element // to do maybe simplify this function a bit so we can do this more recursively? - helpers.each(thing, function(nestedThing) { + for (j = 0, jlen = thing.length; j < jlen; j++) { + nestedThing = thing[j]; // Undefined strings and arrays should not be measured if (nestedThing !== undefined && nestedThing !== null && !helpers.isArray(nestedThing)) { longest = helpers.measureText(ctx, data, gc, longest, nestedThing); } - }); + } } - }); + } var gcLen = gc.length / 2; if (gcLen > arrayOfThings.length) { - for (var i = 0; i < gcLen; i++) { + for (i = 0; i < gcLen; i++) { delete data[gc[i]]; } gc.splice(0, gcLen); @@ -589,6 +594,10 @@ module.exports = function() { } return longest; }; + + /** + * @deprecated + */ helpers.numberOfLabelLines = function(arrayOfThings) { var numberOfLines = 1; helpers.each(arrayOfThings, function(thing) { diff --git a/src/scales/scale.linear.js b/src/scales/scale.linear.js index c9ce8709ded..f54a3eea816 100644 --- a/src/scales/scale.linear.js +++ b/src/scales/scale.linear.js @@ -16,14 +16,14 @@ module.exports = LinearScaleBase.extend({ var me = this; var opts = me.options; var chart = me.chart; - var data = chart.data; - var datasets = data.datasets; + var datasets = chart.data.datasets; var isHorizontal = me.isHorizontal(); var DEFAULT_MIN = 0; var DEFAULT_MAX = 1; + var datasetIndex, meta, value, data, i, ilen; - function IDMatches(meta) { - return isHorizontal ? meta.xAxisID === me.id : meta.yAxisID === me.id; + function IDMatches(scaleMeta) { + return isHorizontal ? scaleMeta.xAxisID === me.id : scaleMeta.yAxisID === me.id; } // First Calculate the range @@ -32,24 +32,20 @@ module.exports = LinearScaleBase.extend({ var hasStacks = opts.stacked; if (hasStacks === undefined) { - helpers.each(datasets, function(dataset, datasetIndex) { - if (hasStacks) { - return; - } - - var meta = chart.getDatasetMeta(datasetIndex); - if (chart.isDatasetVisible(datasetIndex) && IDMatches(meta) && - meta.stack !== undefined) { + for (datasetIndex = 0; datasetIndex < datasets.length; datasetIndex++) { + meta = chart.getDatasetMeta(datasetIndex); + if (chart.isDatasetVisible(datasetIndex) && IDMatches(meta) && meta.stack !== undefined) { hasStacks = true; + break; } - }); + } } if (opts.stacked || hasStacks) { var valuesPerStack = {}; - helpers.each(datasets, function(dataset, datasetIndex) { - var meta = chart.getDatasetMeta(datasetIndex); + for (datasetIndex = 0; datasetIndex < datasets.length; datasetIndex++) { + meta = chart.getDatasetMeta(datasetIndex); var key = [ meta.type, // we have a separate stack for stack=undefined datasets when the opts.stacked is undefined @@ -69,30 +65,31 @@ module.exports = LinearScaleBase.extend({ var negativeValues = valuesPerStack[key].negativeValues; if (chart.isDatasetVisible(datasetIndex) && IDMatches(meta)) { - helpers.each(dataset.data, function(rawValue, index) { - var value = me._parseValue(rawValue); + data = datasets[datasetIndex].data; + for (i = 0, ilen = data.length; i < ilen; i++) { + value = me._parseValue(data[i]); - if (isNaN(value.min) || isNaN(value.max) || meta.data[index].hidden) { - return; + if (isNaN(value.min) || isNaN(value.max) || meta.data[i].hidden) { + continue; } - positiveValues[index] = positiveValues[index] || 0; - negativeValues[index] = negativeValues[index] || 0; + positiveValues[i] = positiveValues[i] || 0; + negativeValues[i] = negativeValues[i] || 0; if (value.min === 0 && !opts.ticks.beginAtZero) { value.min = value.max; } if (opts.relativePoints) { - positiveValues[index] = 100; + positiveValues[i] = 100; } else if (value.min < 0 || value.max < 0) { - negativeValues[index] += value.min; + negativeValues[i] += value.min; } else { - positiveValues[index] += value.max; + positiveValues[i] += value.max; } - }); + } } - }); + } helpers.each(valuesPerStack, function(valuesForType) { var values = valuesForType.positiveValues.concat(valuesForType.negativeValues); @@ -103,14 +100,15 @@ module.exports = LinearScaleBase.extend({ }); } else { - helpers.each(datasets, function(dataset, datasetIndex) { - var meta = chart.getDatasetMeta(datasetIndex); + for (datasetIndex = 0; datasetIndex < datasets.length; datasetIndex++) { + meta = chart.getDatasetMeta(datasetIndex); if (chart.isDatasetVisible(datasetIndex) && IDMatches(meta)) { - helpers.each(dataset.data, function(rawValue, index) { - var value = me._parseValue(rawValue); + data = datasets[datasetIndex].data; + for (i = 0, ilen = data.length; i < ilen; i++) { + value = me._parseValue(data[i]); - if (isNaN(value.min) || isNaN(value.max) || meta.data[index].hidden) { - return; + if (isNaN(value.min) || isNaN(value.max) || meta.data[i].hidden) { + continue; } if (me.min === null || value.min < me.min) { @@ -120,9 +118,9 @@ module.exports = LinearScaleBase.extend({ if (me.max === null || me.max < value.max) { me.max = value.max; } - }); + } } - }); + } } me.min = isFinite(me.min) && !isNaN(me.min) ? me.min : DEFAULT_MIN; diff --git a/src/scales/scale.logarithmic.js b/src/scales/scale.logarithmic.js index 3e9b2849f0b..e086861429a 100644 --- a/src/scales/scale.logarithmic.js +++ b/src/scales/scale.logarithmic.js @@ -72,12 +72,12 @@ module.exports = Scale.extend({ var me = this; var opts = me.options; var chart = me.chart; - var data = chart.data; - var datasets = data.datasets; + var datasets = chart.data.datasets; var isHorizontal = me.isHorizontal(); function IDMatches(meta) { return isHorizontal ? meta.xAxisID === me.id : meta.yAxisID === me.id; } + var datasetIndex, meta, value, data, i, ilen; // Calculate Range me.min = null; @@ -86,24 +86,21 @@ module.exports = Scale.extend({ var hasStacks = opts.stacked; if (hasStacks === undefined) { - helpers.each(datasets, function(dataset, datasetIndex) { - if (hasStacks) { - return; - } - - var meta = chart.getDatasetMeta(datasetIndex); + for (datasetIndex = 0; datasetIndex < datasets.length; datasetIndex++) { + meta = chart.getDatasetMeta(datasetIndex); if (chart.isDatasetVisible(datasetIndex) && IDMatches(meta) && meta.stack !== undefined) { hasStacks = true; + break; } - }); + } } if (opts.stacked || hasStacks) { var valuesPerStack = {}; - helpers.each(datasets, function(dataset, datasetIndex) { - var meta = chart.getDatasetMeta(datasetIndex); + for (datasetIndex = 0; datasetIndex < datasets.length; datasetIndex++) { + meta = chart.getDatasetMeta(datasetIndex); var key = [ meta.type, // we have a separate stack for stack=undefined datasets when the opts.stacked is undefined @@ -116,18 +113,19 @@ module.exports = Scale.extend({ valuesPerStack[key] = []; } - helpers.each(dataset.data, function(rawValue, index) { + data = datasets[datasetIndex].data; + for (i = 0, ilen = data.length; i < ilen; i++) { var values = valuesPerStack[key]; - var value = me._parseValue(rawValue); + value = me._parseValue(data[i]); // invalid, hidden and negative values are ignored - if (isNaN(value.min) || isNaN(value.max) || meta.data[index].hidden || value.min < 0 || value.max < 0) { - return; + if (isNaN(value.min) || isNaN(value.max) || meta.data[i].hidden || value.min < 0 || value.max < 0) { + continue; } - values[index] = values[index] || 0; - values[index] += value.max; - }); + values[i] = values[i] || 0; + values[i] += value.max; + } } - }); + } helpers.each(valuesPerStack, function(valuesForType) { if (valuesForType.length > 0) { @@ -139,14 +137,15 @@ module.exports = Scale.extend({ }); } else { - helpers.each(datasets, function(dataset, datasetIndex) { - var meta = chart.getDatasetMeta(datasetIndex); + for (datasetIndex = 0; datasetIndex < datasets.length; datasetIndex++) { + meta = chart.getDatasetMeta(datasetIndex); if (chart.isDatasetVisible(datasetIndex) && IDMatches(meta)) { - helpers.each(dataset.data, function(rawValue, index) { - var value = me._parseValue(rawValue); + data = datasets[datasetIndex].data; + for (i = 0, ilen = data.length; i < ilen; i++) { + value = me._parseValue(data[i]); // invalid, hidden and negative values are ignored - if (isNaN(value.min) || isNaN(value.max) || meta.data[index].hidden || value.min < 0 || value.max < 0) { - return; + if (isNaN(value.min) || isNaN(value.max) || meta.data[i].hidden || value.min < 0 || value.max < 0) { + continue; } if (me.min === null || value.min < me.min) { @@ -160,9 +159,9 @@ module.exports = Scale.extend({ if (value.min !== 0 && (me.minNotZero === null || value.min < me.minNotZero)) { me.minNotZero = value.min; } - }); + } } - }); + } } // Common base implementation to handle ticks.min, ticks.max From 1c51d8fabc446edf2902131f6bab13f91c505438 Mon Sep 17 00:00:00 2001 From: Ben McCann <322311+benmccann@users.noreply.github.com> Date: Mon, 27 May 2019 14:35:30 -0700 Subject: [PATCH 2/3] Use Math.min/max instead of if statement --- src/scales/scale.linear.js | 23 ++++++++--------------- src/scales/scale.logarithmic.js | 27 +++++++++++++-------------- test/specs/core.ticks.tests.js | 4 ++-- test/specs/scale.linear.tests.js | 5 +++-- 4 files changed, 26 insertions(+), 33 deletions(-) diff --git a/src/scales/scale.linear.js b/src/scales/scale.linear.js index f54a3eea816..b97354a761e 100644 --- a/src/scales/scale.linear.js +++ b/src/scales/scale.linear.js @@ -27,8 +27,8 @@ module.exports = LinearScaleBase.extend({ } // First Calculate the range - me.min = null; - me.max = null; + me.min = Number.POSITIVE_INFINITY; + me.max = Number.NEGATIVE_INFINITY; var hasStacks = opts.stacked; if (hasStacks === undefined) { @@ -93,10 +93,8 @@ module.exports = LinearScaleBase.extend({ helpers.each(valuesPerStack, function(valuesForType) { var values = valuesForType.positiveValues.concat(valuesForType.negativeValues); - var minVal = helpers.min(values); - var maxVal = helpers.max(values); - me.min = me.min === null ? minVal : Math.min(me.min, minVal); - me.max = me.max === null ? maxVal : Math.max(me.max, maxVal); + me.min = Math.min(me.min, helpers.min(values)); + me.max = Math.max(me.max, helpers.max(values)); }); } else { @@ -111,20 +109,15 @@ module.exports = LinearScaleBase.extend({ continue; } - if (me.min === null || value.min < me.min) { - me.min = value.min; - } - - if (me.max === null || me.max < value.max) { - me.max = value.max; - } + me.min = Math.min(value.min, me.min); + me.max = Math.max(value.max, me.max); } } } } - me.min = isFinite(me.min) && !isNaN(me.min) ? me.min : DEFAULT_MIN; - me.max = isFinite(me.max) && !isNaN(me.max) ? me.max : DEFAULT_MAX; + me.min = helpers.isFinite(me.min) && !isNaN(me.min) ? me.min : DEFAULT_MIN; + me.max = helpers.isFinite(me.max) && !isNaN(me.max) ? me.max : DEFAULT_MAX; // Common base implementation to handle ticks.min, ticks.max, ticks.beginAtZero this.handleTickRangeOptions(); diff --git a/src/scales/scale.logarithmic.js b/src/scales/scale.logarithmic.js index e086861429a..7042bf23e08 100644 --- a/src/scales/scale.logarithmic.js +++ b/src/scales/scale.logarithmic.js @@ -80,9 +80,9 @@ module.exports = Scale.extend({ var datasetIndex, meta, value, data, i, ilen; // Calculate Range - me.min = null; - me.max = null; - me.minNotZero = null; + me.min = Number.POSITIVE_INFINITY; + me.max = Number.NEGATIVE_INFINITY; + me.minNotZero = Number.POSITIVE_INFINITY; var hasStacks = opts.stacked; if (hasStacks === undefined) { @@ -131,8 +131,8 @@ module.exports = Scale.extend({ if (valuesForType.length > 0) { var minVal = helpers.min(valuesForType); var maxVal = helpers.max(valuesForType); - me.min = me.min === null ? minVal : Math.min(me.min, minVal); - me.max = me.max === null ? maxVal : Math.max(me.max, maxVal); + me.min = Math.min(me.min, minVal); + me.max = Math.max(me.max, maxVal); } }); @@ -148,22 +148,21 @@ module.exports = Scale.extend({ continue; } - if (me.min === null || value.min < me.min) { - me.min = value.min; - } - - if (me.max === null || me.max < value.max) { - me.max = value.max; - } + me.min = Math.min(value.min, me.min); + me.max = Math.max(value.max, me.max); - if (value.min !== 0 && (me.minNotZero === null || value.min < me.minNotZero)) { - me.minNotZero = value.min; + if (value.min !== 0) { + me.minNotZero = Math.min(value.min, me.minNotZero); } } } } } + me.min = helpers.isFinite(me.min) ? me.min : null; + me.max = helpers.isFinite(me.max) ? me.max : null; + me.minNotZero = helpers.isFinite(me.minNotZero) ? me.minNotZero : null; + // Common base implementation to handle ticks.min, ticks.max this.handleTickRangeOptions(); }, diff --git a/test/specs/core.ticks.tests.js b/test/specs/core.ticks.tests.js index 01cecc5462e..afc2afda205 100644 --- a/test/specs/core.ticks.tests.js +++ b/test/specs/core.ticks.tests.js @@ -45,8 +45,8 @@ describe('Test tick generators', function() { var xAxis = chart.scales['x-axis-0']; var yAxis = chart.scales['y-axis-0']; - expect(xAxis.ticks).toEqual(['-1', '-0.8', '-0.6', '-0.4', '-0.2', '0', '0.2', '0.4', '0.6', '0.8', '1']); - expect(yAxis.ticks).toEqual(['1', '0.8', '0.6', '0.4', '0.2', '0', '-0.2', '-0.4', '-0.6', '-0.8', '-1']); + expect(xAxis.ticks).toEqual(['0', '0.1', '0.2', '0.3', '0.4', '0.5', '0.6', '0.7', '0.8', '0.9', '1']); + expect(yAxis.ticks).toEqual(['1', '0.9', '0.8', '0.7', '0.6', '0.5', '0.4', '0.3', '0.2', '0.1', '0']); }); it('Should generate logarithmic spaced ticks with correct precision', function() { diff --git a/test/specs/scale.linear.tests.js b/test/specs/scale.linear.tests.js index bdf8a9386f9..03f564a1d57 100644 --- a/test/specs/scale.linear.tests.js +++ b/test/specs/scale.linear.tests.js @@ -453,7 +453,7 @@ describe('Linear Scale', function() { }); expect(chart.scales.yScale0).not.toEqual(undefined); // must construct - expect(chart.scales.yScale0.min).toBe(-1); + expect(chart.scales.yScale0.min).toBe(0); expect(chart.scales.yScale0.max).toBe(1); }); @@ -829,10 +829,11 @@ describe('Linear Scale', function() { var chart = window.acquireChart({ type: 'line', data: { + labels: [-1, 1], datasets: [{ xAxisID: 'xScale0', yAxisID: 'yScale0', - data: [] + data: [-1, 1] }], }, options: { From e4b59fcc78e42f763d961d645f09399b54df5e44 Mon Sep 17 00:00:00 2001 From: Ben McCann <322311+benmccann@users.noreply.github.com> Date: Wed, 19 Jun 2019 08:05:01 -0700 Subject: [PATCH 3/3] Rename variable --- src/scales/scale.linear.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/scales/scale.linear.js b/src/scales/scale.linear.js index b97354a761e..93bff1b8298 100644 --- a/src/scales/scale.linear.js +++ b/src/scales/scale.linear.js @@ -22,8 +22,8 @@ module.exports = LinearScaleBase.extend({ var DEFAULT_MAX = 1; var datasetIndex, meta, value, data, i, ilen; - function IDMatches(scaleMeta) { - return isHorizontal ? scaleMeta.xAxisID === me.id : scaleMeta.yAxisID === me.id; + function IDMatches(datasetMeta) { + return isHorizontal ? datasetMeta.xAxisID === me.id : datasetMeta.yAxisID === me.id; } // First Calculate the range