Defer idle requests made during a period unless the requesting callback used its slice

src/requestidlecallback.ts

@@ -3,6 +3,10 @@
 // low. Each callback runs in its own task, with at most 5ms of that budget, so input, rendering, React's
 // MessageChannel scheduler, timers and messages interleave with idle work instead of queueing behind one 10ms block,
 // and promise continuations settle between callbacks as they do with native requestIdleCallback.
+// Like native, requests made while a period is running wait for the next period. A cheap self-rescheduling poller
+// would otherwise run thousands of times a second in one period and hold a core. The exception is a request made
+// synchronously by a callback that used its whole slice: that is a work loop continuing, and may run later in the
+// same period.
 const periodMs = 10
 const sliceMs = 5
 // rAF can stop firing while the document still reports `visible` (for example in a display:none cross-origin
@@ -15,9 +19,11 @@ const hiddenGapMs = 100
 type Entry = {callback: IdleRequestCallback; timeout?: ReturnType<typeof setTimeout>}
 
 const pending = new Map<number, Entry>()
-// Requests made after the frame gate fired but before its period opens have not been through a paint yet, so they
-// wait for the next period.
-const unpainted = new Set<number>()
+// Requests that must wait for the next period: ones made after the frame gate fired but before its period opens
+// (not painted yet), and ones made while a period is running, unless the requesting callback used its whole slice.
+const deferred = new Set<number>()
+// Ids requested synchronously by the idle callback that is running right now, undefined outside of one.
+let requestedByCallback: Set<number> | undefined
 let nextId = 1
 let listening = false
 let running = false
@@ -29,32 +35,43 @@ let task: ReturnType<typeof setTimeout> | undefined
 
 function remove(id: number): void {
   pending.delete(id)
-  unpainted.delete(id)
+  deferred.delete(id)
 }
 
 function slice(first: boolean): void {
   const now = performance.now()
   const sliceEnd = Math.min(now + sliceMs, periodDeadline)
   try {
     if (first || now < periodDeadline) {
-      // Callbacks queued during the period (including self-rescheduling ones) may run in a later task of the same
-      // period, bounded by the wall deadline. The spec defers them to the next idle period, but that would give a
-      // self-rescheduling queue only one callback per frame.
+      // Callbacks queued before the period opened run in later tasks of it, bounded by the wall deadline. Requests
+      // made by a callback that returned with time left in its slice are deferred below, as the spec says. Ones from
+      // a callback that used its whole slice stay eligible, so a work loop that requeues itself keeps its throughput.
       for (const [id, entry] of pending) {
-        if (unpainted.has(id)) continue
+        if (deferred.has(id)) continue
         remove(id)
         clearTimeout(entry.timeout)
         const {callback} = entry
-        callback({didTimeout: false, timeRemaining: () => Math.max(0, sliceEnd - performance.now())})
+        const requested = new Set<number>()
+        requestedByCallback = requested
+        try {
+          callback({didTimeout: false, timeRemaining: () => Math.max(0, sliceEnd - performance.now())})
+        } finally {
+          requestedByCallback = undefined
+          if (performance.now() < sliceEnd) {
+            for (const requestedId of requested) {
+              if (pending.has(requestedId)) deferred.add(requestedId)
+            }
+          }
+        }
         break
       }
     }
   } finally {
-    if (pending.size > unpainted.size && performance.now() < periodDeadline) {
+    if (pending.size > deferred.size && performance.now() < periodDeadline) {
       yieldToEventLoop()
     } else {
       running = false
-      unpainted.clear()
+      deferred.clear()
       if (pending.size > 0) schedule()
     }
   }
@@ -81,7 +98,7 @@ function openPeriod(): void {
   running = true
   periodDeadline = performance.now() + periodMs
   // Nothing was painted while hidden, so there is no paint gate left to honor.
-  if (typeof document !== 'undefined' && document.visibilityState === 'hidden') unpainted.clear()
+  if (typeof document !== 'undefined' && document.visibilityState === 'hidden') deferred.clear()
   slice(true)
 }
 
@@ -129,7 +146,9 @@ export function requestIdleCallback(callback: IdleRequestCallback, options: Idle
   const id = nextId++
   const entry: Entry = {callback}
   pending.set(id, entry)
-  if (task !== undefined) unpainted.add(id)
+  if (task !== undefined) deferred.add(id)
+  else if (requestedByCallback) requestedByCallback.add(id)
+  else if (running) deferred.add(id)
   if (delay > 0) {
     entry.timeout = setTimeout(() => {
       if (pending.get(id) !== entry) return

test/requestidlecallback.js

@@ -376,22 +376,139 @@ describe('requestIdleCallback', () => {
       })
       runFrame()
       await sleep(100)
+      // The short outer callback's request waits for the next frame.
+      expect(log).to.eql(['outer', 'micro'])
+      runFrame()
+      await sleep(100)
       // The microtask runs when the outer callback's task ends; the inner callback must come after it.
       expect(log).to.eql(['outer', 'micro', 'inner'])
     })
 
-    it('may run a callback scheduled from a short callback later in the same period, without a new frame', async () => {
+    it('waits for the next frame to run a callback scheduled from a short callback', async () => {
       const ran = []
       requestIdleCallback(() => {
         ran.push('outer')
         requestIdleCallback(() => ran.push('inner'))
       })
       runFrame()
-      // No further runFrame(): the period still has budget, so the inner callback runs after a task tick.
+      // The period still has budget, but the outer callback finished early, so the spec's rule applies.
+      await sleep(100)
+      expect(ran).to.eql(['outer'])
+
+      runFrame()
       await sleep(100)
       expect(ran).to.eql(['outer', 'inner'])
     })
 
+    describe('a callback that uses its whole slice and requeues', () => {
+      // Whether the continuation fits in the 10ms period depends on how fast the machine runs the next task, and a
+      // continuation that misses the period waits for a frame that never comes here. So retry a few times: the
+      // behavior is proven by any one attempt where the continuation runs without a further frame.
+      const continuesInSamePeriod = async exhaust => {
+        for (let attempt = 0; attempt < 5; attempt += 1) {
+          let continued = false
+          requestIdleCallback(deadline => {
+            exhaust(deadline)
+            requestIdleCallback(() => {
+              continued = true
+            })
+          })
+          runFrame()
+          await sleep(50)
+          if (continued) return true
+          // Flush the missed attempt: its continuation runs in the next frame.
+          runFrame()
+          await sleep(50)
+        }
+        return false
+      }
+
+      it('may run the continuation later in the same period, when it looped on timeRemaining()', async () => {
+        const continued = await continuesInSamePeriod(deadline => {
+          while (deadline.timeRemaining() > 0) {
+            // spin
+          }
+        })
+        expect(continued).to.equal(true)
+      })
+
+      it('may run the continuation later in the same period, when it ran past its slice', async () => {
+        const continued = await continuesInSamePeriod(() => busyWait(6))
+        expect(continued).to.equal(true)
+      })
+    })
+
+    it('runs a cheap self-rescheduling callback about once per frame', async () => {
+      const frames = 5
+      let count = 0
+      let polling = true
+      const poll = () =>
+        requestIdleCallback(() => {
+          count += 1
+          if (polling) poll()
+        })
+      try {
+        poll()
+        for (let i = 0; i < frames; i += 1) {
+          runFrame()
+          await sleep(20)
+        }
+      } finally {
+        polling = false
+      }
+      expect(count).to.be.greaterThanOrEqual(1)
+      expect(count).to.be.lessThanOrEqual(frames + 1)
+    })
+
+    it('waits for the next frame when a request is made outside any callback while a period is running', async () => {
+      const ran = []
+      requestIdleCallback(() => {
+        ran.push('A')
+        // Runs after A's task ends and before the next slice, while the period is still open for B.
+        queueMicrotask(() => requestIdleCallback(() => ran.push('C')))
+      })
+      requestIdleCallback(() => ran.push('B'))
+      runFrame()
+      await sleep(100)
+      expect(ran).to.not.include('C')
+
+      for (let frames = 0; frames < 3 && ran.length < 3; frames += 1) {
+        runFrame()
+        await sleep(100)
+      }
+      expect(ran).to.eql(['A', 'B', 'C'])
+    })
+
+    it('still defers the request of a short callback that throws, and does not strand later callbacks', async () => {
+      const savedOnError = window.onerror
+      const errors = []
+      window.onerror = (message, source, line, column, error) => {
+        errors.push(error?.message ?? String(message))
+        return true
+      }
+      try {
+        const ran = []
+        requestIdleCallback(() => {
+          requestIdleCallback(() => ran.push('again'))
+          throw new Error('after requesting')
+        })
+        requestIdleCallback(() => ran.push('B'))
+        runFrame()
+        await sleep(100)
+        expect(errors).to.eql(['after requesting'])
+        expect(ran).to.not.include('again')
+
+        for (let frames = 0; frames < 3 && ran.length < 2; frames += 1) {
+          runFrame()
+          await sleep(100)
+        }
+        expect(ran).to.eql(['B', 'again'])
+        expect(errors).to.have.lengthOf(1)
+      } finally {
+        window.onerror = savedOnError
+      }
+    })
+
     it('runs a callback scheduled from inside a callback exactly once and after its siblings', async () => {
       const ran = []
       requestIdleCallback(() => {