From c74bb8c2dd9e82aaabb0a2a2b368e900929b513b Mon Sep 17 00:00:00 2001 From: Evan You Date: Mon, 16 Sep 2024 16:00:31 +0800 Subject: [PATCH] fix(reactivity): avoid exponential perf cost and reduce call stack depth for deeply chained computeds (#11944) close #11928 --- packages/reactivity/src/computed.ts | 13 +++++++--- packages/reactivity/src/dep.ts | 13 +++++----- packages/reactivity/src/effect.ts | 38 ++++++++++++++++++++--------- 3 files changed, 42 insertions(+), 22 deletions(-) diff --git a/packages/reactivity/src/computed.ts b/packages/reactivity/src/computed.ts index a5f8e5a3c2b..b16b011c5b7 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' @@ -109,11 +110,15 @@ export class ComputedRefImpl implements Subscriber { /** * @internal */ - notify(): void { + notify(): true | void { this.flags |= EffectFlags.DIRTY - // avoid infinite self recursion - if (activeSub !== this) { - this.dep.notify() + if ( + !(this.flags & EffectFlags.NOTIFIED) && + // avoid infinite self recursion + activeSub !== this + ) { + batch(this) + return true } else if (__DEV__) { // TODO warn } diff --git a/packages/reactivity/src/dep.ts b/packages/reactivity/src/dep.ts index 8e4ad1e649e..c24f123ded4 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( { @@ -180,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 2dae285d166..b8dd62a0f6e 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, @@ -69,7 +72,13 @@ export interface Subscriber extends DebuggerOptions { /** * @internal */ - notify(): void + next?: Subscriber + /** + * returning `true` indicates it's a computed that needs to call notify + * on its dep too + * @internal + */ + notify(): true | void } const pausedQueueEffects = new WeakSet() @@ -92,7 +101,7 @@ export class ReactiveEffect /** * @internal */ - nextEffect?: ReactiveEffect = undefined + next?: Subscriber = undefined /** * @internal */ @@ -134,9 +143,7 @@ export class ReactiveEffect return } if (!(this.flags & EffectFlags.NOTIFIED)) { - this.flags |= EffectFlags.NOTIFIED - this.nextEffect = batchedEffect - batchedEffect = this + batch(this) } } @@ -226,7 +233,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 +258,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 }