From 91f29540feb515a3b772de83654d2e10f7ac953b Mon Sep 17 00:00:00 2001
From: Bas van Meurs <bvanmeurs1985@gmail.com>
Date: Thu, 29 Jul 2021 16:26:24 +0200
Subject: [PATCH] refactor(reactivity): optimize child effect scope
 dereferencing (#4184)

---
 .../reactivity/__tests__/effectScope.spec.ts  |  9 ++--
 packages/reactivity/src/effectScope.ts        | 42 ++++++++++++++-----
 2 files changed, 37 insertions(+), 14 deletions(-)

diff --git a/packages/reactivity/__tests__/effectScope.spec.ts b/packages/reactivity/__tests__/effectScope.spec.ts
index 55073470..e34723cc 100644
--- a/packages/reactivity/__tests__/effectScope.spec.ts
+++ b/packages/reactivity/__tests__/effectScope.spec.ts
@@ -77,8 +77,9 @@ describe('reactivity/effect/scope', () => {
       })
     })
 
-    expect(scope.effects.length).toBe(2)
-    expect(scope.effects[1]).toBeInstanceOf(EffectScope)
+    expect(scope.effects.length).toBe(1)
+    expect(scope.scopes.length).toBe(1)
+    expect(scope.scopes[0]).toBeInstanceOf(EffectScope)
 
     expect(dummy).toBe(0)
     counter.num = 7
@@ -194,9 +195,9 @@ describe('reactivity/effect/scope', () => {
   it('should derefence child scope from parent scope after stopping child scope (no memleaks)', async () => {
     const parent = new EffectScope()
     const child = parent.run(() => new EffectScope())!
-    expect(parent.effects.includes(child)).toBe(true)
+    expect(parent.scopes.includes(child)).toBe(true)
     child.stop()
-    expect(parent.effects.includes(child)).toBe(false)
+    expect(parent.scopes.includes(child)).toBe(false)
   })
 
   it('test with higher level APIs', async () => {
diff --git a/packages/reactivity/src/effectScope.ts b/packages/reactivity/src/effectScope.ts
index 0526f90d..e88abfc2 100644
--- a/packages/reactivity/src/effectScope.ts
+++ b/packages/reactivity/src/effectScope.ts
@@ -1,4 +1,3 @@
-import { remove } from '@vue/shared'
 import { ReactiveEffect } from './effect'
 import { warn } from './warning'
 
@@ -7,17 +6,23 @@ const effectScopeStack: EffectScope[] = []
 
 export class EffectScope {
   active = true
-  effects: (ReactiveEffect | EffectScope)[] = []
+  effects: ReactiveEffect[] = []
   cleanups: (() => void)[] = []
   parent: EffectScope | undefined
+  private children: EffectScope[] | undefined
+  private parentIndex: number | undefined
 
   constructor(detached = false) {
     if (!detached) {
-      recordEffectScope(this)
-      this.parent = activeEffectScope
+      this.recordEffectScope()
     }
   }
 
+  get scopes(): EffectScope[] {
+    if (!this.children) this.children = []
+    return this.children
+  }
+
   run<T>(fn: () => T): T | undefined {
     if (this.active) {
       try {
@@ -45,14 +50,31 @@ export class EffectScope {
     }
   }
 
-  stop(fromParent = false) {
+  stop() {
     if (this.active) {
-      this.effects.forEach(e => e.stop(true))
+      this.effects.forEach(e => e.stop())
+      this.children?.forEach(e => e.stop())
       this.cleanups.forEach(cleanup => cleanup())
+      this.parent?.derefChildScope(this)
       this.active = false
-      if (!fromParent && this.parent) {
-        remove(this.parent.effects, this)
-      }
+    }
+  }
+
+  private recordEffectScope() {
+    const parent = activeEffectScope
+    if (parent && parent.active) {
+      this.parent = parent
+      this.parentIndex = parent.scopes.push(this) - 1
+    }
+  }
+
+  private derefChildScope(scope: EffectScope) {
+    // reuse the freed index by moving the last array entry
+    const last = this.scopes.pop()
+    if (last && last !== scope) {
+      const childIndex = scope.parentIndex!
+      this.scopes[childIndex] = last
+      last.parentIndex = childIndex
     }
   }
 }
@@ -62,7 +84,7 @@ export function effectScope(detached?: boolean) {
 }
 
 export function recordEffectScope(
-  effect: ReactiveEffect | EffectScope,
+  effect: ReactiveEffect,
   scope?: EffectScope | null
 ) {
   scope = scope || activeEffectScope