Skip to content

Commit 91f2954

Browse files
authored
refactor(reactivity): optimize child effect scope dereferencing (#4184)
1 parent 3b38c9a commit 91f2954

File tree

2 files changed

+37
-14
lines changed

2 files changed

+37
-14
lines changed

packages/reactivity/__tests__/effectScope.spec.ts

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -77,8 +77,9 @@ describe('reactivity/effect/scope', () => {
7777
})
7878
})
7979

80-
expect(scope.effects.length).toBe(2)
81-
expect(scope.effects[1]).toBeInstanceOf(EffectScope)
80+
expect(scope.effects.length).toBe(1)
81+
expect(scope.scopes.length).toBe(1)
82+
expect(scope.scopes[0]).toBeInstanceOf(EffectScope)
8283

8384
expect(dummy).toBe(0)
8485
counter.num = 7
@@ -194,9 +195,9 @@ describe('reactivity/effect/scope', () => {
194195
it('should derefence child scope from parent scope after stopping child scope (no memleaks)', async () => {
195196
const parent = new EffectScope()
196197
const child = parent.run(() => new EffectScope())!
197-
expect(parent.effects.includes(child)).toBe(true)
198+
expect(parent.scopes.includes(child)).toBe(true)
198199
child.stop()
199-
expect(parent.effects.includes(child)).toBe(false)
200+
expect(parent.scopes.includes(child)).toBe(false)
200201
})
201202

202203
it('test with higher level APIs', async () => {

packages/reactivity/src/effectScope.ts

Lines changed: 32 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,3 @@
1-
import { remove } from '@vue/shared'
21
import { ReactiveEffect } from './effect'
32
import { warn } from './warning'
43

@@ -7,17 +6,23 @@ const effectScopeStack: EffectScope[] = []
76

87
export class EffectScope {
98
active = true
10-
effects: (ReactiveEffect | EffectScope)[] = []
9+
effects: ReactiveEffect[] = []
1110
cleanups: (() => void)[] = []
1211
parent: EffectScope | undefined
12+
private children: EffectScope[] | undefined
13+
private parentIndex: number | undefined
1314

1415
constructor(detached = false) {
1516
if (!detached) {
16-
recordEffectScope(this)
17-
this.parent = activeEffectScope
17+
this.recordEffectScope()
1818
}
1919
}
2020

21+
get scopes(): EffectScope[] {
22+
if (!this.children) this.children = []
23+
return this.children
24+
}
25+
2126
run<T>(fn: () => T): T | undefined {
2227
if (this.active) {
2328
try {
@@ -45,14 +50,31 @@ export class EffectScope {
4550
}
4651
}
4752

48-
stop(fromParent = false) {
53+
stop() {
4954
if (this.active) {
50-
this.effects.forEach(e => e.stop(true))
55+
this.effects.forEach(e => e.stop())
56+
this.children?.forEach(e => e.stop())
5157
this.cleanups.forEach(cleanup => cleanup())
58+
this.parent?.derefChildScope(this)
5259
this.active = false
53-
if (!fromParent && this.parent) {
54-
remove(this.parent.effects, this)
55-
}
60+
}
61+
}
62+
63+
private recordEffectScope() {
64+
const parent = activeEffectScope
65+
if (parent && parent.active) {
66+
this.parent = parent
67+
this.parentIndex = parent.scopes.push(this) - 1
68+
}
69+
}
70+
71+
private derefChildScope(scope: EffectScope) {
72+
// reuse the freed index by moving the last array entry
73+
const last = this.scopes.pop()
74+
if (last && last !== scope) {
75+
const childIndex = scope.parentIndex!
76+
this.scopes[childIndex] = last
77+
last.parentIndex = childIndex
5678
}
5779
}
5880
}
@@ -62,7 +84,7 @@ export function effectScope(detached?: boolean) {
6284
}
6385

6486
export function recordEffectScope(
65-
effect: ReactiveEffect | EffectScope,
87+
effect: ReactiveEffect,
6688
scope?: EffectScope | null
6789
) {
6890
scope = scope || activeEffectScope

0 commit comments

Comments
 (0)