From 8c9fe2d36b1a8e3f0eb9ae53524c45252a858361 Mon Sep 17 00:00:00 2001 From: Andrew Henry Date: Mon, 16 Jul 2018 15:23:34 -0700 Subject: [PATCH] [Tables] Address performance issues observed in testathon (#2112) * Use datum key of 'utc' for timestamp * Do not try to get column values for data that does not have those values * Collapse current time system columns * Noop parsing for numbers in LocalTimeFormat * Do not attempt to provide data for columns that object does not have telemetry for. Fixes #2027 --- example/eventGenerator/bundle.js | 4 +- .../table/res/templates/telemetry-table.html | 2 +- platform/features/table/src/TableColumn.js | 69 +++++++++++ .../features/table/src/TableConfiguration.js | 107 +++++++----------- .../features/table/src/TelemetryCollection.js | 6 +- .../controllers/TelemetryTableController.js | 68 ++++------- .../table/test/TableConfigurationSpec.js | 68 +++++++---- .../TelemetryTableControllerSpec.js | 59 ++++------ .../localTimeSystem/LocalTimeFormat.js | 3 + 9 files changed, 215 insertions(+), 171 deletions(-) create mode 100644 platform/features/table/src/TableColumn.js diff --git a/example/eventGenerator/bundle.js b/example/eventGenerator/bundle.js index 16a5512e66..2fb56947ec 100644 --- a/example/eventGenerator/bundle.js +++ b/example/eventGenerator/bundle.js @@ -60,8 +60,8 @@ define([ "source": "eventGenerator", "domains": [ { - "key": "time", - "name": "Time", + "key": "utc", + "name": "Timestamp", "format": "utc" } ], diff --git a/platform/features/table/res/templates/telemetry-table.html b/platform/features/table/res/templates/telemetry-table.html index 0a552dd6dc..d76de4c43c 100644 --- a/platform/features/table/res/templates/telemetry-table.html +++ b/platform/features/table/res/templates/telemetry-table.html @@ -3,7 +3,7 @@ 0) { + this.titleValue = title; + } + return this.titleValue; + }; + + TableColumn.prototype.isCurrentTimeSystem = function () { + var isCurrentTimeSystem = this.metadatum.hints.hasOwnProperty('domain') && + this.metadatum.key === this.openmct.time.timeSystem().key; + + return isCurrentTimeSystem; + }; + + TableColumn.prototype.hasValue = function (telemetryObject, telemetryDatum) { + var keyStringForDatum = this.openmct.objects.makeKeyString(telemetryObject.identifier); + var keyStringForColumn = this.openmct.objects.makeKeyString(this.telemetryObject.identifier); + return keyStringForDatum === keyStringForColumn && telemetryDatum.hasOwnProperty(this.metadatum.source); + }; + + TableColumn.prototype.getValue = function (telemetryDatum, limitEvaluator) { + var isValueColumn = !!(this.metadatum.hints.y || this.metadatum.hints.range); + var alarm = isValueColumn && + limitEvaluator && + limitEvaluator.evaluate(telemetryDatum, this.metadatum); + var value = { + text: this.formatter.format(telemetryDatum), + value: this.formatter.parse(telemetryDatum) + }; + + if (alarm) { + value.cssClass = alarm.cssClass; + } + return value; + }; + + return TableColumn; +}); diff --git a/platform/features/table/src/TableConfiguration.js b/platform/features/table/src/TableConfiguration.js index d97d6838a3..d5525410aa 100644 --- a/platform/features/table/src/TableConfiguration.js +++ b/platform/features/table/src/TableConfiguration.js @@ -19,10 +19,10 @@ * this source code distribution or the Licensing information page available * at runtime from the About dialog for additional information. *****************************************************************************/ - +/* global Set */ define( - [], - function () { + ['./TableColumn'], + function (TableColumn) { /** * Class that manages table metadata, state, and contents. @@ -32,62 +32,31 @@ define( */ function TableConfiguration(domainObject, openmct) { this.domainObject = domainObject; - this.columns = []; this.openmct = openmct; + this.timeSystemColumn = undefined; + this.columns = []; + this.headers = new Set(); + this.timeSystemColumnTitle = undefined; } /** - * Build column definitions based on supplied telemetry metadata + * Build column definition based on supplied telemetry metadata + * @param telemetryObject the telemetry producing object associated with this column * @param metadata Metadata describing the domains and ranges available * @returns {TableConfiguration} This object */ - TableConfiguration.prototype.populateColumns = function (metadata) { - var self = this; - var telemetryApi = this.openmct.telemetry; + TableConfiguration.prototype.addColumn = function (telemetryObject, metadatum) { + var column = new TableColumn(this.openmct, telemetryObject, metadatum); - this.columns = []; - - if (metadata) { - - metadata.forEach(function (metadatum) { - var formatter = telemetryApi.getValueFormatter(metadatum); - - self.columns.push({ - getKey: function () { - return metadatum.key; - }, - getTitle: function () { - return metadatum.name; - }, - getValue: function (telemetryDatum, limitEvaluator) { - var isValueColumn = !!(metadatum.hints.y || metadatum.hints.range); - var alarm = isValueColumn && - limitEvaluator && - limitEvaluator.evaluate(telemetryDatum, metadatum); - var value = { - text: formatter.format(telemetryDatum), - value: formatter.parse(telemetryDatum) - }; - - if (alarm) { - value.cssClass = alarm.cssClass; - } - return value; - } - }); - }); + if (column.isCurrentTimeSystem()) { + if (!this.timeSystemColumnTitle) { + this.timeSystemColumnTitle = column.title(); + } + column.title(this.timeSystemColumnTitle); } - return this; - }; - /** - * Get a simple list of column titles - * @returns {Array} The titles of the columns - */ - TableConfiguration.prototype.getHeaders = function () { - return this.columns.map(function (column, i) { - return column.getTitle() || 'Column ' + (i + 1); - }); + this.columns.push(column); + this.headers.add(column.title()); }; /** @@ -98,22 +67,32 @@ define( * @returns {Object} Key value pairs where the key is the column * title, and the value is the formatted value from the provided datum. */ - TableConfiguration.prototype.getRowValues = function (limitEvaluator, datum) { - return this.columns.reduce(function (rowObject, column, i) { - var columnTitle = column.getTitle() || 'Column ' + (i + 1), - columnValue = column.getValue(datum, limitEvaluator); - - if (columnValue !== undefined && columnValue.text === undefined) { - columnValue.text = ''; - } - // Don't replace something with nothing. - // This occurs when there are multiple columns with the same - // column title - if (rowObject[columnTitle] === undefined || - rowObject[columnTitle].text === undefined || - rowObject[columnTitle].text.length === 0) { + TableConfiguration.prototype.getRowValues = function (telemetryObject, limitEvaluator, datum) { + return this.columns.reduce(function (rowObject, column) { + var columnTitle = column.title(); + var columnValue = { + text: '', + value: undefined + }; + if (rowObject[columnTitle] === undefined) { rowObject[columnTitle] = columnValue; } + + if (column.hasValue(telemetryObject, datum)) { + columnValue = column.getValue(datum, limitEvaluator); + + if (columnValue.text === undefined) { + columnValue.text = ''; + } + // Don't replace something with nothing. + // This occurs when there are multiple columns with the same + // column title + if (rowObject[columnTitle].text === undefined || + rowObject[columnTitle].text.length === 0) { + rowObject[columnTitle] = columnValue; + } + } + return rowObject; }, {}); }; @@ -164,7 +143,7 @@ define( * specifying whether the column is visible or not. Default to * existing (persisted) configuration if available */ - this.getHeaders().forEach(function (columnTitle) { + this.headers.forEach(function (columnTitle) { configuration[columnTitle] = typeof defaultConfig[columnTitle] === 'undefined' ? true : defaultConfig[columnTitle]; diff --git a/platform/features/table/src/TelemetryCollection.js b/platform/features/table/src/TelemetryCollection.js index 24d690c491..69eab0daed 100644 --- a/platform/features/table/src/TelemetryCollection.js +++ b/platform/features/table/src/TelemetryCollection.js @@ -93,7 +93,9 @@ define( // Calculate the new index of the last item in bounds endIndex = _.sortedLastIndex(this.highBuffer, testValue, this.sortField); added = this.highBuffer.splice(0, endIndex); - this.telemetry = this.telemetry.concat(added); + added.forEach(function (datum) { + this.telemetry.push(datum); + }.bind(this)); } if (discarded && discarded.length > 0) { @@ -132,6 +134,7 @@ define( // bounds events, so no bounds checking necessary if (this.sortField === undefined) { this.telemetry.push(item); + return true; } @@ -153,7 +156,6 @@ define( // If out of bounds low, disregard data if (!boundsLow) { - // Going to check for duplicates. Bound the search problem to // items around the given time. Use sortedIndex because it // employs a binary search which is O(log n). Can use binary search diff --git a/platform/features/table/src/controllers/TelemetryTableController.js b/platform/features/table/src/controllers/TelemetryTableController.js index 5a51eb53ff..c89ab09677 100644 --- a/platform/features/table/src/controllers/TelemetryTableController.js +++ b/platform/features/table/src/controllers/TelemetryTableController.js @@ -118,23 +118,18 @@ define( * to sort by. By default will just match on key. * * @private - * @param {TimeSystem} timeSystem */ - TelemetryTableController.prototype.sortByTimeSystem = function (timeSystem) { + TelemetryTableController.prototype.sortByTimeSystem = function () { var scope = this.$scope; var sortColumn; scope.defaultSort = undefined; - if (timeSystem !== undefined) { - this.table.columns.forEach(function (column) { - if (column.getKey() === timeSystem.key) { - sortColumn = column; - } - }); - if (sortColumn) { - scope.defaultSort = sortColumn.getTitle(); - this.telemetry.sort(sortColumn.getTitle() + '.value'); - } + sortColumn = this.table.columns.filter(function (column) { + return column.isCurrentTimeSystem(); + })[0]; + if (sortColumn) { + scope.defaultSort = sortColumn.title(); + this.telemetry.sort(sortColumn.title() + '.value'); } }; @@ -172,9 +167,6 @@ define( * @param rows */ TelemetryTableController.prototype.addRowsToTable = function (rows) { - rows.forEach(function (row) { - this.$scope.rows.push(row); - }, this); this.$scope.$broadcast('add:rows', rows); }; @@ -237,35 +229,21 @@ define( TelemetryTableController.prototype.loadColumns = function (objects) { var telemetryApi = this.openmct.telemetry; + this.table = new TableConfiguration(this.$scope.domainObject, + this.openmct); + this.$scope.headers = []; if (objects.length > 0) { - var allMetadata = objects.map(telemetryApi.getMetadata.bind(telemetryApi)); - var allValueMetadata = _.flatten(allMetadata.map( - function getMetadataValues(metadata) { - return metadata.values(); - } - )); - - this.table.populateColumns(allValueMetadata); - - var domainColumns = telemetryApi.commonValuesForHints(allMetadata, ['domain']); - this.timeColumns = domainColumns.map(function (metadatum) { - return metadatum.name; - }); + objects.forEach(function (object) { + var metadataValues = telemetryApi.getMetadata(object).values(); + metadataValues.forEach(function (metadatum) { + this.table.addColumn(object, metadatum); + }.bind(this)); + }.bind(this)); this.filterColumns(); - - // Default to no sort on underlying telemetry collection. Sorting - // is necessary to do bounds filtering, but this is only possible - // if data matches selected time system - this.telemetry.sort(undefined); - - var timeSystem = this.openmct.time.timeSystem(); - if (timeSystem !== undefined) { - this.sortByTimeSystem(timeSystem); - } - + this.sortByTimeSystem(); } return objects; @@ -302,7 +280,7 @@ define( /* * Process a batch of historical data */ - function processData(historicalData, index, limitEvaluator) { + function processData(object, historicalData, index, limitEvaluator) { if (index >= historicalData.length) { processedObjects++; @@ -311,14 +289,13 @@ define( } } else { rowData = rowData.concat(historicalData.slice(index, index + self.batchSize) - .map(self.table.getRowValues.bind(self.table, limitEvaluator))); - + .map(self.table.getRowValues.bind(self.table, object, limitEvaluator))); /* Use timeout to yield process to other UI activities. On return, process next batch */ self.timeoutHandle = self.$timeout(function () { - processData(historicalData, index + self.batchSize, limitEvaluator); + processData(object, historicalData, index + self.batchSize, limitEvaluator); }); } } @@ -327,7 +304,7 @@ define( // Only process the most recent request if (requestTime === self.lastRequestTime) { var limitEvaluator = openmct.telemetry.limitEvaluator(object); - processData(historicalData, 0, limitEvaluator); + processData(object, historicalData, 0, limitEvaluator); } else { resolve(rowData); } @@ -367,7 +344,6 @@ define( var telemetryCollection = this.telemetry; //Set table max length to avoid unbounded growth. var limitEvaluator; - var added = false; var table = this.table; this.subscriptions.forEach(function (subscription) { @@ -377,7 +353,7 @@ define( function newData(domainObject, datum) { limitEvaluator = telemetryApi.limitEvaluator(domainObject); - added = telemetryCollection.add([table.getRowValues(limitEvaluator, datum)]); + telemetryCollection.add([table.getRowValues(domainObject, limitEvaluator, datum)]); } objects.forEach(function (object) { diff --git a/platform/features/table/test/TableConfigurationSpec.js b/platform/features/table/test/TableConfigurationSpec.js index 54b6efb8c7..02de9168bb 100644 --- a/platform/features/table/test/TableConfigurationSpec.js +++ b/platform/features/table/test/TableConfigurationSpec.js @@ -27,31 +27,52 @@ define( function (Table) { describe("A table", function () { - var mockDomainObject, + var mockTableObject, + mockTelemetryObject, mockAPI, mockTelemetryAPI, table, + mockTimeAPI, + mockObjectsAPI, mockModel; beforeEach(function () { - mockDomainObject = jasmine.createSpyObj('domainObject', + mockTableObject = jasmine.createSpyObj('domainObject', ['getModel', 'useCapability', 'getCapability', 'hasCapability'] ); mockModel = {}; - mockDomainObject.getModel.and.returnValue(mockModel); - mockDomainObject.getCapability.and.callFake(function (name) { + mockTableObject.getModel.and.returnValue(mockModel); + mockTableObject.getCapability.and.callFake(function (name) { return name === 'editor' && { isEditContextRoot: function () { return true; } }; }); + mockTelemetryObject = { + identifier: { + namespace: 'mock', + key: 'domainObject' + } + }; mockTelemetryAPI = jasmine.createSpyObj('telemetryAPI', [ 'getValueFormatter' ]); + mockTimeAPI = jasmine.createSpyObj('timeAPI', [ + 'timeSystem' + ]); + mockObjectsAPI = jasmine.createSpyObj('objectsAPI', [ + 'makeKeyString' + ]); + mockObjectsAPI.makeKeyString.and.callFake(function (identifier) { + return [identifier.namespace, identifier.key].join(':'); + }); + mockAPI = { - telemetry: mockTelemetryAPI + telemetry: mockTelemetryAPI, + time: mockTimeAPI, + objects: mockObjectsAPI }; mockTelemetryAPI.getValueFormatter.and.callFake(function (metadata) { var formatter = jasmine.createSpyObj( @@ -69,7 +90,7 @@ define( return formatter; }); - table = new Table(mockDomainObject, mockAPI); + table = new Table(mockTableObject, mockAPI); }); describe("Building columns from telemetry metadata", function () { @@ -77,51 +98,57 @@ define( { name: 'Range 1', key: 'range1', + source: 'range1', hints: { - y: 1 + range: 1 } }, { name: 'Range 2', key: 'range2', + source: 'range2', hints: { - y: 2 + range: 2 } }, { name: 'Domain 1', key: 'domain1', + source: 'domain1', format: 'utc', hints: { - x: 1 + domain: 1 } }, { name: 'Domain 2', key: 'domain2', + source: 'domain2', format: 'utc', hints: { - x: 2 + domain: 2 } } ]; beforeEach(function () { - table.populateColumns(metadata); + mockTimeAPI.timeSystem.and.returnValue({ + key: 'domain1' + }); + metadata.forEach(function (metadatum) { + table.addColumn(mockTelemetryObject, metadatum); + }); }); it("populates columns", function () { expect(table.columns.length).toBe(4); }); - it("Produces headers for each column based on title", function () { - var headers, - firstColumn = table.columns[0]; - - spyOn(firstColumn, 'getTitle'); - headers = table.getHeaders(); - expect(headers.length).toBe(4); - expect(firstColumn.getTitle).toHaveBeenCalled(); + it("Produces headers for each column based on metadata name", function () { + expect(table.headers.size).toBe(4); + Array.from(table.headers.values).forEach(function (header, i) { + expect(header).toEqual(metadata[i].name); + }); }); it("Provides a default configuration with all columns" + @@ -169,11 +196,10 @@ define( }; } }; - rowValues = table.getRowValues(limitEvaluator, datum); + rowValues = table.getRowValues(mockTelemetryObject, limitEvaluator, datum); }); it("Returns a value for every column", function () { - expect(rowValues['Range 1'].text).toBeDefined(); expect(rowValues['Range 1'].text).toEqual(10); }); diff --git a/platform/features/table/test/controllers/TelemetryTableControllerSpec.js b/platform/features/table/test/controllers/TelemetryTableControllerSpec.js index 2b928f999c..bb52d3153f 100644 --- a/platform/features/table/test/controllers/TelemetryTableControllerSpec.js +++ b/platform/features/table/test/controllers/TelemetryTableControllerSpec.js @@ -78,7 +78,8 @@ define( ]); mockObjectAPI = jasmine.createSpyObj("objectAPI", [ - "observe" + "observe", + "makeKeyString" ]); unobserve = jasmine.createSpy("unobserve"); mockObjectAPI.observe.and.returnValue(unobserve); @@ -184,8 +185,7 @@ define( var mockComposition, mockTelemetryObject, mockChildren, - unsubscribe, - done; + unsubscribe; beforeEach(function () { mockComposition = jasmine.createSpyObj("composition", [ @@ -207,8 +207,6 @@ define( mockTelemetryAPI.isTelemetryObject.and.callFake(function (obj) { return obj.identifier.key === mockTelemetryObject.identifier.key; }); - - done = false; }); it('fetches historical data for the time period specified by the conductor bounds', function () { @@ -292,40 +290,37 @@ define( }); describe('populates table columns', function () { - var domainMetadata; var allMetadata; - var mockTimeSystem; + var mockTimeSystem1; + var mockTimeSystem2; beforeEach(function () { - domainMetadata = [{ - key: "column1", - name: "Column 1", - hints: {} - }]; - allMetadata = [{ key: "column1", name: "Column 1", - hints: {} + hints: { + domain: 1 + } }, { key: "column2", name: "Column 2", - hints: {} + hints: { + domain: 2 + } }, { key: "column3", name: "Column 3", hints: {} }]; - mockTimeSystem = { + mockTimeSystem1 = { key: "column1" }; + mockTimeSystem2 = { + key: "column2" + }; - mockTelemetryAPI.commonValuesForHints.and.callFake(function (metadata, hints) { - if (_.eq(hints, ["domain"])) { - return domainMetadata; - } - }); + mockConductor.timeSystem.and.returnValue(mockTimeSystem1); mockTelemetryAPI.getMetadata.and.returnValue({ values: function () { @@ -345,9 +340,12 @@ define( }); it('and sorts by column matching time system', function () { - expect(mockScope.defaultSort).not.toEqual("Column 1"); - controller.sortByTimeSystem(mockTimeSystem); expect(mockScope.defaultSort).toEqual("Column 1"); + + mockConductor.timeSystem.and.returnValue(mockTimeSystem2); + controller.sortByTimeSystem(); + + expect(mockScope.defaultSort).toEqual("Column 2"); }); it('batches processing of rows for performance when receiving historical telemetry', function () { @@ -403,25 +401,16 @@ define( describe('when telemetry is added', function () { var testRows; - var expectedRows; beforeEach(function () { testRows = [{ a: 0 }, { a: 1 }, { a: 2 }]; - mockScope.rows = [{ a: -1 }]; - expectedRows = mockScope.rows.concat(testRows); - spyOn(controller.telemetry, "on").and.callThrough(); controller.registerChangeListeners(); - - controller.telemetry.on.calls.all().forEach(function (call) { - if (call.args[0] === 'added') { - call.args[1](testRows); - } - }); + controller.telemetry.add(testRows); }); - it("adds it to rows in scope", function () { - expect(mockScope.rows).toEqual(expectedRows); + it("Adds the rows to the MCTTable directive", function () { + expect(mockScope.$broadcast).toHaveBeenCalledWith("add:rows", testRows); }); }); }); diff --git a/src/plugins/localTimeSystem/LocalTimeFormat.js b/src/plugins/localTimeSystem/LocalTimeFormat.js index b1949da35c..86c12ccd6a 100644 --- a/src/plugins/localTimeSystem/LocalTimeFormat.js +++ b/src/plugins/localTimeSystem/LocalTimeFormat.js @@ -113,6 +113,9 @@ define([ }; LocalTimeFormat.prototype.parse = function (text) { + if (typeof text === 'number') { + return text; + } return moment(text, DATE_FORMATS).valueOf(); };