refactor(scheduler): use bitwise flags for scheduler jobs + optimize …

…queueJob
vue-mini · Sep 8, 2024 · ad6b79a · ad6b79a
1 parent 5680df1
commit ad6b79a
Show file tree

Hide file tree

Showing 3 changed files with 63 additions and 93 deletions.
diff --git a/packages/core/__tests__/scheduler.spec.ts b/packages/core/__tests__/scheduler.spec.ts
@@ -1,4 +1,7 @@
+/* eslint-disable no-bitwise */
+import type { SchedulerJob } from '../src/scheduler'
 import {
+  SchedulerJobFlags,
   flushPostFlushCbs,
   nextTick,
   queueJob,
@@ -84,25 +87,6 @@ describe('scheduler', () => {
     })
   })
 
-  describe('pre flush jobs', () => {
-    it('queueJob inside preFlushCb', async () => {
-      const calls: string[] = []
-      const job1 = () => {
-        calls.push('job1')
-      }
-
-      const cb1 = () => {
-        // QueueJob in postFlushCb
-        calls.push('cb1')
-        queueJob(job1)
-      }
-
-      queueJob(cb1)
-      await nextTick()
-      expect(calls).toEqual(['cb1', 'job1'])
-    })
-  })
-
   describe('queuePostFlushCb', () => {
     it('basic usage', async () => {
       const calls: string[] = []
@@ -352,27 +336,27 @@ describe('scheduler', () => {
   test('should allow explicitly marked jobs to trigger itself', async () => {
     // Normal job
     let count = 0
-    const job = () => {
+    const job: SchedulerJob = () => {
       if (count < 3) {
         count++
         queueJob(job)
       }
     }
 
-    job.allowRecurse = true
+    job.flags! |= SchedulerJobFlags.ALLOW_RECURSE
     queueJob(job)
     await nextTick()
     expect(count).toBe(3)
 
     // Post cb
-    const cb = () => {
+    const cb: SchedulerJob = () => {
       if (count < 5) {
         count++
         queuePostFlushCb(cb)
       }
     }
 
-    cb.allowRecurse = true
+    cb.flags! |= SchedulerJobFlags.ALLOW_RECURSE
     queuePostFlushCb(cb)
     flushPostFlushCbs()
     expect(count).toBe(4)
@@ -382,29 +366,6 @@ describe('scheduler', () => {
     expect(count).toBe(5)
   })
 
-  // #910
-  test('should not run stopped reactive effects', async () => {
-    const spy = vi.fn()
-
-    // Simulate parent component that toggles child
-    const job1 = () => {
-      // @ts-expect-error
-      job2.active = false
-    }
-
-    // Simulate child that's triggered by the same reactive change that
-    // triggers its toggle
-    const job2 = () => spy()
-    expect(spy).toHaveBeenCalledTimes(0)
-
-    queueJob(job1)
-    queueJob(job2)
-    await nextTick()
-
-    // Should not be called
-    expect(spy).toHaveBeenCalledTimes(0)
-  })
-
   it('nextTick should return promise', async () => {
     const fn = vi.fn(() => 1)
 
@@ -415,23 +376,22 @@ describe('scheduler', () => {
     expect(fn).toHaveBeenCalledTimes(1)
   })
 
-  /** Dividing line, the above tests is directly copy from vue.js **/
+  /** Dividing line, the above tests is directly copy from vue.js with some changes **/
 
-  test('should not run inactive callback', async () => {
-    const spy = vi.fn()
+  test('queueJob inside job', async () => {
+    const calls: string[] = []
 
     const job1 = () => {
-      // @ts-expect-error
-      job2.active = false
+      calls.push('job1')
     }
 
-    const job2 = () => spy()
-    expect(spy).toHaveBeenCalledTimes(0)
-
-    queuePostFlushCb(job1)
-    queuePostFlushCb(job2)
-    flushPostFlushCbs()
+    const cb1 = () => {
+      calls.push('cb1')
+      queueJob(job1)
+    }
 
-    expect(spy).toHaveBeenCalledTimes(0)
+    queueJob(cb1)
+    await nextTick()
+    expect(calls).toEqual(['cb1', 'job1'])
   })
 })
diff --git a/packages/core/src/scheduler.ts b/packages/core/src/scheduler.ts
@@ -1,8 +1,17 @@
+/* eslint-disable no-bitwise, unicorn/prefer-math-trunc, @typescript-eslint/prefer-literal-enum-member */
 import { NOOP } from './utils'
 
+export enum SchedulerJobFlags {
+  QUEUED = 1 << 0,
+  ALLOW_RECURSE = 1 << 2,
+}
+
 export interface SchedulerJob extends Function {
-  active?: boolean
-  allowRecurse?: boolean
+  /**
+   * Flags can technically be undefined, but it can still be used in bitwise
+   * operations just like 0.
+   */
+  flags?: SchedulerJobFlags
 }
 
 let isFlushing = false
@@ -16,7 +25,7 @@ let activePostFlushCbs: SchedulerJob[] | null = null
 let postFlushIndex = 0
 
 // eslint-disable-next-line spaced-comment
-const resolvedPromise = /*#__PURE__*/ Promise.resolve() as Promise<any>
+const resolvedPromise = /*@__PURE__*/ Promise.resolve() as Promise<any>
 let currentFlushPromise: Promise<void> | null = null
 
 const RECURSION_LIMIT = 100
@@ -28,21 +37,10 @@ export function nextTick<R = void>(fn?: () => R): Promise<Awaited<R>> {
   return fn ? p.then(fn) : p
 }
 
-export function queueJob(job: SchedulerJob) {
-  // The dedupe search uses the startIndex argument of Array.includes()
-  // by default the search index includes the current job that is being run
-  // so it cannot recursively trigger itself again.
-  // if the job is a watch() callback, the search will start with a +1 index to
-  // allow it recursively trigger itself - it is the user's responsibility to
-  // ensure it doesn't end up in an infinite loop.
-  if (
-    queue.length === 0 ||
-    !queue.includes(
-      job,
-      isFlushing && job.allowRecurse ? flushIndex + 1 : flushIndex,
-    )
-  ) {
+export function queueJob(job: SchedulerJob): void {
+  if (!(job.flags! & SchedulerJobFlags.QUEUED)) {
     queue.push(job)
+    job.flags! |= SchedulerJobFlags.QUEUED
     queueFlush()
   }
 }
@@ -55,19 +53,14 @@ function queueFlush(): void {
   }
 }
 
-export function queuePostFlushCb(cb: SchedulerJob) {
-  if (
-    !activePostFlushCbs ||
-    !activePostFlushCbs.includes(
-      cb,
-      cb.allowRecurse ? postFlushIndex + 1 : postFlushIndex,
-    )
-  ) {
+export function queuePostFlushCb(cb: SchedulerJob): void {
+  if (!(cb.flags! & SchedulerJobFlags.QUEUED)) {
     pendingPostFlushCbs.push(cb)
+    cb.flags! |= SchedulerJobFlags.QUEUED
   }
 }
 
-export function flushPostFlushCbs() {
+export function flushPostFlushCbs(): void {
   if (pendingPostFlushCbs.length > 0) {
     activePostFlushCbs = [...new Set(pendingPostFlushCbs)]
     pendingPostFlushCbs.length = 0
@@ -78,7 +71,12 @@ export function flushPostFlushCbs() {
       postFlushIndex++
     ) {
       const cb = activePostFlushCbs[postFlushIndex]
-      if (cb.active !== false) cb()
+      if (cb.flags! & SchedulerJobFlags.ALLOW_RECURSE) {
+        cb.flags! &= ~SchedulerJobFlags.QUEUED
+      }
+
+      cb()
+      cb.flags! &= ~SchedulerJobFlags.QUEUED
     }
 
     activePostFlushCbs = null
@@ -107,16 +105,25 @@ function flushJobs(seen?: CountMap): void {
   try {
     for (flushIndex = 0; flushIndex < queue.length; flushIndex++) {
       const job = queue[flushIndex]
-      if (job.active !== false) {
-        /* istanbul ignore if -- @preserve  */
-        if (__DEV__ && check(job)) {
-          continue
-        }
+      /* istanbul ignore if -- @preserve  */
+      if (__DEV__ && check(job)) {
+        continue
+      }
 
-        job()
+      if (job.flags! & SchedulerJobFlags.ALLOW_RECURSE) {
+        job.flags! &= ~SchedulerJobFlags.QUEUED
       }
+
+      job()
+      job.flags! &= ~SchedulerJobFlags.QUEUED
     }
   } finally {
+    // If there was an error we still need to clear the QUEUED flags
+    for (; flushIndex < queue.length; flushIndex++) {
+      const job = queue[flushIndex]
+      job.flags! &= ~SchedulerJobFlags.QUEUED
+    }
+
     flushIndex = 0
     queue.length = 0
 

diff --git a/packages/core/src/watch.ts b/packages/core/src/watch.ts
@@ -14,7 +14,7 @@ import {
   getCurrentScope,
 } from '@vue/reactivity'
 import type { SchedulerJob } from './scheduler'
-import { queueJob, queuePostFlushCb } from './scheduler'
+import { SchedulerJobFlags, queueJob, queuePostFlushCb } from './scheduler'
 import {
   NOOP,
   extend,
@@ -328,7 +328,10 @@ function doWatch(
 
   // Important: mark the job as a watcher callback so that scheduler knows
   // it is allowed to self-trigger
-  job.allowRecurse = Boolean(cb)
+  if (cb) {
+    // eslint-disable-next-line no-bitwise
+    job.flags! |= SchedulerJobFlags.ALLOW_RECURSE
+  }
 
   let scheduler: EffectScheduler
   if (flush === 'sync') {