From c78fba4b13326be6a52c8a330c01f02896e372a9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Maciej=20Filip=20Szkodzi=C5=84ski?= Date: Mon, 11 Nov 2013 23:55:18 +0100 Subject: [PATCH] Sets and executes timeouts set during a tick. All timeouts and intervals set during a tick were being scheduled to run at delay + end-of-tick, instead of delay + time-of-outer-timeout. Scheduled run-at times were shifted because currentTime was being incremented before executing scheduled functions. Additionally, the execute loop was iterating over a functions-to-run array, created from scheduledFunctions before starting. Any changes to scheduledFunctions were being ignored during the tick, and the next tick would ignore any functions which should have been executed in the past. The commit is a rewrite of DelayedFunctionScheduler, preserving the public interface. Execution of scheduled functions updates currentTime on each iteration, and each time takes the functions with the lowest runAtMillis from the schedule, if they aren't higher than endTime. --- spec/core/ClockSpec.js | 15 +++ spec/core/DelayedFunctionSchedulerSpec.js | 67 ++++++++++++ src/core/DelayedFunctionScheduler.js | 121 ++++++++++++++-------- 3 files changed, 157 insertions(+), 46 deletions(-) diff --git a/spec/core/ClockSpec.js b/spec/core/ClockSpec.js index 168e17b5..b6ccec5d 100644 --- a/spec/core/ClockSpec.js +++ b/spec/core/ClockSpec.js @@ -341,4 +341,19 @@ describe("Clock (acceptance)", function() { clock.tick(); expect(delayedFn2).toHaveBeenCalled(); }); + + it("correctly calls functions scheduled while the Clock is advancing", function() { + var delayedFn1 = jasmine.createSpy('delayedFn1'), + delayedFn2 = jasmine.createSpy('delayedFn2'), + delayedFunctionScheduler = new j$.DelayedFunctionScheduler(), + clock = new j$.Clock({setTimeout: function() {}}, delayedFunctionScheduler); + + delayedFn1.and.callFake(function() { clock.setTimeout(delayedFn2, 1); }); + clock.install(); + clock.setTimeout(delayedFn1, 5); + + clock.tick(6); + expect(delayedFn1).toHaveBeenCalled(); + expect(delayedFn2).toHaveBeenCalled(); + }); }); diff --git a/spec/core/DelayedFunctionSchedulerSpec.js b/spec/core/DelayedFunctionSchedulerSpec.js index fe5e0acc..6d0698f5 100644 --- a/spec/core/DelayedFunctionSchedulerSpec.js +++ b/spec/core/DelayedFunctionSchedulerSpec.js @@ -175,5 +175,72 @@ describe("DelayedFunctionScheduler", function() { expect(fn).toHaveBeenCalled(); }); + it("schedules a function for later execution during a tick", function () { + var scheduler = new j$.DelayedFunctionScheduler(), + fn = jasmine.createSpy('fn'), + fnDelay = 10; + + scheduler.scheduleFunction(function () { + scheduler.scheduleFunction(fn, fnDelay); + }, 0); + + expect(fn).not.toHaveBeenCalled(); + + scheduler.tick(fnDelay); + + expect(fn).toHaveBeenCalled(); + }); + + it("#removeFunctionWithId removes a previously scheduled function with a given id during a tick", function () { + var scheduler = new j$.DelayedFunctionScheduler(), + fn = jasmine.createSpy('fn'), + fnDelay = 10, + timeoutKey; + + scheduler.scheduleFunction(function () { + scheduler.removeFunctionWithId(timeoutKey); + }, 0); + timeoutKey = scheduler.scheduleFunction(fn, fnDelay); + + expect(fn).not.toHaveBeenCalled(); + + scheduler.tick(fnDelay); + + expect(fn).not.toHaveBeenCalled(); + }); + + it("executes recurring functions interleaved with regular functions and functions scheduled during a tick in the correct order", function () { + var scheduler = new j$.DelayedFunctionScheduler(), + fn = jasmine.createSpy('fn'), + recurringCallCount = 0, + recurring = jasmine.createSpy('recurring').and.callFake(function() { + recurringCallCount++; + if (recurringCallCount < 5) { + expect(fn).not.toHaveBeenCalled(); + } + }), + innerFn = jasmine.createSpy('innerFn').and.callFake(function() { + expect(recurring.calls.count()).toBe(4); + expect(fn).not.toHaveBeenCalled(); + }), + scheduling = jasmine.createSpy('scheduling').and.callFake(function() { + expect(recurring.calls.count()).toBe(3); + expect(fn).not.toHaveBeenCalled(); + scheduler.scheduleFunction(innerFn, 10); // 41ms absolute + }); + + scheduler.scheduleFunction(recurring, 10, [], true); + scheduler.scheduleFunction(fn, 50); + scheduler.scheduleFunction(scheduling, 31); + + scheduler.tick(60); + + expect(recurring).toHaveBeenCalled(); + expect(recurring.calls.count()).toBe(6); + expect(fn).toHaveBeenCalled(); + expect(scheduling).toHaveBeenCalled(); + expect(innerFn).toHaveBeenCalled(); + }); + }); diff --git a/src/core/DelayedFunctionScheduler.js b/src/core/DelayedFunctionScheduler.js index ef004510..1efdce66 100644 --- a/src/core/DelayedFunctionScheduler.js +++ b/src/core/DelayedFunctionScheduler.js @@ -1,14 +1,16 @@ getJasmineRequireObj().DelayedFunctionScheduler = function() { function DelayedFunctionScheduler() { var self = this; + var scheduledLookup = []; var scheduledFunctions = {}; var currentTime = 0; var delayedFnCount = 0; self.tick = function(millis) { millis = millis || 0; - currentTime = currentTime + millis; - runFunctionsWithinRange(currentTime - millis, currentTime); + var endTime = currentTime + millis; + + runScheduledFunctions(endTime); }; self.scheduleFunction = function(funcToCall, millis, params, recurring, timeoutKey, runAtMillis) { @@ -24,7 +26,8 @@ getJasmineRequireObj().DelayedFunctionScheduler = function() { millis = millis || 0; timeoutKey = timeoutKey || ++delayedFnCount; runAtMillis = runAtMillis || (currentTime + millis); - scheduledFunctions[timeoutKey] = { + + var funcToSchedule = { runAtMillis: runAtMillis, funcToCall: f, recurring: recurring, @@ -32,58 +35,73 @@ getJasmineRequireObj().DelayedFunctionScheduler = function() { timeoutKey: timeoutKey, millis: millis }; + + if (runAtMillis in scheduledFunctions) { + scheduledFunctions[runAtMillis].push(funcToSchedule); + } else { + scheduledFunctions[runAtMillis] = [funcToSchedule]; + scheduledLookup.push(runAtMillis); + scheduledLookup.sort(function (a, b) { + return a - b; + }); + } + return timeoutKey; }; self.removeFunctionWithId = function(timeoutKey) { - delete scheduledFunctions[timeoutKey]; + for (var runAtMillis in scheduledFunctions) { + var funcs = scheduledFunctions[runAtMillis]; + var i = indexOfFirstToPass(funcs, function (func) { + return func.timeoutKey === timeoutKey; + }); + + if (i > -1) { + if (funcs.length === 1) { + delete scheduledFunctions[runAtMillis]; + deleteFromLookup(runAtMillis); + } else { + funcs.splice(i, 1); + } + + // intervals get rescheduled when executed, so there's never more + // than a single scheduled function with a given timeoutKey + break; + } + } }; self.reset = function() { currentTime = 0; + scheduledLookup = []; scheduledFunctions = {}; delayedFnCount = 0; }; return self; - // finds/dupes functions within range and removes them. - function functionsWithinRange(startMillis, endMillis) { - var fnsToRun = []; - for (var timeoutKey in scheduledFunctions) { - var scheduledFunc = scheduledFunctions[timeoutKey]; - if (scheduledFunc && - scheduledFunc.runAtMillis >= startMillis && - scheduledFunc.runAtMillis <= endMillis) { + function indexOfFirstToPass(array, testFn) { + var index = -1; - // remove fn -- we'll reschedule later if it is recurring. - self.removeFunctionWithId(timeoutKey); - if (!scheduledFunc.recurring) { - fnsToRun.push(scheduledFunc); // schedules each function only once - } else { - fnsToRun.push(buildNthInstanceOf(scheduledFunc, 0)); - var additionalTimesFnRunsInRange = - Math.floor((endMillis - scheduledFunc.runAtMillis) / scheduledFunc.millis); - for (var i = 0; i < additionalTimesFnRunsInRange; i++) { - fnsToRun.push(buildNthInstanceOf(scheduledFunc, i + 1)); - } - reschedule(buildNthInstanceOf(scheduledFunc, additionalTimesFnRunsInRange)); - } + for (var i = 0; i < array.length; ++i) { + if (testFn(array[i])) { + index = i; + break; } } - return fnsToRun; + return index; } - function buildNthInstanceOf(scheduledFunc, n) { - return { - runAtMillis: scheduledFunc.runAtMillis + (scheduledFunc.millis * n), - funcToCall: scheduledFunc.funcToCall, - params: scheduledFunc.params, - millis: scheduledFunc.millis, - recurring: scheduledFunc.recurring, - timeoutKey: scheduledFunc.timeoutKey - }; + function deleteFromLookup(key) { + var value = Number(key); + var i = indexOfFirstToPass(scheduledLookup, function (millis) { + return millis === value; + }); + + if (i > -1) { + scheduledLookup.splice(i, 1); + } } function reschedule(scheduledFn) { @@ -95,21 +113,32 @@ getJasmineRequireObj().DelayedFunctionScheduler = function() { scheduledFn.runAtMillis + scheduledFn.millis); } - - function runFunctionsWithinRange(startMillis, endMillis) { - var funcsToRun = functionsWithinRange(startMillis, endMillis); - if (funcsToRun.length === 0) { + function runScheduledFunctions(endTime) { + if (scheduledLookup.length === 0 || scheduledLookup[0] > endTime) { return; } - funcsToRun.sort(function(a, b) { - return a.runAtMillis - b.runAtMillis; - }); + do { + currentTime = scheduledLookup.shift(); - for (var i = 0; i < funcsToRun.length; ++i) { - var funcToRun = funcsToRun[i]; - funcToRun.funcToCall.apply(null, funcToRun.params || []); - } + var funcsToRun = scheduledFunctions[currentTime]; + delete scheduledFunctions[currentTime]; + + for (var i = 0; i < funcsToRun.length; ++i) { + var funcToRun = funcsToRun[i]; + funcToRun.funcToCall.apply(null, funcToRun.params || []); + + if (funcToRun.recurring) { + reschedule(funcToRun); + } + } + } while (scheduledLookup.length > 0 && + // checking first if we're out of time prevents setTimeout(0) + // scheduled in a funcToRun from forcing an extra iteration + currentTime !== endTime && + scheduledLookup[0] <= endTime); + + currentTime = endTime; } }