perf: Cache currentTime and buffered from Flash (#3705)

Calling into the SWF too often is expensive. Current time and buffered don't actually change that often but it's very common to call them a couple times in the handling of a single event. Cache their return values for 100ms so the performance penalty of going through ExternalInterface is limited.

src/js/tech/flash-cache.js

@@ -0,0 +1,31 @@
+/**
+ * @file flash-cache.js
+ *
+ * Auto-caching method wrapper to avoid calling through to Flash too
+ * often.
+ */
+
+/**
+ * Returns a new getter function that returns a cached value if
+ * invoked multiple times within the specified duration.
+ *
+ * @param {Function} getter the function to be cached
+ * @param {Number} cacheDuration the number of milliseconds to cache
+ * results for
+ * @return {Function} a new function that returns cached results if
+ * invoked multiple times within the cache duration
+ */
+export default function timeExpiringCache(getter, cacheDuration) {
+  const result = function cachedGetter() {
+    const now = new Date().getTime();
+
+    if (now - result.lastCheckTime_ >= cacheDuration) {
+      result.lastCheckTime_ = now;
+      result.cache_ = getter();
+    }
+    return result.cache_;
+  };
+
+  result.lastCheckTime_ = -Infinity;
+  return result;
+}

src/js/tech/flash.js

@@ -10,6 +10,7 @@ import * as Dom from '../utils/dom.js';
 import * as Url from '../utils/url.js';
 import { createTimeRange } from '../utils/time-ranges.js';
 import FlashRtmpDecorator from './flash-rtmp';
+import timeExpiringCache from './flash-cache.js';
 import Component from '../component';
 import window from 'global/window';
 import assign from 'object.assign';
@@ -59,6 +60,34 @@ class Flash extends Tech {
     this.on('seeked', function() {
       this.lastSeekTarget_ = undefined;
     });
+
+    // calling into the SWF can be expensive, especially if Flash is
+    // busy rendering video frames
+    // automatically cache commonly used properties for a short period
+    // of time so that multiple calls within a short time period don't
+    // all pay a big performance penalty for properties that change
+    // relatively slowly over time
+    const getCurrentTimeCached = timeExpiringCache(() => {
+      return this.el_.vjs_getProperty('currentTime');
+    }, 100);
+
+    this.currentTime = (time) => {
+      // when seeking make the reported time keep up with the requested time
+      // by reading the time we're seeking to
+      if (this.seeking()) {
+        return this.lastSeekTarget_ || 0;
+      }
+
+      return getCurrentTimeCached();
+    };
+    this.buffered = timeExpiringCache(() => {
+      const ranges = this.el_.vjs_getProperty('buffered');
+
+      if (ranges.length === 0) {
+        return createTimeRange();
+      }
+      return createTimeRange(ranges[0][0], ranges[0][1]);
+    }, 100);
   }
 
   /**
@@ -213,14 +242,6 @@ class Flash extends Tech {
    * @return {Number} Current time
    * @method currentTime
    */
-  currentTime(time) {
-    // when seeking make the reported time keep up with the requested time
-    // by reading the time we're seeking to
-    if (this.seeking()) {
-      return this.lastSeekTarget_ || 0;
-    }
-    return this.el_.vjs_getProperty('currentTime');
-  }
 
   /**
    * Get current source
@@ -294,14 +315,6 @@ class Flash extends Tech {
    * @return {TimeRangeObject}
    * @method buffered
    */
-  buffered() {
-    const ranges = this.el_.vjs_getProperty('buffered');
-
-    if (ranges.length === 0) {
-      return createTimeRange();
-    }
-    return createTimeRange(ranges[0][0], ranges[0][1]);
-  }
 
   /**
    * Get fullscreen support -

test/unit/tech/flash-cache.test.js

@@ -0,0 +1,50 @@
+/* eslint-env qunit */
+import timeExpiringCache from '../../../src/js/tech/flash-cache.js';
+import sinon from 'sinon';
+
+QUnit.module('Flash Cache', {
+  beforeEach() {
+    this.clock = sinon.useFakeTimers();
+  },
+  afterEach() {
+    this.clock.restore();
+  }
+});
+
+QUnit.test('calls the getter on first invocation', function(assert) {
+  let calls = 0;
+  const cached = timeExpiringCache(() => calls++, 100);
+
+  QUnit.equal(cached(), 0, 'returned a value');
+  QUnit.equal(calls, 1, 'called the getter');
+});
+
+QUnit.test('caches results', function(assert) {
+  let calls = 0;
+  const cached = timeExpiringCache(() => calls++, 100);
+
+  for (let i = 0; i < 31; i++) {
+    QUnit.equal(cached(), 0, 'returned a cached value');
+  }
+  QUnit.equal(calls, 1, 'only called getter once');
+});
+
+QUnit.test('refreshes the cache after the result expires', function(assert) {
+  let calls = 0;
+  const cached = timeExpiringCache(() => calls++, 1);
+
+  QUnit.equal(cached(), 0, 'returned a value');
+  QUnit.equal(cached(), 0, 'returned a cached value');
+  QUnit.equal(calls, 1, 'only called getter once');
+  this.clock.tick(1);
+
+  QUnit.equal(cached(), 1, 'returned a value');
+  QUnit.equal(cached(), 1, 'returned a cached value');
+  QUnit.equal(calls, 2, 'called getter again');
+
+  this.clock.tick(10);
+  QUnit.equal(calls, 2, 'only refreshes on-demand');
+  QUnit.equal(cached(), 2, 'returned a value');
+  QUnit.equal(cached(), 2, 'returned a cached value');
+  QUnit.equal(calls, 3, 'called getter again');
+});

test/unit/tech/flash.test.js

@@ -31,53 +31,50 @@ QUnit.test('Flash.canPlaySource', function(assert) {
 });
 
 QUnit.test('currentTime', function(assert) {
-  const getCurrentTime = Flash.prototype.currentTime;
-  const setCurrentTime = Flash.prototype.setCurrentTime;
+  const mockFlash = new MockFlash();
   let seekingCount = 0;
   let seeking = false;
   let setPropVal;
   let getPropVal;
   let result;
 
   // Mock out a Flash instance to avoid creating the swf object
-  const mockFlash = {
-    el_: {
-      /* eslint-disable camelcase */
-      vjs_setProperty(prop, val) {
-        setPropVal = val;
-      },
-      vjs_getProperty() {
-        return getPropVal;
-      }
-      /* eslint-enable camelcase */
+  mockFlash.el_ = {
+    /* eslint-disable camelcase */
+    vjs_setProperty(prop, val) {
+      setPropVal = val;
     },
-    seekable() {
-      return createTimeRange(5, 1000);
-    },
-    trigger(event) {
-      if (event === 'seeking') {
-        seekingCount++;
-      }
-    },
-    seeking() {
-      return seeking;
+    vjs_getProperty() {
+      return getPropVal;
     }
+    /* eslint-enable camelcase */
+  };
+  mockFlash.seekable = function() {
+    return createTimeRange(5, 1000);
+  };
+  mockFlash.trigger = function(event) {
+    if (event === 'seeking') {
+      seekingCount++;
+    }
+  };
+  mockFlash.seeking = function() {
+    return seeking;
   };
 
   // Test the currentTime getter
   getPropVal = 3;
-  result = getCurrentTime.call(mockFlash);
+  result = mockFlash.currentTime();
   assert.equal(result, 3, 'currentTime is retreived from the swf element');
 
   // Test the currentTime setter
-  setCurrentTime.call(mockFlash, 10);
+  mockFlash.setCurrentTime(10);
   assert.equal(setPropVal, 10, 'currentTime is set on the swf element');
   assert.equal(seekingCount, 1, 'triggered seeking');
 
   // Test current time while seeking
-  setCurrentTime.call(mockFlash, 20);
+  mockFlash.setCurrentTime(20);
   seeking = true;
-  result = getCurrentTime.call(mockFlash);
+  result = mockFlash.currentTime();
   assert.equal(result,
               20,
               'currentTime is retrieved from the lastSeekTarget while seeking');
@@ -87,13 +84,13 @@ QUnit.test('currentTime', function(assert) {
   assert.equal(seekingCount, 2, 'triggered seeking');
 
   // clamp seeks to seekable
-  setCurrentTime.call(mockFlash, 1001);
-  result = getCurrentTime.call(mockFlash);
+  mockFlash.setCurrentTime(1001);
+  result = mockFlash.currentTime();
   assert.equal(result, mockFlash.seekable().end(0), 'clamped to the seekable end');
   assert.equal(seekingCount, 3, 'triggered seeking');
 
-  setCurrentTime.call(mockFlash, 1);
-  result = getCurrentTime.call(mockFlash);
+  mockFlash.setCurrentTime(1);
+  result = mockFlash.currentTime();
   assert.equal(result, mockFlash.seekable().start(0), 'clamped to the seekable start');
   assert.equal(seekingCount, 4, 'triggered seeking');
 });