From d1a09c018049db021036a6ef7f54b60883c7da05 Mon Sep 17 00:00:00 2001 From: Victor Woeltjen Date: Fri, 29 May 2015 14:51:16 -0700 Subject: [PATCH 1/5] [Telemetry] Remove linear search Remove linear search for unused positions when queuing telemetry updates; instead, track available positions such that insertion is more performant at a modest cost (bound to the number of subscriptions) of retrieval. Additionally, add implementation notes in-line. WTD-1202. --- platform/telemetry/src/TelemetryQueue.js | 60 ++++++++++++++++++++---- 1 file changed, 51 insertions(+), 9 deletions(-) diff --git a/platform/telemetry/src/TelemetryQueue.js b/platform/telemetry/src/TelemetryQueue.js index bd463b61be..cf82d08ff8 100644 --- a/platform/telemetry/src/TelemetryQueue.js +++ b/platform/telemetry/src/TelemetryQueue.js @@ -35,28 +35,68 @@ define( * @constructor */ function TelemetryQueue() { - var queue = []; + // General approach here: + // * Maintain a queue as an array of objects containing key-value + // pairs. Putting values into the queue will assign to the + // earliest-available queue position for the associated key + // (appending to the array if necessary.) + // * Maintain a set of counts for each key, such that determining + // the next available queue position is easy; O(1) insertion. + // * When retrieving objects, pop off the queue and decrement + // counts. This provides O(n+k) or O(k) retrieval for a queue + // of length n with k unique keys; this depends on whether + // the browser's implementation of Array.prototype.shift is + // O(n) or O(1). + + // Graphically (indexes at top, keys along side, values as *'s), + // if we have a queue that looks like: + // 0 1 2 3 4 + // a * * * * * + // b * * + // c * * * + // + // And we put a new value for b, we expect: + // 0 1 2 3 4 + // a * * * * * + // b * * * + // c * * * + var queue = [], + counts = {}; // Look up an object in the queue that does not have a value // assigned to this key (or, add a new one) function getFreeObject(key) { - var index = 0, object; + var index = counts[key] || 0, object; - // Look for an existing queue position where we can store - // a value to this key without overwriting an existing value. - for (index = 0; index < queue.length; index += 1) { - if (queue[index][key] === undefined) { - return queue[index]; - } + // Track the largest free position for this key + counts[key] = index + 1; + + // If it's before the end of the queue, add it there + if (index < queue.length) { + return queue[index]; } - // If we made it through the loop, values have been assigned + // Otherwise, values have been assigned // to that key in all queued containers, so we need to queue // up a new container for key-value pairs. object = {}; queue.push(object); return object; } + + // Decrement counts for a specific key + function decrementCount(key) { + if (counts[key] < 2) { + delete counts[key]; + } else { + counts[key] -= 1; + } + } + + // Decrement all counts + function decrementCounts() { + Object.keys(counts).forEach(decrementCount); + } return { /** @@ -74,6 +114,8 @@ define( * @return {object} key-value pairs */ poll: function () { + // Decrement counts for the object that will be popped + decrementCounts(); return queue.shift(); }, /** From e06d11dcb2c2c10c2be4446f2875846f20d3f64d Mon Sep 17 00:00:00 2001 From: Victor Woeltjen Date: Fri, 29 May 2015 15:50:30 -0700 Subject: [PATCH 2/5] [Core] Add throttle service Add service for throttling function calls; specifically supports reducing tick mark recalculation, WTD-1202. --- platform/core/bundle.json | 5 ++ platform/core/src/services/Throttle.js | 63 +++++++++++++++++++++ platform/core/test/services/ThrottleSpec.js | 49 ++++++++++++++++ platform/core/test/suite.json | 1 + 4 files changed, 118 insertions(+) create mode 100644 platform/core/src/services/Throttle.js create mode 100644 platform/core/test/services/ThrottleSpec.js diff --git a/platform/core/bundle.json b/platform/core/bundle.json index 5a2727eb75..3f7376f3f3 100644 --- a/platform/core/bundle.json +++ b/platform/core/bundle.json @@ -180,6 +180,11 @@ { "key": "now", "implementation": "services/Now.js" + }, + { + "key": "throttle", + "implementation": "services/Throttle.js", + "depends": [ "$timeout" ] } ], "roots": [ diff --git a/platform/core/src/services/Throttle.js b/platform/core/src/services/Throttle.js new file mode 100644 index 0000000000..619f53161e --- /dev/null +++ b/platform/core/src/services/Throttle.js @@ -0,0 +1,63 @@ +/*global define*/ + +define( + [], + function () { + "use strict"; + + /** + * Throttler for function executions, registered as the `throttle` + * service. + * + * Usage: + * + * throttle(fn, delay, [apply]) + * + * Returns a function that, when invoked, will invoke `fn` after + * `delay` milliseconds, only if no other invocations are pending. + * The optional argument `apply` determines whether. + * + * The returned function will itself return a `Promise` which will + * resolve to the returned value of `fn` whenever that is invoked. + * + * @returns {Function} + */ + function Throttle($timeout) { + /** + * Throttle this function. + * @param {Function} fn the function to throttle + * @param {number} [delay] the delay, in milliseconds, before + * executing this function; defaults to 0. + * @param {boolean} apply true if a `$apply` call should be + * invoked after this function executes; defaults to + * `false`. + */ + return function (fn, delay, apply) { + var activeTimeout; + + // Clear active timeout, so that next invocation starts + // a new one. + function clearActiveTimeout() { + activeTimeout = undefined; + } + + // Defaults + delay = delay || 0; + apply = apply || false; + + return function () { + // Start a timeout if needed + if (!activeTimeout) { + activeTimeout = $timeout(fn, delay, apply); + activeTimeout.then(clearActiveTimeout); + } + // Return whichever timeout is active (to get + // a promise for the results of fn) + return activeTimeout; + }; + }; + } + + return Throttle; + } +); \ No newline at end of file diff --git a/platform/core/test/services/ThrottleSpec.js b/platform/core/test/services/ThrottleSpec.js new file mode 100644 index 0000000000..ccd6644eb7 --- /dev/null +++ b/platform/core/test/services/ThrottleSpec.js @@ -0,0 +1,49 @@ +/*global define,Promise,describe,it,expect,beforeEach,waitsFor,jasmine*/ + +define( + ["../../src/services/Throttle"], + function (Throttle) { + "use strict"; + + describe("The 'throttle' service", function () { + var throttle, + mockTimeout, + mockFn, + mockPromise; + + beforeEach(function () { + mockTimeout = jasmine.createSpy("$timeout"); + mockPromise = jasmine.createSpyObj("promise", ["then"]); + mockFn = jasmine.createSpy("fn"); + mockTimeout.andReturn(mockPromise); + throttle = new Throttle(mockTimeout); + }); + + it("provides functions which run on a timeout", function () { + var throttled = throttle(mockFn); + // Verify precondition: Not called at throttle-time + expect(mockTimeout).not.toHaveBeenCalled(); + expect(throttled()).toEqual(mockPromise); + expect(mockTimeout).toHaveBeenCalledWith(mockFn, 0, false); + }); + + it("schedules only one timeout at a time", function () { + var throttled = throttle(mockFn); + throttled(); + throttled(); + throttled(); + expect(mockTimeout.calls.length).toEqual(1); + }); + + it("schedules additional invocations after resolution", function () { + var throttled = throttle(mockFn); + throttled(); + mockPromise.then.mostRecentCall.args[0](); // Resolve timeout + throttled(); + mockPromise.then.mostRecentCall.args[0](); + throttled(); + expect(mockTimeout.calls.length).toEqual(3); + }); + }); + } +); \ No newline at end of file diff --git a/platform/core/test/suite.json b/platform/core/test/suite.json index 36f3e81980..5fd8f97810 100644 --- a/platform/core/test/suite.json +++ b/platform/core/test/suite.json @@ -23,6 +23,7 @@ "objects/DomainObjectProvider", "services/Now", + "services/Throttle", "types/MergeModels", "types/TypeCapability", From 35b5fbefd0801a3dd0d77e012ebc599ca2d70738 Mon Sep 17 00:00:00 2001 From: Victor Woeltjen Date: Fri, 29 May 2015 16:35:14 -0700 Subject: [PATCH 3/5] [Plot] Throttle updates Throttle plot updates to subplots; WTD-1202. --- platform/features/plot/bundle.json | 2 +- platform/features/plot/src/PlotController.js | 13 +++++++++---- platform/features/plot/src/modes/PlotOverlayMode.js | 2 -- platform/features/plot/src/modes/PlotStackMode.js | 2 -- 4 files changed, 10 insertions(+), 9 deletions(-) diff --git a/platform/features/plot/bundle.json b/platform/features/plot/bundle.json index ab8cf89b7c..21d98cc4e7 100644 --- a/platform/features/plot/bundle.json +++ b/platform/features/plot/bundle.json @@ -22,7 +22,7 @@ { "key": "PlotController", "implementation": "PlotController.js", - "depends": [ "$scope", "telemetryFormatter", "telemetryHandler" ] + "depends": [ "$scope", "telemetryFormatter", "telemetryHandler", "throttle" ] } ] } diff --git a/platform/features/plot/src/PlotController.js b/platform/features/plot/src/PlotController.js index 0f06ddccb4..fcce051968 100644 --- a/platform/features/plot/src/PlotController.js +++ b/platform/features/plot/src/PlotController.js @@ -51,13 +51,14 @@ define( * * @constructor */ - function PlotController($scope, telemetryFormatter, telemetryHandler) { + function PlotController($scope, telemetryFormatter, telemetryHandler, throttle) { var subPlotFactory = new SubPlotFactory(telemetryFormatter), modeOptions = new PlotModeOptions([], subPlotFactory), subplots = [], cachedObjects = [], updater, handle, + scheduleUpdate, domainOffset; // Populate the scope with axis information (specifically, options @@ -89,9 +90,7 @@ define( // Update all sub-plots function update() { - modeOptions.getModeHandler() - .getSubPlots() - .forEach(updateSubplot); + scheduleUpdate(); } // Reinstantiate the plot updater (e.g. because we have a @@ -162,6 +161,12 @@ define( // Unsubscribe when the plot is destroyed $scope.$on("$destroy", releaseSubscription); + + // Create a throttled update function + scheduleUpdate = throttle(function () { + modeOptions.getModeHandler().getSubPlots() + .forEach(updateSubplot); + }); return { /** diff --git a/platform/features/plot/src/modes/PlotOverlayMode.js b/platform/features/plot/src/modes/PlotOverlayMode.js index ec32f2300d..501d4b0e78 100644 --- a/platform/features/plot/src/modes/PlotOverlayMode.js +++ b/platform/features/plot/src/modes/PlotOverlayMode.js @@ -62,8 +62,6 @@ define( points: buf.getLength() }; }); - - subplot.update(); } return { diff --git a/platform/features/plot/src/modes/PlotStackMode.js b/platform/features/plot/src/modes/PlotStackMode.js index 4b6c5cbbb9..5d54b461f1 100644 --- a/platform/features/plot/src/modes/PlotStackMode.js +++ b/platform/features/plot/src/modes/PlotStackMode.js @@ -58,8 +58,6 @@ define( color: PlotPalette.getFloatColor(0), points: buffer.getLength() }]; - - subplot.update(); } function plotTelemetry(prepared) { From 500d88b5a195691f5bae2bfcb9f8558844f590ca Mon Sep 17 00:00:00 2001 From: Victor Woeltjen Date: Wed, 10 Jun 2015 17:02:16 -0700 Subject: [PATCH 4/5] [Plot] Update spec Update spec for PlotController to account for usage of the throttle service, WTD-1202. --- platform/features/plot/test/PlotControllerSpec.js | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/platform/features/plot/test/PlotControllerSpec.js b/platform/features/plot/test/PlotControllerSpec.js index 1cd4b43331..138071d339 100644 --- a/platform/features/plot/test/PlotControllerSpec.js +++ b/platform/features/plot/test/PlotControllerSpec.js @@ -33,6 +33,7 @@ define( var mockScope, mockFormatter, mockHandler, + mockThrottle, mockHandle, mockDomainObject, mockSeries, @@ -56,6 +57,7 @@ define( "telemetrySubscriber", ["handle"] ); + mockThrottle = jasmine.createSpy("throttle"); mockHandle = jasmine.createSpyObj( "subscription", [ @@ -73,12 +75,18 @@ define( ); mockHandler.handle.andReturn(mockHandle); + mockThrottle.andCallFake(function (fn) { return fn; }); mockHandle.getTelemetryObjects.andReturn([mockDomainObject]); mockHandle.getMetadata.andReturn([{}]); mockHandle.getDomainValue.andReturn(123); mockHandle.getRangeValue.andReturn(42); - controller = new PlotController(mockScope, mockFormatter, mockHandler); + controller = new PlotController( + mockScope, + mockFormatter, + mockHandler, + mockThrottle + ); }); it("provides plot colors", function () { @@ -224,4 +232,4 @@ define( }); }); } -); \ No newline at end of file +); From 60296b532357e349e8bddcda783f334ecf031384 Mon Sep 17 00:00:00 2001 From: Victor Woeltjen Date: Wed, 10 Jun 2015 17:05:28 -0700 Subject: [PATCH 5/5] [Core] Add newlines Add newlines to scripts added to core for WTD-1202. --- platform/core/src/services/Throttle.js | 18 +++++++++--------- platform/core/test/services/ThrottleSpec.js | 10 +++++----- 2 files changed, 14 insertions(+), 14 deletions(-) diff --git a/platform/core/src/services/Throttle.js b/platform/core/src/services/Throttle.js index 619f53161e..0c86a403c7 100644 --- a/platform/core/src/services/Throttle.js +++ b/platform/core/src/services/Throttle.js @@ -4,7 +4,7 @@ define( [], function () { "use strict"; - + /** * Throttler for function executions, registered as the `throttle` * service. @@ -13,7 +13,7 @@ define( * * throttle(fn, delay, [apply]) * - * Returns a function that, when invoked, will invoke `fn` after + * Returns a function that, when invoked, will invoke `fn` after * `delay` milliseconds, only if no other invocations are pending. * The optional argument `apply` determines whether. * @@ -28,36 +28,36 @@ define( * @param {Function} fn the function to throttle * @param {number} [delay] the delay, in milliseconds, before * executing this function; defaults to 0. - * @param {boolean} apply true if a `$apply` call should be + * @param {boolean} apply true if a `$apply` call should be * invoked after this function executes; defaults to * `false`. */ return function (fn, delay, apply) { var activeTimeout; - + // Clear active timeout, so that next invocation starts // a new one. function clearActiveTimeout() { activeTimeout = undefined; } - + // Defaults delay = delay || 0; apply = apply || false; - + return function () { // Start a timeout if needed if (!activeTimeout) { activeTimeout = $timeout(fn, delay, apply); activeTimeout.then(clearActiveTimeout); } - // Return whichever timeout is active (to get + // Return whichever timeout is active (to get // a promise for the results of fn) return activeTimeout; }; }; } - + return Throttle; } -); \ No newline at end of file +); diff --git a/platform/core/test/services/ThrottleSpec.js b/platform/core/test/services/ThrottleSpec.js index ccd6644eb7..173fad8006 100644 --- a/platform/core/test/services/ThrottleSpec.js +++ b/platform/core/test/services/ThrottleSpec.js @@ -10,7 +10,7 @@ define( mockTimeout, mockFn, mockPromise; - + beforeEach(function () { mockTimeout = jasmine.createSpy("$timeout"); mockPromise = jasmine.createSpyObj("promise", ["then"]); @@ -18,7 +18,7 @@ define( mockTimeout.andReturn(mockPromise); throttle = new Throttle(mockTimeout); }); - + it("provides functions which run on a timeout", function () { var throttled = throttle(mockFn); // Verify precondition: Not called at throttle-time @@ -26,7 +26,7 @@ define( expect(throttled()).toEqual(mockPromise); expect(mockTimeout).toHaveBeenCalledWith(mockFn, 0, false); }); - + it("schedules only one timeout at a time", function () { var throttled = throttle(mockFn); throttled(); @@ -34,7 +34,7 @@ define( throttled(); expect(mockTimeout.calls.length).toEqual(1); }); - + it("schedules additional invocations after resolution", function () { var throttled = throttle(mockFn); throttled(); @@ -46,4 +46,4 @@ define( }); }); } -); \ No newline at end of file +);