perf: Throttle ProgressControl ui updates, add document.hidden check to time-displays #6150

brandonocasey · 2019-07-31T20:16:58Z

So the activeElement mixin does the following:

Manages starting and stoping an update function when:
a. The player is/is not paused
b. The component gains or loses Keyboard/mouse focus or player.userActive changes
c. The tabs visibility (wether it is in the background or not) changes.
Does not start anything until the player has started playback.

src/js/mixins/visiblity.js

src/js/control-bar/time-controls/time-display.js

brandonocasey · 2019-08-01T14:59:13Z

src/js/control-bar/progress-control/seek-bar.js

+      );
+
+      // Update the `PlayProgressBar` when we are in mouse focus
+      if (this.bar && (!this.parentComponent_ || this.parentComponent_.mouseFocus_)) {


This is only shown when the mouse is over progressControl, so only update then.

brandonocasey · 2019-08-01T14:59:30Z

src/js/control-bar/progress-control/progress-control.js

@@ -4,6 +4,8 @@
 import Component from '../../component.js';


We need to do an update in progressControl rather then seekBar because progressControl is the component that is tabbed to and moused over.

1. Only update SeekBar's PlayProgressBar on mouseover 2. Only update SeekBar when seekbar is visible or in mouse/keyboard focus

brandonocasey · 2019-08-01T20:18:32Z

src/js/control-bar/progress-control/load-progress-bar.js

+import activeElement from '../../mixins/active-element.js';
+
+// get the percent width of a time compared to the total end
+const percentify = function(time, end) {


There isn't a good reason for this to define this every time the update function is run.

Also css only takes into account the first two decimal places so we should just always round.

brandonocasey · 2019-08-01T20:19:09Z

src/js/control-bar/progress-control/load-progress-bar.js

-
-      if (rounded) {
-        percent = percent.toFixed(2);
+    this.requestAnimationFrame(() => {


Without requestAnimationFrame this ran on the main thread, and it took a good chunk of time too.

hiding whitespace changes makes this a lot easier to diff.

brandonocasey · 2019-08-01T20:22:50Z

src/js/control-bar/progress-control/load-progress-bar.js

+      const controlTextPercentage = this.$('.vjs-control-text-loaded-percentage');
+      const percent = percentify(bufferedEnd, duration);
+
+      if (this.el_.style.width !== percent) {


Only update the style if it changed

brandonocasey · 2019-08-01T20:25:54Z

src/js/control-bar/progress-control/load-progress-bar.js

-        part = this.el_.appendChild(Dom.createEl());
-        children[i] = part;
-      }
+        if (part.style.left !== leftPercent) {


only update when changed.

brandonocasey · 2019-08-01T20:26:51Z

src/js/control-bar/progress-control/seek-bar.js

@@ -169,12 +109,6 @@ class SeekBar extends Slider {
   *          The current percent at a number from 0-1
   */
  update(event) {
-    // if the offsetParent is null, then this element is hidden, in which case
-    // we don't need to update it.
-    if (this.el().offsetParent === null) {


This is not needed as this component is always visible for screen readers.

brandonocasey · 2019-08-01T20:27:14Z

src/js/control-bar/time-controls/time-display.js

+      }
+
+      if (oldNode) {
+        this.contentEl_.replaceChild(this.textNode_, oldNode);


replaceChild is about 10% faster than appendChild.

brandonocasey · 2019-08-02T19:31:06Z

This just became too complicated, I broke out most of the logic into #6155. I will close this one and open another with the changes here.

brandonocasey changed the title ~~perf: Add a mixin that throttles ui updates~~ perf: Throttle ProgressControl ui updates, add document.hidden check to time-displays Jul 31, 2019

brandonocasey force-pushed the perf/active-update branch from 198dc31 to 84657d1 Compare July 31, 2019 23:12