From 3bc798ee0a8af4befa89e23429302e8ce7c64270 Mon Sep 17 00:00:00 2001 From: Evan You Date: Mon, 16 Sep 2024 13:53:18 +0800 Subject: [PATCH 1/3] fix(reactivity): avoid exponential perf cost with deeply chained computeds close #11928 --- packages/reactivity/src/computed.ts | 9 ++++++-- packages/reactivity/src/dep.ts | 6 +---- packages/reactivity/src/effect.ts | 35 ++++++++++++++++++++--------- 3 files changed, 32 insertions(+), 18 deletions(-) diff --git a/packages/reactivity/src/computed.ts b/packages/reactivity/src/computed.ts index a5f8e5a3c2b..a07d5d52afe 100644 --- a/packages/reactivity/src/computed.ts +++ b/packages/reactivity/src/computed.ts @@ -5,6 +5,7 @@ import { EffectFlags, type Subscriber, activeSub, + batch, refreshComputed, } from './effect' import type { Ref } from './ref' @@ -111,8 +112,12 @@ export class ComputedRefImpl implements Subscriber { */ notify(): void { this.flags |= EffectFlags.DIRTY - // avoid infinite self recursion - if (activeSub !== this) { + if ( + !(this.flags & EffectFlags.NOTIFIED) && + // avoid infinite self recursion + activeSub !== this + ) { + batch(this) this.dep.notify() } else if (__DEV__) { // TODO warn diff --git a/packages/reactivity/src/dep.ts b/packages/reactivity/src/dep.ts index 8e4ad1e649e..d6290d69af2 100644 --- a/packages/reactivity/src/dep.ts +++ b/packages/reactivity/src/dep.ts @@ -163,11 +163,7 @@ export class Dep { // original order at the end of the batch, but onTrigger hooks should // be invoked in original order here. for (let head = this.subsHead; head; head = head.nextSub) { - if ( - __DEV__ && - head.sub.onTrigger && - !(head.sub.flags & EffectFlags.NOTIFIED) - ) { + if (head.sub.onTrigger && !(head.sub.flags & EffectFlags.NOTIFIED)) { head.sub.onTrigger( extend( { diff --git a/packages/reactivity/src/effect.ts b/packages/reactivity/src/effect.ts index 2dae285d166..7978c3d514b 100644 --- a/packages/reactivity/src/effect.ts +++ b/packages/reactivity/src/effect.ts @@ -39,6 +39,9 @@ export interface ReactiveEffectRunner { export let activeSub: Subscriber | undefined export enum EffectFlags { + /** + * ReactiveEffect only + */ ACTIVE = 1 << 0, RUNNING = 1 << 1, TRACKING = 1 << 2, @@ -66,6 +69,10 @@ export interface Subscriber extends DebuggerOptions { * @internal */ flags: EffectFlags + /** + * @internal + */ + next?: Subscriber /** * @internal */ @@ -92,7 +99,7 @@ export class ReactiveEffect /** * @internal */ - nextEffect?: ReactiveEffect = undefined + next?: Subscriber = undefined /** * @internal */ @@ -134,9 +141,7 @@ export class ReactiveEffect return } if (!(this.flags & EffectFlags.NOTIFIED)) { - this.flags |= EffectFlags.NOTIFIED - this.nextEffect = batchedEffect - batchedEffect = this + batch(this) } } @@ -226,7 +231,13 @@ export class ReactiveEffect // } let batchDepth = 0 -let batchedEffect: ReactiveEffect | undefined +let batchedSub: Subscriber | undefined + +export function batch(sub: Subscriber): void { + sub.flags |= EffectFlags.NOTIFIED + sub.next = batchedSub + batchedSub = sub +} /** * @internal @@ -245,16 +256,17 @@ export function endBatch(): void { } let error: unknown - while (batchedEffect) { - let e: ReactiveEffect | undefined = batchedEffect - batchedEffect = undefined + while (batchedSub) { + let e: Subscriber | undefined = batchedSub + batchedSub = undefined while (e) { - const next: ReactiveEffect | undefined = e.nextEffect - e.nextEffect = undefined + const next: Subscriber | undefined = e.next + e.next = undefined e.flags &= ~EffectFlags.NOTIFIED if (e.flags & EffectFlags.ACTIVE) { try { - e.trigger() + // ACTIVE flag is effect-only + ;(e as ReactiveEffect).trigger() } catch (err) { if (!error) error = err } @@ -331,6 +343,7 @@ function isDirty(sub: Subscriber): boolean { * @internal */ export function refreshComputed(computed: ComputedRefImpl): undefined { + computed.flags &= ~EffectFlags.NOTIFIED if ( computed.flags & EffectFlags.TRACKING && !(computed.flags & EffectFlags.DIRTY) From adbf7749a12e4da9bb9e20e3e245b7b96bf45f40 Mon Sep 17 00:00:00 2001 From: Evan You Date: Mon, 16 Sep 2024 15:43:53 +0800 Subject: [PATCH 2/3] refactor: reduce call stack depth for chained computeds --- packages/reactivity/src/computed.ts | 4 ++-- packages/reactivity/src/dep.ts | 7 ++++++- packages/reactivity/src/effect.ts | 4 +++- 3 files changed, 11 insertions(+), 4 deletions(-) diff --git a/packages/reactivity/src/computed.ts b/packages/reactivity/src/computed.ts index a07d5d52afe..b16b011c5b7 100644 --- a/packages/reactivity/src/computed.ts +++ b/packages/reactivity/src/computed.ts @@ -110,7 +110,7 @@ export class ComputedRefImpl implements Subscriber { /** * @internal */ - notify(): void { + notify(): true | void { this.flags |= EffectFlags.DIRTY if ( !(this.flags & EffectFlags.NOTIFIED) && @@ -118,7 +118,7 @@ export class ComputedRefImpl implements Subscriber { activeSub !== this ) { batch(this) - this.dep.notify() + return true } else if (__DEV__) { // TODO warn } diff --git a/packages/reactivity/src/dep.ts b/packages/reactivity/src/dep.ts index d6290d69af2..c24f123ded4 100644 --- a/packages/reactivity/src/dep.ts +++ b/packages/reactivity/src/dep.ts @@ -176,7 +176,12 @@ export class Dep { } } for (let link = this.subs; link; link = link.prevSub) { - link.sub.notify() + if (link.sub.notify()) { + // if notify() returns `true`, this is a computed. Also call notify + // on its dep - it's called here instead of inside computed's notify + // in order to reduce call stack depth. + ;(link.sub as ComputedRefImpl).dep.notify() + } } } finally { endBatch() diff --git a/packages/reactivity/src/effect.ts b/packages/reactivity/src/effect.ts index 7978c3d514b..efbdf7471df 100644 --- a/packages/reactivity/src/effect.ts +++ b/packages/reactivity/src/effect.ts @@ -74,9 +74,11 @@ export interface Subscriber extends DebuggerOptions { */ next?: Subscriber /** + * returning `true` indicates it's a computed that needs to call notify + * on its dep too * @internal */ - notify(): void + notify(): true | void } const pausedQueueEffects = new WeakSet() From d9dd0f3a3f05413f42eb25a62ea75291fe25d101 Mon Sep 17 00:00:00 2001 From: Evan You Date: Mon, 16 Sep 2024 15:54:34 +0800 Subject: [PATCH 3/3] chore: remove not needed statement --- packages/reactivity/src/effect.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/packages/reactivity/src/effect.ts b/packages/reactivity/src/effect.ts index efbdf7471df..b8dd62a0f6e 100644 --- a/packages/reactivity/src/effect.ts +++ b/packages/reactivity/src/effect.ts @@ -345,7 +345,6 @@ function isDirty(sub: Subscriber): boolean { * @internal */ export function refreshComputed(computed: ComputedRefImpl): undefined { - computed.flags &= ~EffectFlags.NOTIFIED if ( computed.flags & EffectFlags.TRACKING && !(computed.flags & EffectFlags.DIRTY)