Skip to content

Commit

Permalink
perf: Do not add/remove listeners for each timer (#6144)
Browse files Browse the repository at this point in the history
  • Loading branch information
@brandonocasey @gkatsev
brandonocasey authored and gkatsev committed Aug 7, 2019
1 parent bd51e9e commit 5ee2477
Show file tree
Hide file tree
Showing 2 changed files with 188 additions and 61 deletions.
125 changes: 74 additions & 51 deletions src/js/component.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -98,6 +98,11 @@ class Component {
this.childIndex_ = {};
this.childNameIndex_ = {};

this.setTimeoutIds_ = new Set();
this.setIntervalIds_ = new Set();
this.rafIds_ = new Set();
this.clearingTimersOnDispose_ = false;

// Add any child components in options
if (options.initChildren !== false) {
this.initChildren();
Expand Down Expand Up @@ -1293,16 +1298,16 @@ class Component {

fn = Fn.bind(this, fn);

this.clearTimersOnDispose_();

timeoutId = window.setTimeout(() => {
this.off('dispose', disposeFn);
if (this.setTimeoutIds_.has(timeoutId)) {
this.setTimeoutIds_.delete(timeoutId);
}
fn();
}, timeout);

disposeFn = () => this.clearTimeout(timeoutId);

disposeFn.guid = `vjs-timeout-${timeoutId}`;

this.on('dispose', disposeFn);
this.setTimeoutIds_.add(timeoutId);

return timeoutId;
}
Expand All @@ -1323,13 +1328,10 @@ class Component {
* @see [Similar to]{@link https://developer.mozilla.org/en-US/docs/Web/API/WindowTimers/clearTimeout}
*/
clearTimeout(timeoutId) {
window.clearTimeout(timeoutId);

const disposeFn = function() {};

disposeFn.guid = `vjs-timeout-${timeoutId}`;

this.off('dispose', disposeFn);
if (this.setTimeoutIds_.has(timeoutId)) {
this.setTimeoutIds_.delete(timeoutId);
window.clearTimeout(timeoutId);
}

return timeoutId;
}
Expand Down Expand Up @@ -1357,13 +1359,11 @@ class Component {
setInterval(fn, interval) {
fn = Fn.bind(this, fn);

const intervalId = window.setInterval(fn, interval);

const disposeFn = () => this.clearInterval(intervalId);
this.clearTimersOnDispose_();

disposeFn.guid = `vjs-interval-${intervalId}`;
const intervalId = window.setInterval(fn, interval);

this.on('dispose', disposeFn);
this.setIntervalIds_.add(intervalId);

return intervalId;
}
Expand All @@ -1384,13 +1384,10 @@ class Component {
* @see [Similar to]{@link https://developer.mozilla.org/en-US/docs/Web/API/WindowTimers/clearInterval}
*/
clearInterval(intervalId) {
window.clearInterval(intervalId);

const disposeFn = function() {};

disposeFn.guid = `vjs-interval-${intervalId}`;

this.off('dispose', disposeFn);
if (this.setIntervalIds_.has(intervalId)) {
this.setIntervalIds_.delete(intervalId);
window.clearInterval(intervalId);
}

return intervalId;
}
Expand Down Expand Up @@ -1421,28 +1418,27 @@ class Component {
* @see [Similar to]{@link https://developer.mozilla.org/en-US/docs/Web/API/window/requestAnimationFrame}
*/
requestAnimationFrame(fn) {
// declare as variables so they are properly available in rAF function
// eslint-disable-next-line
var id, disposeFn;

if (this.supportsRaf_) {
fn = Fn.bind(this, fn);

id = window.requestAnimationFrame(() => {
this.off('dispose', disposeFn);
fn();
});
// Fall back to using a timer.
if (!this.supportsRaf_) {
return this.setTimeout(fn, 1000 / 60);
}

disposeFn = () => this.cancelAnimationFrame(id);
this.clearTimersOnDispose_();

disposeFn.guid = `vjs-raf-${id}`;
this.on('dispose', disposeFn);
// declare as variables so they are properly available in rAF function
// eslint-disable-next-line
var id;
fn = Fn.bind(this, fn);

return id;
}
id = window.requestAnimationFrame(() => {
if (this.rafIds_.has(id)) {
this.rafIds_.delete(id);
}
fn();
});
this.rafIds_.add(id);

// Fall back to using a timer.
return this.setTimeout(fn, 1000 / 60);
return id;
}

/**
Expand All @@ -1462,20 +1458,47 @@ class Component {
* @see [Similar to]{@link https://developer.mozilla.org/en-US/docs/Web/API/window/cancelAnimationFrame}
*/
cancelAnimationFrame(id) {
if (this.supportsRaf_) {
window.cancelAnimationFrame(id);
// Fall back to using a timer.
if (!this.supportsRaf_) {
return this.clearTimeout(id);
}

const disposeFn = function() {};
if (this.rafIds_.has(id)) {
this.rafIds_.delete(id);
window.cancelAnimationFrame(id);
}

disposeFn.guid = `vjs-raf-${id}`;
return id;

this.off('dispose', disposeFn);
}

return id;
/**
* A function to setup `requestAnimationFrame`, `setTimeout`,
* and `setInterval`, clearing on dispose.
*
* > Previously each timer added and removed dispose listeners on it's own.
* For better performance it was decided to batch them all, and use `Set`s
* to track outstanding timer ids.
*
* @private
*/
clearTimersOnDispose_() {
if (this.clearingTimersOnDispose_) {
return;
}

// Fall back to using a timer.
return this.clearTimeout(id);
this.clearingTimersOnDispose_ = true;
this.one('dispose', () => {
[
['rafIds_', 'cancelAnimationFrame'],
['setTimeoutIds_', 'clearTimeout'],
['setIntervalIds_', 'clearInterval']
].forEach(([idName, cancelName]) => {
this[idName].forEach(this[cancelName], this);
});

this.clearingTimersOnDispose_ = false;
});
}

/**
Expand Down
124 changes: 114 additions & 10 deletions test/unit/component.test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -978,25 +978,20 @@ QUnit.test('*AnimationFrame methods fall back to timers if rAF not supported', f

QUnit.test('setTimeout should remove dispose handler on trigger', function(assert) {
const comp = new Component(getFakePlayer());
const el = comp.el();
const data = DomData.get(el);

comp.setTimeout(() => {}, 1);

assert.equal(data.handlers.dispose.length, 2, 'we got a new dispose handler');
assert.ok(/vjs-timeout-\d/.test(data.handlers.dispose[1].guid), 'we got a new dispose handler');
assert.equal(comp.setTimeoutIds_.size, 1, 'we removed our dispose handle');

this.clock.tick(1);

assert.equal(data.handlers.dispose.length, 1, 'we removed our dispose handle');
assert.equal(comp.setTimeoutIds_.size, 0, 'we removed our dispose handle');

comp.dispose();
});

QUnit.test('requestAnimationFrame should remove dispose handler on trigger', function(assert) {
const comp = new Component(getFakePlayer());
const el = comp.el();
const data = DomData.get(el);
const oldRAF = window.requestAnimationFrame;
const oldCAF = window.cancelAnimationFrame;

Expand All @@ -1012,19 +1007,128 @@ QUnit.test('requestAnimationFrame should remove dispose handler on trigger', fun

comp.requestAnimationFrame(spyRAF);

assert.equal(data.handlers.dispose.length, 2, 'we got a new dispose handler');
assert.ok(/vjs-raf-\d/.test(data.handlers.dispose[1].guid), 'we got a new dispose handler');
assert.equal(comp.rafIds_.size, 1, 'we got a new dispose handler');

this.clock.tick(1);

assert.equal(comp.rafIds_.size, 0, 'we removed our dispose handle');

comp.dispose();

window.requestAnimationFrame = oldRAF;
window.cancelAnimationFrame = oldCAF;
});

QUnit.test('requestAnimationFrame should remove dispose handler on trigger', function(assert) {
const comp = new Component(getFakePlayer());
const oldRAF = window.requestAnimationFrame;
const oldCAF = window.cancelAnimationFrame;

// Stub the window.*AnimationFrame methods with window.setTimeout methods
// so we can control when the callbacks are called via sinon's timer stubs.
window.requestAnimationFrame = (fn) => window.setTimeout(fn, 1);
window.cancelAnimationFrame = (id) => window.clearTimeout(id);

// Make sure the component thinks it supports rAF.
comp.supportsRaf_ = true;

const spyRAF = sinon.spy();

comp.requestAnimationFrame(spyRAF);

assert.equal(comp.rafIds_.size, 1, 'we got a new dispose handler');

this.clock.tick(1);

assert.equal(data.handlers.dispose.length, 1, 'we removed our dispose handle');
assert.equal(comp.rafIds_.size, 0, 'we removed our dispose handle');

comp.dispose();

window.requestAnimationFrame = oldRAF;
window.cancelAnimationFrame = oldCAF;
});

QUnit.test('setTimeout should be canceled on dispose', function(assert) {
const comp = new Component(getFakePlayer());
let called = false;
let clearId;
const setId = comp.setTimeout(() => {
called = true;
}, 1);

const clearTimeout = comp.clearTimeout;

comp.clearTimeout = (id) => {
clearId = id;
return clearTimeout.call(comp, id);
};

assert.equal(comp.setTimeoutIds_.size, 1, 'we added a timeout id');

comp.dispose();

assert.equal(comp.setTimeoutIds_.size, 0, 'we removed our timeout id');
assert.equal(clearId, setId, 'clearTimeout was called');

this.clock.tick(1);

assert.equal(called, false, 'setTimeout was never called');
});

QUnit.test('requestAnimationFrame should be canceled on dispose', function(assert) {
const comp = new Component(getFakePlayer());
let called = false;
let clearId;
const setId = comp.requestAnimationFrame(() => {
called = true;
});

const cancelAnimationFrame = comp.cancelAnimationFrame;

comp.cancelAnimationFrame = (id) => {
clearId = id;
return cancelAnimationFrame.call(comp, id);
};

assert.equal(comp.rafIds_.size, 1, 'we added a raf id');

comp.dispose();

assert.equal(comp.rafIds_.size, 0, 'we removed a raf id');
assert.equal(clearId, setId, 'clearAnimationFrame was called');

this.clock.tick(1);

assert.equal(called, false, 'requestAnimationFrame was never called');
});

QUnit.test('setInterval should be canceled on dispose', function(assert) {
const comp = new Component(getFakePlayer());
let called = false;
let clearId;
const setId = comp.setInterval(() => {
called = true;
});

const clearInterval = comp.clearInterval;

comp.clearInterval = (id) => {
clearId = id;
return clearInterval.call(comp, id);
};

assert.equal(comp.setIntervalIds_.size, 1, 'we added an interval id');

comp.dispose();

assert.equal(comp.setIntervalIds_.size, 0, 'we removed a raf id');
assert.equal(clearId, setId, 'clearInterval was called');

this.clock.tick(1);

assert.equal(called, false, 'setInterval was never called');
});

QUnit.test('$ and $$ functions', function(assert) {
const comp = new Component(getFakePlayer());
const contentEl = document.createElement('div');
Expand Down

0 comments on commit 5ee2477

Please sign in to comment.