refactor(reactivity): fix cleanup performance regression

Author: johnsoncodehk

packages/reactivity/__tests__/effectScope.spec.ts

@@ -361,7 +361,7 @@ describe('reactivity/effect/scope', () => {
     expect(cleanupCalls).toBe(1)
 
     expect(getEffectsCount(scope)).toBe(0)
-    expect(scope.cleanups).toBe(0)
+    expect(scope.cleanupsLength).toBe(0)
   })
 })
 

packages/reactivity/src/effect.ts

@@ -61,12 +61,20 @@ export class ReactiveEffect<T = any>
   deps: Link | undefined = undefined
   depsTail: Link | undefined = undefined
   flags: number = SubscriberFlags.Dirty
-  cleanups: number = 0
 
   // Dependency
   subs: Link | undefined = undefined
   subsTail: Link | undefined = undefined
 
+  /**
+   * @internal
+   */
+  cleanups: (() => void)[] = []
+  /**
+   * @internal
+   */
+  cleanupsLength = 0
+
   // dev only
   onTrack?: (event: DebuggerEvent) => void
   // dev only
@@ -85,7 +93,7 @@ export class ReactiveEffect<T = any>
   }
 
   get active(): boolean {
-    return this.deps !== undefined
+    return !!this.flags || this.deps !== undefined
   }
 
   pause(): void {
@@ -121,10 +129,7 @@ export class ReactiveEffect<T = any>
   }
 
   run(): T {
-    const cleanups = this.cleanups
-    if (cleanups) {
-      cleanup(this, cleanups)
-    }
+    cleanup(this)
     const prevSub = activeSub
     setActiveSub(this)
     startTracking(this)
@@ -153,15 +158,13 @@ export class ReactiveEffect<T = any>
 
   stop(): void {
     const sub = this.subs
-    const cleanups = this.cleanups
     if (sub !== undefined) {
       unlink(sub)
     }
     startTracking(this)
     endTracking(this)
-    if (cleanups) {
-      cleanup(this, cleanups)
-    }
+    cleanup(this)
+    this.flags = 0
   }
 
   get dirty(): boolean {
@@ -280,30 +283,16 @@ export function resetTracking(): void {
   }
 }
 
-const cleanupCbs = new WeakMap()
-
-export function onCleanup(
-  sub: Subscriber & { cleanups: number },
-  cb: () => void,
-): void {
-  const cbs = cleanupCbs.get(sub)
-  if (cbs === undefined) {
-    cleanupCbs.set(sub, [cb])
-    sub.cleanups = 1
-  } else {
-    cbs[sub.cleanups!++] = cb
-  }
-}
-
 export function cleanup(
-  sub: Subscriber & { cleanups: number },
-  length: number,
+  sub: Subscriber & { cleanups: (() => void)[]; cleanupsLength: number },
 ): void {
-  const cbs = cleanupCbs.get(sub)!
-  for (let i = 0; i < length; ++i) {
-    cbs[i]()
+  const l = sub.cleanupsLength
+  if (l) {
+    for (let i = 0; i < l; i++) {
+      sub.cleanups[i]()
+    }
+    sub.cleanupsLength = 0
   }
-  sub.cleanups = 0
 }
 
 /**
@@ -319,9 +308,8 @@ export function cleanup(
  * an active effect.
  */
 export function onEffectCleanup(fn: () => void, failSilently = false): void {
-  const e = activeSub
-  if (e instanceof ReactiveEffect) {
-    onCleanup(e, () => cleanupEffect(fn))
+  if (activeSub instanceof ReactiveEffect) {
+    activeSub.cleanups[activeSub.cleanupsLength++] = () => cleanupEffect(fn)
   } else if (__DEV__ && !failSilently) {
     warn(
       `onEffectCleanup() was called when there was no active effect` +

packages/reactivity/src/effectScope.ts

@@ -1,4 +1,4 @@
-import { EffectFlags, cleanup, onCleanup } from './effect'
+import { EffectFlags, cleanup } from './effect'
 import {
   type Dependency,
   type Link,
@@ -17,20 +17,28 @@ export class EffectScope implements Subscriber, Dependency {
   deps: Link | undefined = undefined
   depsTail: Link | undefined = undefined
   flags: number = 0
-  cleanups: number = 0
 
   // Dependency
   subs: Link | undefined = undefined
   subsTail: Link | undefined = undefined
 
+  /**
+   * @internal
+   */
+  cleanups: (() => void)[] = []
+  /**
+   * @internal
+   */
+  cleanupsLength = 0
+
   constructor(detached = false) {
     if (!detached && activeEffectScope) {
       link(this, activeEffectScope)
     }
   }
 
   get active(): boolean {
-    return this.deps !== undefined
+    return !!this.flags || this.deps !== undefined
   }
 
   notify(): void {}
@@ -75,15 +83,13 @@ export class EffectScope implements Subscriber, Dependency {
 
   stop(): void {
     const sub = this.subs
-    const cleanups = this.cleanups
     if (sub !== undefined) {
       unlink(sub)
     }
     startTracking(this)
     endTracking(this)
-    if (cleanups) {
-      cleanup(this, cleanups)
-    }
+    cleanup(this)
+    this.flags = 0
   }
 }
 
@@ -126,7 +132,7 @@ export function setCurrentScope(
  */
 export function onScopeDispose(fn: () => void, failSilently = false): void {
   if (activeEffectScope !== undefined) {
-    onCleanup(activeEffectScope, fn)
+    activeEffectScope.cleanups[activeEffectScope.cleanupsLength++] = fn
   } else if (__DEV__ && !failSilently) {
     warn(
       `onScopeDispose() is called when there is no active effect scope` +

packages/reactivity/src/watch.ts

@@ -15,7 +15,6 @@ import {
   type DebuggerOptions,
   ReactiveEffect,
   cleanup,
-  onCleanup,
   pauseTracking,
   resetTracking,
 } from './effect'
@@ -98,9 +97,10 @@ export function onWatcherCleanup(
   if (owner) {
     const { call } = owner.options
     if (call) {
-      onCleanup(owner, () => call(cleanupFn, WatchErrorCodes.WATCH_CLEANUP))
+      owner.cleanups[owner.cleanupsLength++] = () =>
+        call(cleanupFn, WatchErrorCodes.WATCH_CLEANUP)
     } else {
-      onCleanup(owner, cleanupFn)
+      owner.cleanups[owner.cleanupsLength++] = cleanupFn
     }
   } else if (__DEV__ && !failSilently) {
     warn(
@@ -158,10 +158,10 @@ export class WatcherEffect extends ReactiveEffect {
       } else {
         // no cb -> simple effect
         getter = () => {
-          if (this.cleanups) {
+          if (this.cleanupsLength) {
             pauseTracking()
             try {
-              cleanup(this, this.cleanups)
+              cleanup(this)
             } finally {
               resetTracking()
             }
@@ -231,9 +231,7 @@ export class WatcherEffect extends ReactiveEffect {
         : hasChanged(newValue, this.oldValue))
     ) {
       // cleanup before running cb again
-      if (this.cleanups) {
-        cleanup(this, this.cleanups)
-      }
+      cleanup(this)
       const currentWatcher = activeWatcher
       activeWatcher = this
       try {