From ae3bd2bda2f918bbdbf8959c7d76f057a8d1b866 Mon Sep 17 00:00:00 2001
From: Nolan Lawson <nolan@nolanlawson.com>
Date: Sun, 26 Apr 2020 16:54:00 -0700
Subject: [PATCH] perf: make timeline rendering less janky (#1747)

* perf: make timeline rendering less janky

1. Ensures all statuses are rendered from top to bottom (no more shuffling-card-effect rendering)
2. Wraps all individual status renders in their own requestIdleCallback to improve input responsiveness especially only slow devices like Nexus 5.

* fix focus restoration

* only do rIC on mobile
---
 package.json                                  |  1 +
 src/routes/_actions/createMakeProps.js        | 39 +++++++------------
 src/routes/_actions/showMoreAndScrollToTop.js | 31 +++------------
 src/routes/_components/FocusRestoration.html  | 13 +------
 .../virtualList/VirtualListLazyItem.html      | 35 +++++++++++++++--
 src/routes/_utils/createPriorityQueue.js      | 24 ++++++++++++
 src/routes/_utils/queueMicrotask.js           | 12 ++++++
 src/routes/_utils/tryToFocusElement.js        | 25 ++++++++++++
 8 files changed, 116 insertions(+), 64 deletions(-)
 create mode 100644 src/routes/_utils/createPriorityQueue.js
 create mode 100644 src/routes/_utils/queueMicrotask.js
 create mode 100644 src/routes/_utils/tryToFocusElement.js

diff --git a/package.json b/package.json
index 54aa0821..f74bd642 100644
--- a/package.json
+++ b/package.json
@@ -163,6 +163,7 @@
       "matchMedia",
       "performance",
       "postMessage",
+      "queueMicrotask",
       "requestAnimationFrame",
       "requestIdleCallback",
       "self",
diff --git a/src/routes/_actions/createMakeProps.js b/src/routes/_actions/createMakeProps.js
index 4ce7d17d..9c71d727 100644
--- a/src/routes/_actions/createMakeProps.js
+++ b/src/routes/_actions/createMakeProps.js
@@ -43,29 +43,13 @@ async function decodeAllBlurhashes (statusOrNotification) {
     }))
     stop(`decodeBlurhash-${status.id}`)
   }
-  return statusOrNotification
 }
 
 export function createMakeProps (instanceName, timelineType, timelineValue) {
-  let taskCount = 0
-  let pending = []
+  let promiseChain = Promise.resolve()
 
   tryInitBlurhash() // start the blurhash worker a bit early to save time
 
-  // The worker-powered indexeddb promises can resolve in arbitrary order,
-  // causing the timeline to load in a jerky way. With this function, we
-  // wait for all promises to resolve before resolving them all in one go.
-  function awaitAllTasksComplete () {
-    return new Promise(resolve => {
-      taskCount--
-      pending.push(resolve)
-      if (taskCount === 0) {
-        pending.forEach(_ => _())
-        pending = []
-      }
-    })
-  }
-
   async function fetchFromIndexedDB (itemId) {
     mark(`fetchFromIndexedDB-${itemId}`)
     try {
@@ -78,13 +62,20 @@ export function createMakeProps (instanceName, timelineType, timelineValue) {
     }
   }
 
-  return (itemId) => {
-    taskCount++
+  async function getStatusOrNotification (itemId) {
+    const statusOrNotification = await fetchFromIndexedDB(itemId)
+    await decodeAllBlurhashes(statusOrNotification)
+    return statusOrNotification
+  }
 
-    return fetchFromIndexedDB(itemId)
-      .then(decodeAllBlurhashes)
-      .then(statusOrNotification => {
-        return awaitAllTasksComplete().then(() => statusOrNotification)
-      })
+  // The results from IndexedDB or the worker thread can return in random order,
+  // so we ensure consistent ordering based on the order this function is called in.
+  return itemId => {
+    const getStatusOrNotificationPromise = getStatusOrNotification(itemId) // start the promise ASAP
+    return new Promise((resolve, reject) => {
+      promiseChain = promiseChain
+        .then(() => getStatusOrNotificationPromise)
+        .then(resolve, reject)
+    })
   }
 }
diff --git a/src/routes/_actions/showMoreAndScrollToTop.js b/src/routes/_actions/showMoreAndScrollToTop.js
index 188fea86..d1450cd1 100644
--- a/src/routes/_actions/showMoreAndScrollToTop.js
+++ b/src/routes/_actions/showMoreAndScrollToTop.js
@@ -1,11 +1,8 @@
 import { showMoreItemsForCurrentTimeline } from './timeline'
 import { scrollToTop } from '../_utils/scrollToTop'
-import { scheduleIdleTask } from '../_utils/scheduleIdleTask'
 import { createStatusOrNotificationUuid } from '../_utils/createStatusOrNotificationUuid'
 import { store } from '../_store/store'
-
-const RETRIES = 5
-const TIMEOUT = 50
+import { tryToFocusElement } from '../_utils/tryToFocusElement'
 
 export function showMoreAndScrollToTop () {
   // Similar to Twitter, pressing "." will click the "show more" button and select
@@ -24,25 +21,9 @@ export function showMoreAndScrollToTop () {
   const notificationId = currentTimelineType === 'notifications' && firstItemSummary.id
   const statusId = currentTimelineType !== 'notifications' && firstItemSummary.id
   scrollToTop(/* smooth */ false)
-  // try 5 times to wait for the element to be rendered and then focus it
-  let count = 0
-  const tryToFocusElement = () => {
-    const uuid = createStatusOrNotificationUuid(
-      currentInstance, currentTimelineType,
-      currentTimelineValue, notificationId, statusId
-    )
-    const element = document.getElementById(uuid)
-    if (element) {
-      try {
-        element.focus({ preventScroll: true })
-      } catch (e) {
-        console.error(e)
-      }
-    } else {
-      if (++count <= RETRIES) {
-        setTimeout(() => scheduleIdleTask(tryToFocusElement), TIMEOUT)
-      }
-    }
-  }
-  scheduleIdleTask(tryToFocusElement)
+  const id = createStatusOrNotificationUuid(
+    currentInstance, currentTimelineType,
+    currentTimelineValue, notificationId, statusId
+  )
+  tryToFocusElement(id)
 }
diff --git a/src/routes/_components/FocusRestoration.html b/src/routes/_components/FocusRestoration.html
index 05420a21..b775e8fd 100644
--- a/src/routes/_components/FocusRestoration.html
+++ b/src/routes/_components/FocusRestoration.html
@@ -7,7 +7,7 @@
 <script>
   import { PAGE_HISTORY_SIZE } from '../_static/pages'
   import { QuickLRU } from '../_thirdparty/quick-lru/quick-lru'
-  import { doubleRAF } from '../_utils/doubleRAF'
+  import { tryToFocusElement } from '../_utils/tryToFocusElement'
 
   const cache = new QuickLRU({ maxSize: PAGE_HISTORY_SIZE })
 
@@ -64,16 +64,7 @@
           return
         }
         console.log('restoreFocus', realm, elementId)
-        doubleRAF(() => {
-          const element = document.getElementById(elementId)
-          if (element) {
-            try {
-              element.focus({ preventScroll: true })
-            } catch (err) {
-              console.warn('failed to focus', elementId, err)
-            }
-          }
-        })
+        tryToFocusElement(elementId)
       },
       clearFocus () {
         const { realm } = this.get()
diff --git a/src/routes/_components/virtualList/VirtualListLazyItem.html b/src/routes/_components/virtualList/VirtualListLazyItem.html
index ae522910..9d53d390 100644
--- a/src/routes/_components/virtualList/VirtualListLazyItem.html
+++ b/src/routes/_components/virtualList/VirtualListLazyItem.html
@@ -9,15 +9,42 @@
 <script>
   import VirtualListItem from './VirtualListItem'
   import { mark, stop } from '../../_utils/marks'
+  import { scheduleIdleTask } from '../../_utils/scheduleIdleTask'
+  import { createPriorityQueue } from '../../_utils/createPriorityQueue'
+  import { isMobile } from '../../_utils/userAgent/isMobile'
+
+  // In Svelte's implementation of lists, these VirtualListLazyItems can be
+  // created in any order. By default in fact it seems to do it in reverse
+  // order, which we don't really want, because then items render in a janky
+  // way, with the last ones first and then replaced by the first ones,
+  // resulting in a UI that looks like a deck of cards being shuffled.
+  // This functions waits a microtask and then ensures that the callbacks
+  // are called by index, in ascending order.
+  const priorityQueue = createPriorityQueue()
 
   export default {
     async oncreate () {
-      const { makeProps, key } = this.get()
+      const { makeProps, key, index } = this.get()
       if (makeProps) {
+        await priorityQueue(index)
         const props = await makeProps(key)
-        mark('VirtualListLazyItem set props')
-        this.set({ props: props })
-        stop('VirtualListLazyItem set props')
+        const setProps = () => {
+          mark('VirtualListLazyItem set props')
+          this.set({ props: props })
+          stop('VirtualListLazyItem set props')
+        }
+        // On mobile, render in rIC for maximum input responsiveness. The reason we do
+        // different behavior for desktop versus mobile is:
+        // 1. On desktop, the scrollbar is really distracting as it changes size. It may
+        //    even cause issues for people with vestibular disorders (see also prefers-reduced-motion).
+        // 2. On mobile, the CPU is generally slower, so we want better input responsiveness.
+        // TODO: someday we can use isInputPending as a better way to break up work
+        // https://www.chromestatus.com/feature/5719830432841728
+        if (isMobile()) {
+          scheduleIdleTask(setProps)
+        } else {
+          setProps()
+        }
       }
     },
     data: () => ({
diff --git a/src/routes/_utils/createPriorityQueue.js b/src/routes/_utils/createPriorityQueue.js
new file mode 100644
index 00000000..4e0eb61f
--- /dev/null
+++ b/src/routes/_utils/createPriorityQueue.js
@@ -0,0 +1,24 @@
+// Promise-based implementation that waits a microtask tick
+// and executes the resolve() functions in priority order
+import { queueMicrotask } from './queueMicrotask'
+
+export function createPriorityQueue () {
+  const tasks = []
+
+  function flush () {
+    if (tasks.length) {
+      const sortedTasks = tasks.sort((a, b) => a.priority < b.priority ? -1 : 1)
+      for (const task of sortedTasks) {
+        task.resolve()
+      }
+      tasks.length = 0
+    }
+  }
+
+  return function next (priority) {
+    return new Promise(resolve => {
+      tasks.push({ priority, resolve })
+      queueMicrotask(flush)
+    })
+  }
+}
diff --git a/src/routes/_utils/queueMicrotask.js b/src/routes/_utils/queueMicrotask.js
new file mode 100644
index 00000000..0a57b443
--- /dev/null
+++ b/src/routes/_utils/queueMicrotask.js
@@ -0,0 +1,12 @@
+// via https://developer.mozilla.org/en-US/docs/Web/API/WindowOrWorkerGlobalScope/queueMicrotask
+function queueMicrotaskPolyfill (callback) {
+  Promise.resolve()
+    .then(callback)
+    .catch(e => setTimeout(() => { throw e }))
+}
+
+const qM = typeof queueMicrotask === 'function' ? queueMicrotask : queueMicrotaskPolyfill
+
+export {
+  qM as queueMicrotask
+}
diff --git a/src/routes/_utils/tryToFocusElement.js b/src/routes/_utils/tryToFocusElement.js
new file mode 100644
index 00000000..2530ad05
--- /dev/null
+++ b/src/routes/_utils/tryToFocusElement.js
@@ -0,0 +1,25 @@
+// try 5 times to wait for the element to be rendered and then focus it
+import { scheduleIdleTask } from './scheduleIdleTask'
+
+const RETRIES = 5
+const TIMEOUT = 50
+
+export async function tryToFocusElement (id) {
+  for (let i = 0; i < RETRIES; i++) {
+    if (i > 0) {
+      await new Promise(resolve => setTimeout(resolve, TIMEOUT))
+    }
+    await new Promise(resolve => scheduleIdleTask(resolve))
+    const element = document.getElementById(id)
+    if (element) {
+      try {
+        element.focus({ preventScroll: true })
+        console.log('focused element', id)
+        return
+      } catch (e) {
+        console.error(e)
+      }
+    }
+  }
+  console.log('failed to focus element', id)
+}