From 6dde54bd25c89ae12274686635eaca5bf88be091 Mon Sep 17 00:00:00 2001 From: Shefali Joshi Date: Mon, 16 Aug 2021 14:21:09 -0700 Subject: [PATCH] Fix plots performance (#4092) * Fix no mutating props violation for Browsebar and StyleEditor * Separate plot series data from the configuration (like it should be!) --- src/plugins/plot/MctPlot.vue | 51 ++++++-------- src/plugins/plot/axis/XAxis.vue | 5 +- .../plot/chart/MCTChartAlarmPointSet.js | 2 +- .../plot/chart/MCTChartSeriesElement.js | 4 +- .../configuration/PlotConfigurationModel.js | 1 + src/plugins/plot/configuration/PlotSeries.js | 69 +++++++++++++------ src/plugins/plot/configuration/configStore.js | 5 +- src/plugins/plot/draw/DrawWebGL.js | 4 +- src/plugins/plot/pluginSpec.js | 5 +- src/ui/layout/BrowseBar.vue | 3 +- 10 files changed, 86 insertions(+), 63 deletions(-) diff --git a/src/plugins/plot/MctPlot.vue b/src/plugins/plot/MctPlot.vue index c423598e9b..02f48c5e14 100644 --- a/src/plugins/plot/MctPlot.vue +++ b/src/plugins/plot/MctPlot.vue @@ -25,16 +25,16 @@ :class="[plotLegendExpandedStateClass, plotLegendPositionClass]" >
- @@ -141,8 +141,8 @@ >
- @@ -213,7 +213,8 @@ export default { plotHistory: [], selectedXKeyOption: {}, xKeyOptions: [], - config: {}, + seriesModels: [], + legend: {}, pending: 0, isRealTime: this.openmct.time.clock() !== undefined, loaded: false, @@ -239,18 +240,13 @@ export default { watch: { plotTickWidth(newTickWidth) { this.onTickWidthChange(newTickWidth, true); - }, - gridLines(newGridLines) { - this.setGridLinesVisibility(newGridLines); - }, - cursorGuide(newCursorGuide) { - this.setCursorGuideVisibility(newCursorGuide); } }, mounted() { eventHelpers.extend(this); this.config = this.getConfig(); + this.legend = this.config.legend; this.listenTo(this.config.series, 'add', this.addSeries, this); this.listenTo(this.config.series, 'remove', this.removeSeries, this); @@ -290,14 +286,18 @@ export default { config = new PlotConfigurationModel({ id: configId, domainObject: this.domainObject, - openmct: this.openmct + openmct: this.openmct, + callback: (data) => { + this.data = data; + } }); configStore.add(configId, config); } return config; }, - addSeries(series) { + addSeries(series, index) { + this.$set(this.seriesModels, index, series); this.listenTo(series, 'change:xKey', (xKey) => { this.setDisplayRange(series, xKey); }, this); @@ -377,11 +377,8 @@ export default { }, stopLoading() { - //TODO: Is Vue.$nextTick ok to replace $scope.$evalAsync? - this.$nextTick().then(() => { - this.pending -= 1; - this.updateLoading(); - }); + this.pending -= 1; + this.updateLoading(); }, updateLoading() { @@ -507,7 +504,7 @@ export default { }, initialize() { - _.debounce(this.handleWindowResize, 400); + this.handleWindowResize = _.debounce(this.handleWindowResize, 500); this.plotContainerResizeObserver = new ResizeObserver(this.handleWindowResize); this.plotContainerResizeObserver.observe(this.$parent.$refs.plotWrapper); @@ -623,7 +620,7 @@ export default { this.config.series.models.forEach(series => delete series.closest); } else { this.highlights = this.config.series.models - .filter(series => series.data.length > 0) + .filter(series => series.getSeriesData().length > 0) .map(series => { series.closest = series.nearestPoint(point); @@ -927,14 +924,6 @@ export default { this.userViewportChangeEnd(); }, - setCursorGuideVisibility(cursorGuide) { - this.cursorGuide = cursorGuide === true; - }, - - setGridLinesVisibility(gridLines) { - this.gridLines = gridLines === true; - }, - setYAxisKey(yKey) { this.config.series.models[0].set('yKey', yKey); }, diff --git a/src/plugins/plot/axis/XAxis.vue b/src/plugins/plot/axis/XAxis.vue index 42668b2b0b..8e414ca8c5 100644 --- a/src/plugins/plot/axis/XAxis.vue +++ b/src/plugins/plot/axis/XAxis.vue @@ -106,8 +106,9 @@ export default { }, toggleXKeyOption() { const selectedXKey = this.selectedXKeyOptionKey; - const dataForSelectedXKey = this.seriesModel.data - ? this.seriesModel.data[0][selectedXKey] + const seriesData = this.seriesModel.getSeriesData(); + const dataForSelectedXKey = seriesData + ? seriesData[0][selectedXKey] : undefined; if (dataForSelectedXKey !== undefined) { diff --git a/src/plugins/plot/chart/MCTChartAlarmPointSet.js b/src/plugins/plot/chart/MCTChartAlarmPointSet.js index 8167720f65..58a6a24b10 100644 --- a/src/plugins/plot/chart/MCTChartAlarmPointSet.js +++ b/src/plugins/plot/chart/MCTChartAlarmPointSet.js @@ -36,7 +36,7 @@ export default class MCTChartAlarmPointSet { this.listenTo(series, 'reset', this.reset, this); this.listenTo(series, 'destroy', this.destroy, this); - series.data.forEach(function (point, index) { + this.series.getSeriesData().forEach(function (point, index) { this.append(point, index, series); }, this); } diff --git a/src/plugins/plot/chart/MCTChartSeriesElement.js b/src/plugins/plot/chart/MCTChartSeriesElement.js index 6a28b8664e..116a3ea747 100644 --- a/src/plugins/plot/chart/MCTChartSeriesElement.js +++ b/src/plugins/plot/chart/MCTChartSeriesElement.js @@ -36,7 +36,7 @@ export default class MCTChartSeriesElement { this.listenTo(series, 'remove', this.remove, this); this.listenTo(series, 'reset', this.reset, this); this.listenTo(series, 'destroy', this.destroy, this); - series.data.forEach(function (point, index) { + this.series.getSeriesData().forEach(function (point, index) { this.append(point, index, series); }, this); } @@ -133,7 +133,7 @@ export default class MCTChartSeriesElement { this.buffer = new Float32Array(20000); this.count = 0; if (this.offset.x) { - this.series.data.forEach(function (point, index) { + this.series.getSeriesData().forEach(function (point, index) { this.append(point, index, this.series); }, this); } diff --git a/src/plugins/plot/configuration/PlotConfigurationModel.js b/src/plugins/plot/configuration/PlotConfigurationModel.js index 24773f9989..f9f14cef54 100644 --- a/src/plugins/plot/configuration/PlotConfigurationModel.js +++ b/src/plugins/plot/configuration/PlotConfigurationModel.js @@ -107,6 +107,7 @@ export default class PlotConfigurationModel extends Model { updateDomainObject(domainObject) { this.set('domainObject', domainObject); } + /** * Clean up all objects and remove all listeners. */ diff --git a/src/plugins/plot/configuration/PlotSeries.js b/src/plugins/plot/configuration/PlotSeries.js index a5778c3a4c..7863dcdeb5 100644 --- a/src/plugins/plot/configuration/PlotSeries.js +++ b/src/plugins/plot/configuration/PlotSeries.js @@ -22,6 +22,7 @@ import _ from 'lodash'; import Model from "./Model"; import { MARKER_SHAPES } from '../draw/MarkerShapes'; +import configStore from "../configuration/configStore"; /** * Plot series handle interpreting telemetry metadata for a single telemetry @@ -62,7 +63,6 @@ import { MARKER_SHAPES } from '../draw/MarkerShapes'; export default class PlotSeries extends Model { constructor(options) { super(options); - this.data = []; this.listenTo(this, 'change:xKey', this.onXKeyChange, this); this.listenTo(this, 'change:yKey', this.onYKeyChange, this); @@ -115,6 +115,8 @@ export default class PlotSeries extends Model { this.openmct = options.openmct; this.domainObject = options.domainObject; this.keyString = this.openmct.objects.makeKeyString(this.domainObject.identifier); + this.dataStoreId = `data-${options.collection.plot.id}-${this.keyString}`; + this.updateSeriesData([]); this.limitEvaluator = this.openmct.telemetry.limitEvaluator(options.domainObject); this.limitDefinition = this.openmct.telemetry.limitDefinition(options.domainObject); this.limits = []; @@ -182,7 +184,8 @@ export default class PlotSeries extends Model { .telemetry .request(this.domainObject, options) .then(function (points) { - const newPoints = _(this.data) + const data = this.getSeriesData(); + const newPoints = _(data) .concat(points) .sortBy(this.getXVal) .uniq(true, point => [this.getXVal(point), this.getYVal(point)].join()) @@ -236,7 +239,7 @@ export default class PlotSeries extends Model { */ resetStats() { this.unset('stats'); - this.data.forEach(this.updateStats, this); + this.getSeriesData().forEach(this.updateStats, this); } /** @@ -244,7 +247,7 @@ export default class PlotSeries extends Model { * data to series after reset. */ reset(newData) { - this.data = []; + this.updateSeriesData([]); this.resetStats(); this.emit('reset'); if (newData) { @@ -258,8 +261,9 @@ export default class PlotSeries extends Model { */ nearestPoint(xValue) { const insertIndex = this.sortedIndex(xValue); - const lowPoint = this.data[insertIndex - 1]; - const highPoint = this.data[insertIndex]; + const data = this.getSeriesData(); + const lowPoint = data[insertIndex - 1]; + const highPoint = data[insertIndex]; const indexVal = this.getXVal(xValue); const lowDistance = lowPoint ? indexVal - this.getXVal(lowPoint) @@ -292,7 +296,7 @@ export default class PlotSeries extends Model { * @private */ sortedIndex(point) { - return _.sortedIndexBy(this.data, point, this.getXVal); + return _.sortedIndexBy(this.getSeriesData(), point, this.getXVal); } /** * Update min/max stats for the series. @@ -346,9 +350,10 @@ export default class PlotSeries extends Model { * a point to the end without dupe checking. */ add(point, appendOnly) { - let insertIndex = this.data.length; + let data = this.getSeriesData(); + let insertIndex = data.length; const currentYVal = this.getYVal(point); - const lastYVal = this.getYVal(this.data[insertIndex - 1]); + const lastYVal = this.getYVal(data[insertIndex - 1]); if (this.isValueInvalid(currentYVal) && this.isValueInvalid(lastYVal)) { console.warn('[Plot] Invalid Y Values detected'); @@ -358,18 +363,19 @@ export default class PlotSeries extends Model { if (!appendOnly) { insertIndex = this.sortedIndex(point); - if (this.getXVal(this.data[insertIndex]) === this.getXVal(point)) { + if (this.getXVal(data[insertIndex]) === this.getXVal(point)) { return; } - if (this.getXVal(this.data[insertIndex - 1]) === this.getXVal(point)) { + if (this.getXVal(data[insertIndex - 1]) === this.getXVal(point)) { return; } } this.updateStats(point); point.mctLimitState = this.evaluate(point); - this.data.splice(insertIndex, 0, point); + data.splice(insertIndex, 0, point); + this.updateSeriesData(data); this.emit('add', point, insertIndex, this); } @@ -386,8 +392,10 @@ export default class PlotSeries extends Model { * @private */ remove(point) { - const index = this.data.indexOf(point); - this.data.splice(index, 1); + let data = this.getSeriesData(); + const index = data.indexOf(point); + data.splice(index, 1); + this.updateSeriesData(data); this.emit('remove', point, index, this); } /** @@ -403,14 +411,16 @@ export default class PlotSeries extends Model { purgeRecordsOutsideRange(range) { const startIndex = this.sortedIndex(range.min); const endIndex = this.sortedIndex(range.max) + 1; - const pointsToRemove = startIndex + (this.data.length - endIndex + 1); + let data = this.getSeriesData(); + const pointsToRemove = startIndex + (data.length - endIndex + 1); if (pointsToRemove > 0) { if (pointsToRemove < 1000) { - this.data.slice(0, startIndex).forEach(this.remove, this); - this.data.slice(endIndex, this.data.length).forEach(this.remove, this); + data.slice(0, startIndex).forEach(this.remove, this); + data.slice(endIndex, data.length).forEach(this.remove, this); + this.updateSeriesData(data); this.resetStats(); } else { - const newData = this.data.slice(startIndex, endIndex); + const newData = this.getSeriesData().slice(startIndex, endIndex); this.reset(newData); } } @@ -441,12 +451,13 @@ export default class PlotSeries extends Model { } } getDisplayRange(xKey) { - const unsortedData = this.data; - this.data = []; + const unsortedData = this.getSeriesData(); + this.updateSeriesData([]); unsortedData.forEach(point => this.add(point, false)); - const minValue = this.getXVal(this.data[0]); - const maxValue = this.getXVal(this.data[this.data.length - 1]); + let data = this.getSeriesData(); + const minValue = this.getXVal(data[0]); + const maxValue = this.getXVal(data[data.length - 1]); return { min: minValue, @@ -470,4 +481,18 @@ export default class PlotSeries extends Model { return this.get('name') + (unit ? ' ' + unit : ''); } + + /** + * Update the series data with the given value. + */ + updateSeriesData(data) { + configStore.add(this.dataStoreId, data); + } + + /** + * Update the series data with the given value. + */ + getSeriesData() { + return configStore.get(this.dataStoreId) || []; + } } diff --git a/src/plugins/plot/configuration/configStore.js b/src/plugins/plot/configuration/configStore.js index 9a0cd15c0b..c7e07a82ec 100644 --- a/src/plugins/plot/configuration/configStore.js +++ b/src/plugins/plot/configuration/configStore.js @@ -25,7 +25,10 @@ function ConfigStore() { ConfigStore.prototype.deleteStore = function (id) { if (this.store[id]) { - this.store[id].destroy(); + if (this.store[id].destroy) { + this.store[id].destroy(); + } + delete this.store[id]; } }; diff --git a/src/plugins/plot/draw/DrawWebGL.js b/src/plugins/plot/draw/DrawWebGL.js index 00dea398e4..7918852a0c 100644 --- a/src/plugins/plot/draw/DrawWebGL.js +++ b/src/plugins/plot/draw/DrawWebGL.js @@ -176,7 +176,9 @@ DrawWebGL.prototype.doDraw = function (drawType, buf, color, points, shape) { this.gl.vertexAttribPointer(this.aVertexPosition, 2, this.gl.FLOAT, false, 0, 0); this.gl.uniform4fv(this.uColor, color); this.gl.uniform1i(this.uMarkerShape, shapeCode); - this.gl.drawArrays(drawType, 0, points); + if (points !== 0) { + this.gl.drawArrays(drawType, 0, points); + } }; DrawWebGL.prototype.clear = function () { diff --git a/src/plugins/plot/pluginSpec.js b/src/plugins/plot/pluginSpec.js index e3d18876a7..d10a5edeff 100644 --- a/src/plugins/plot/pluginSpec.js +++ b/src/plugins/plot/pluginSpec.js @@ -715,14 +715,15 @@ describe("the plugin", function () { }); it("Adds a new point to the plot", (done) => { - let originalLength = config.series.models[0].data.length; + let originalLength = config.series.models[0].getSeriesData().length; config.series.models[0].add({ utc: 2, 'some-key': 1, 'some-other-key': 2 }); Vue.nextTick(() => { - expect(config.series.models[0].data.length).toEqual(originalLength + 1); + const seriesData = config.series.models[0].getSeriesData(); + expect(seriesData.length).toEqual(originalLength + 1); done(); }); }); diff --git a/src/ui/layout/BrowseBar.vue b/src/ui/layout/BrowseBar.vue index cd828626ec..df5287c82e 100644 --- a/src/ui/layout/BrowseBar.vue +++ b/src/ui/layout/BrowseBar.vue @@ -233,6 +233,7 @@ export default { }, mounted: function () { document.addEventListener('click', this.closeViewAndSaveMenu); + this.promptUserbeforeNavigatingAway = this.promptUserbeforeNavigatingAway.bind(this); window.addEventListener('beforeunload', this.promptUserbeforeNavigatingAway); this.openmct.editor.on('isEditing', (isEditing) => { @@ -253,7 +254,7 @@ export default { } document.removeEventListener('click', this.closeViewAndSaveMenu); - window.removeEventListener('click', this.promptUserbeforeNavigatingAway); + window.removeEventListener('beforeunload', this.promptUserbeforeNavigatingAway); }, methods: { toggleSaveMenu() {