From cd5c3efb548f08c19afb6f3ff822b3d9854b17d7 Mon Sep 17 00:00:00 2001 From: Evan You Date: Thu, 24 Jun 2021 16:07:18 -0400 Subject: [PATCH 1/4] perf(reactivity): refactor effect for memory usage improvement - Use class implementation for ReactiveEffect - Avoid options object allocation - Lazy initailize bound runner function only when necessary Benchmark showing around ~17% memory saving when creating computed and watcher instances. --- .../reactivity/__tests__/computed.spec.ts | 5 +- packages/reactivity/__tests__/effect.spec.ts | 22 +- .../reactivity/__tests__/readonly.spec.ts | 2 +- packages/reactivity/src/computed.ts | 18 +- packages/reactivity/src/effect.ts | 221 ++++++++++-------- packages/reactivity/src/index.ts | 2 + packages/runtime-core/src/apiWatch.ts | 34 +-- packages/runtime-core/src/compat/global.ts | 3 +- packages/runtime-core/src/component.ts | 7 +- packages/runtime-core/src/renderer.ts | 61 +++-- packages/runtime-core/src/scheduler.ts | 37 +-- 11 files changed, 220 insertions(+), 192 deletions(-) diff --git a/packages/reactivity/__tests__/computed.spec.ts b/packages/reactivity/__tests__/computed.spec.ts index 0a9f9ecc8d1..6811f1029e5 100644 --- a/packages/reactivity/__tests__/computed.spec.ts +++ b/packages/reactivity/__tests__/computed.spec.ts @@ -2,7 +2,6 @@ import { computed, reactive, effect, - stop, ref, WritableComputedRef, isReadonly @@ -125,7 +124,7 @@ describe('reactivity/computed', () => { expect(dummy).toBe(undefined) value.foo = 1 expect(dummy).toBe(1) - stop(cValue.effect) + cValue.effect.stop() value.foo = 2 expect(dummy).toBe(1) }) @@ -196,7 +195,7 @@ describe('reactivity/computed', () => { it('should expose value when stopped', () => { const x = computed(() => 1) - stop(x.effect) + x.effect.stop() expect(x.value).toBe(1) }) }) diff --git a/packages/reactivity/__tests__/effect.spec.ts b/packages/reactivity/__tests__/effect.spec.ts index e4a8f338c3e..89997abe357 100644 --- a/packages/reactivity/__tests__/effect.spec.ts +++ b/packages/reactivity/__tests__/effect.spec.ts @@ -490,12 +490,12 @@ describe('reactivity/effect', () => { expect(conditionalSpy).toHaveBeenCalledTimes(2) }) - it('should not double wrap if the passed function is a effect', () => { - const runner = effect(() => {}) - const otherRunner = effect(runner) - expect(runner).not.toBe(otherRunner) - expect(runner.raw).toBe(otherRunner.raw) - }) + // it('should not double wrap if the passed function is a effect', () => { + // const runner = effect(() => {}) + // const otherRunner = effect(runner) + // expect(runner).not.toBe(otherRunner) + // expect(runner.raw).toBe(otherRunner.raw) + // }) it('should not run multiple times for a single mutation', () => { let dummy @@ -633,19 +633,19 @@ describe('reactivity/effect', () => { expect(onTrack).toHaveBeenCalledTimes(3) expect(events).toEqual([ { - effect: runner, + effect: runner.effect, target: toRaw(obj), type: TrackOpTypes.GET, key: 'foo' }, { - effect: runner, + effect: runner.effect, target: toRaw(obj), type: TrackOpTypes.HAS, key: 'bar' }, { - effect: runner, + effect: runner.effect, target: toRaw(obj), type: TrackOpTypes.ITERATE, key: ITERATE_KEY @@ -671,7 +671,7 @@ describe('reactivity/effect', () => { expect(dummy).toBe(2) expect(onTrigger).toHaveBeenCalledTimes(1) expect(events[0]).toEqual({ - effect: runner, + effect: runner.effect, target: toRaw(obj), type: TriggerOpTypes.SET, key: 'foo', @@ -684,7 +684,7 @@ describe('reactivity/effect', () => { expect(dummy).toBeUndefined() expect(onTrigger).toHaveBeenCalledTimes(2) expect(events[1]).toEqual({ - effect: runner, + effect: runner.effect, target: toRaw(obj), type: TriggerOpTypes.DELETE, key: 'foo', diff --git a/packages/reactivity/__tests__/readonly.spec.ts b/packages/reactivity/__tests__/readonly.spec.ts index 80115b2645d..c8bf65b3876 100644 --- a/packages/reactivity/__tests__/readonly.spec.ts +++ b/packages/reactivity/__tests__/readonly.spec.ts @@ -382,7 +382,7 @@ describe('reactivity/readonly', () => { const eff = effect(() => { roArr.includes(2) }) - expect(eff.deps.length).toBe(0) + expect(eff.effect.deps.length).toBe(0) }) test('readonly should track and trigger if wrapping reactive original (collection)', () => { diff --git a/packages/reactivity/src/computed.ts b/packages/reactivity/src/computed.ts index 965ff3eec6b..396e50ecdb0 100644 --- a/packages/reactivity/src/computed.ts +++ b/packages/reactivity/src/computed.ts @@ -1,4 +1,4 @@ -import { effect, ReactiveEffect } from './effect' +import { ReactiveEffect } from './effect' import { Ref, trackRefValue, triggerRefValue } from './ref' import { isFunction, NOOP } from '@vue/shared' import { ReactiveFlags, toRaw } from './reactive' @@ -35,16 +35,12 @@ class ComputedRefImpl { private readonly _setter: ComputedSetter, isReadonly: boolean ) { - this.effect = effect(getter, { - lazy: true, - scheduler: () => { - if (!this._dirty) { - this._dirty = true - triggerRefValue(this) - } + this.effect = new ReactiveEffect(getter, () => { + if (!this._dirty) { + this._dirty = true + triggerRefValue(this) } }) - this[ReactiveFlags.IS_READONLY] = isReadonly } @@ -52,10 +48,10 @@ class ComputedRefImpl { // the computed ref may get wrapped by other proxies e.g. readonly() #3376 const self = toRaw(this) if (self._dirty) { - self._value = this.effect() + self._value = self.effect.run()! self._dirty = false } - trackRefValue(this) + trackRefValue(self) return self._value } diff --git a/packages/reactivity/src/effect.ts b/packages/reactivity/src/effect.ts index df74769e14b..baab9f97b25 100644 --- a/packages/reactivity/src/effect.ts +++ b/packages/reactivity/src/effect.ts @@ -1,5 +1,5 @@ import { TrackOpTypes, TriggerOpTypes } from './operations' -import { EMPTY_OBJ, extend, isArray, isIntegerKey, isMap } from '@vue/shared' +import { extend, isArray, isIntegerKey, isMap } from '@vue/shared' // The main WeakMap that stores {target -> key -> dep} connections. // Conceptually, it's easier to think of a dependency as a Dep class @@ -9,40 +9,7 @@ type Dep = Set type KeyToDepMap = Map const targetMap = new WeakMap() -export interface ReactiveEffect { - (): T - _isEffect: true - id: number - active: boolean - raw: () => T - deps: Array - options: ReactiveEffectOptions - allowRecurse: boolean -} - -export interface ReactiveEffectOptions { - lazy?: boolean - scheduler?: (job: ReactiveEffect) => void - onTrack?: (event: DebuggerEvent) => void - onTrigger?: (event: DebuggerEvent) => void - onStop?: () => void - /** - * Indicates whether the job is allowed to recursively trigger itself when - * managed by the scheduler. - * - * By default, a job cannot trigger itself because some built-in method calls, - * e.g. Array.prototype.push actually performs reads as well (#1740) which - * can lead to confusing infinite loops. - * The allowed cases are component update functions and watch callbacks. - * Component update functions may update child component props, which in turn - * trigger flush: "pre" watch callbacks that mutates state that the parent - * relies on (#1801). Watch callbacks doesn't track its dependencies so if it - * triggers itself again, it's likely intentional and it is the user's - * responsibility to perform recursive state mutation that eventually - * stabilizes (#1727). - */ - allowRecurse?: boolean -} +export type EffectScheduler = (job: () => void) => void export type DebuggerEvent = { effect: ReactiveEffect @@ -63,51 +30,58 @@ let activeEffect: ReactiveEffect | undefined export const ITERATE_KEY = Symbol(__DEV__ ? 'iterate' : '') export const MAP_KEY_ITERATE_KEY = Symbol(__DEV__ ? 'Map key iterate' : '') -export function isEffect(fn: any): fn is ReactiveEffect { - return fn && fn._isEffect === true -} +let uid = 0 -export function effect( - fn: () => T, - options: ReactiveEffectOptions = EMPTY_OBJ -): ReactiveEffect { - if (isEffect(fn)) { - fn = fn.raw - } - const effect = createReactiveEffect(fn, options) - if (!options.lazy) { - effect() - } - return effect +export interface ReactiveEffectRunner { + (): any + id: number + active: boolean + allowRecurse: boolean + effect: ReactiveEffect } -export function stop(effect: ReactiveEffect) { - if (effect.active) { - cleanup(effect) - if (effect.options.onStop) { - effect.options.onStop() - } - effect.active = false - } -} +export class ReactiveEffect { + active = true + deps: Dep[] = [] -let uid = 0 + // can be attached after creation + onStop?: () => void + // dev only + onTrack?: (event: DebuggerEvent) => void + // dev only + onTrigger?: (event: DebuggerEvent) => void -function createReactiveEffect( - fn: () => T, - options: ReactiveEffectOptions -): ReactiveEffect { - const effect = function reactiveEffect(): unknown { - if (!effect.active) { - return fn() + constructor( + public fn: () => T, + public scheduler: EffectScheduler | null = null, + /** + * Indicates whether the effect is allowed to recursively trigger itself + * when managed by the scheduler. + * + * By default, a job cannot trigger itself because some built-in method calls, + * e.g. Array.prototype.push actually performs reads as well (#1740) which + * can lead to confusing infinite loops. + * The allowed cases are component update functions and watch callbacks. + * Component update functions may update child component props, which in turn + * trigger flush: "pre" watch callbacks that mutates state that the parent + * relies on (#1801). Watch callbacks doesn't track its dependencies so if it + * triggers itself again, it's likely intentional and it is the user's + * responsibility to perform recursive state mutation that eventually + * stabilizes (#1727). + */ + public allowRecurse = false + ) {} + + run() { + if (!this.active) { + return this.fn() } - if (!effectStack.includes(effect)) { - cleanup(effect) + if (!effectStack.includes(this)) { + this.cleanup() try { enableTracking() - effectStack.push(effect) - activeEffect = effect - return fn() + effectStack.push((activeEffect = this)) + return this.fn() } finally { effectStack.pop() resetTracking() @@ -115,27 +89,80 @@ function createReactiveEffect( activeEffect = n > 0 ? effectStack[n - 1] : undefined } } - } as ReactiveEffect - effect.id = uid++ - effect.allowRecurse = !!options.allowRecurse - effect._isEffect = true - effect.active = true - effect.raw = fn - effect.deps = [] - effect.options = options - return effect -} + } + + /** + * Lazy initialized bound runner function that can be passed to a scheduler. + * Also attaches a few properties that are needed by @vue/runtime-core's + * scheduler. + * + * This is only needed when a scheduler is used so making it lazy provides + * decent memory savings. + */ + _boundRun?: ReactiveEffectRunner + get boundRun(): ReactiveEffectRunner { + if (this._boundRun) { + return this._boundRun + } + const run = (this._boundRun = this.run.bind(this) as ReactiveEffectRunner) + run.id = uid++ + run.active = this.active + run.allowRecurse = this.allowRecurse + run.effect = this + return run + } + + cleanup() { + const { deps } = this + if (deps.length) { + for (let i = 0; i < deps.length; i++) { + deps[i].delete(this) + } + deps.length = 0 + } + } -function cleanup(effect: ReactiveEffect) { - const { deps } = effect - if (deps.length) { - for (let i = 0; i < deps.length; i++) { - deps[i].delete(effect) + stop() { + if (this.active) { + this.cleanup() + if (this.onStop) { + this.onStop() + } + this.active = false + if (this._boundRun) { + this._boundRun.active = false + } } - deps.length = 0 } } +export interface ReactiveEffectOptions { + lazy?: boolean + scheduler?: EffectScheduler + allowRecurse?: boolean + onStop?: () => void + onTrack?: (event: DebuggerEvent) => void + onTrigger?: (event: DebuggerEvent) => void +} + +export function effect( + fn: () => T, + options?: ReactiveEffectOptions +): ReactiveEffectRunner { + const _effect = new ReactiveEffect(fn) + if (options) { + extend(_effect, options) + } + if (!options || !options.lazy) { + _effect.run() + } + return _effect.boundRun +} + +export function stop(runner: ReactiveEffectRunner) { + runner.effect.stop() +} + let shouldTrack = true const trackStack: boolean[] = [] @@ -185,8 +212,8 @@ export function trackEffects( if (!dep.has(activeEffect!)) { dep.add(activeEffect!) activeEffect!.deps.push(dep) - if (__DEV__ && activeEffect!.options.onTrack) { - activeEffect!.options.onTrack( + if (__DEV__ && activeEffect!.onTrack) { + activeEffect!.onTrack( Object.assign( { effect: activeEffect! @@ -284,13 +311,17 @@ export function triggerEffects( // spread into array for stabilization for (const effect of [...dep]) { if (effect !== activeEffect || effect.allowRecurse) { - if (__DEV__ && effect.options.onTrigger) { - effect.options.onTrigger(extend({ effect }, debuggerEventExtraInfo)) + if (__DEV__ && effect.onTrigger) { + effect.onTrigger(extend({ effect }, debuggerEventExtraInfo)) } - if (effect.options.scheduler) { - effect.options.scheduler(effect) + const { scheduler } = effect + if (scheduler) { + // optimization: avoid creating bound runner fn when scheduler expects + // no arguments. + // @ts-ignore + scheduler(scheduler.length ? effect.boundRun : null) } else { - effect() + effect.run() } } } diff --git a/packages/reactivity/src/index.ts b/packages/reactivity/src/index.ts index 240410141e4..e392f182439 100644 --- a/packages/reactivity/src/index.ts +++ b/packages/reactivity/src/index.ts @@ -46,7 +46,9 @@ export { resetTracking, ITERATE_KEY, ReactiveEffect, + ReactiveEffectRunner, ReactiveEffectOptions, + EffectScheduler, DebuggerEvent } from './effect' export { TrackOpTypes, TriggerOpTypes } from './operations' diff --git a/packages/runtime-core/src/apiWatch.ts b/packages/runtime-core/src/apiWatch.ts index b7092248e24..37b817eba53 100644 --- a/packages/runtime-core/src/apiWatch.ts +++ b/packages/runtime-core/src/apiWatch.ts @@ -1,12 +1,12 @@ import { - effect, - stop, isRef, Ref, ComputedRef, + ReactiveEffect, ReactiveEffectOptions, isReactive, - ReactiveFlags + ReactiveFlags, + EffectScheduler } from '@vue/reactivity' import { SchedulerJob, queuePreFlushCb } from './scheduler' import { @@ -244,7 +244,7 @@ function doWatch( let cleanup: () => void let onInvalidate: InvalidateCbRegistrator = (fn: () => void) => { - cleanup = runner.options.onStop = () => { + cleanup = runner.onStop = () => { callWithErrorHandling(fn, instance, ErrorCodes.WATCH_CLEANUP) } } @@ -273,7 +273,7 @@ function doWatch( } if (cb) { // watch(source, cb) - const newValue = runner() + const newValue = runner.run() if ( deep || forceTrigger || @@ -300,7 +300,7 @@ function doWatch( } } else { // watchEffect - runner() + runner.run() } } @@ -308,7 +308,7 @@ function doWatch( // it is allowed to self-trigger (#1727) job.allowRecurse = !!cb - let scheduler: ReactiveEffectOptions['scheduler'] + let scheduler: EffectScheduler if (flush === 'sync') { scheduler = job as any // the scheduler function gets called directly } else if (flush === 'post') { @@ -326,12 +326,12 @@ function doWatch( } } - const runner = effect(getter, { - lazy: true, - onTrack, - onTrigger, - scheduler - }) + const runner = new ReactiveEffect(getter, scheduler) + + if (__DEV__) { + runner.onTrack = onTrack + runner.onTrigger = onTrigger + } recordInstanceBoundEffect(runner, instance) @@ -340,16 +340,16 @@ function doWatch( if (immediate) { job() } else { - oldValue = runner() + oldValue = runner.run() } } else if (flush === 'post') { - queuePostRenderEffect(runner, instance && instance.suspense) + queuePostRenderEffect(runner.boundRun, instance && instance.suspense) } else { - runner() + runner.run() } return () => { - stop(runner) + runner.stop() if (instance) { remove(instance.effects!, runner) } diff --git a/packages/runtime-core/src/compat/global.ts b/packages/runtime-core/src/compat/global.ts index 7823cc6a7e1..f843b5ead14 100644 --- a/packages/runtime-core/src/compat/global.ts +++ b/packages/runtime-core/src/compat/global.ts @@ -1,7 +1,6 @@ import { isReactive, reactive, - stop, track, TrackOpTypes, trigger, @@ -575,7 +574,7 @@ function installCompatMount( // stop effects if (effects) { for (let i = 0; i < effects.length; i++) { - stop(effects[i]) + effects[i].stop() } } // unmounted hook diff --git a/packages/runtime-core/src/component.ts b/packages/runtime-core/src/component.ts index 78cabdd3538..b2d1ca1052b 100644 --- a/packages/runtime-core/src/component.ts +++ b/packages/runtime-core/src/component.ts @@ -4,7 +4,8 @@ import { pauseTracking, resetTracking, shallowReadonly, - proxyRefs + proxyRefs, + ReactiveEffectRunner } from '@vue/reactivity' import { ComponentPublicInstance, @@ -215,9 +216,9 @@ export interface ComponentInternalInstance { */ subTree: VNode /** - * The reactive effect for rendering and patching the component. Callable. + * Bound effect runner */ - update: ReactiveEffect + update: ReactiveEffectRunner /** * The render function that returns vdom tree. * @internal diff --git a/packages/runtime-core/src/renderer.ts b/packages/runtime-core/src/renderer.ts index c7549ae1598..707b75ddc54 100644 --- a/packages/runtime-core/src/renderer.ts +++ b/packages/runtime-core/src/renderer.ts @@ -47,15 +47,13 @@ import { flushPostFlushCbs, invalidateJob, flushPreFlushCbs, - SchedulerCb + SchedulerJob } from './scheduler' import { - effect, - stop, - ReactiveEffectOptions, isRef, pauseTracking, - resetTracking + resetTracking, + ReactiveEffect } from '@vue/reactivity' import { updateProps } from './componentProps' import { updateSlots } from './componentSlots' @@ -285,23 +283,6 @@ export const enum MoveType { REORDER } -const prodEffectOptions = { - scheduler: queueJob, - // #1801, #2043 component render effects should allow recursive updates - allowRecurse: true -} - -function createDevEffectOptions( - instance: ComponentInternalInstance -): ReactiveEffectOptions { - return { - scheduler: queueJob, - allowRecurse: true, - onTrack: instance.rtc ? e => invokeArrayFns(instance.rtc!, e) : void 0, - onTrigger: instance.rtg ? e => invokeArrayFns(instance.rtg!, e) : void 0 - } -} - export const queuePostRenderEffect = __FEATURE_SUSPENSE__ ? queueEffectWithSuspense : queuePostFlushCb @@ -377,7 +358,7 @@ export const setRef = ( // null values means this is unmount and it should not overwrite another // ref with the same key if (value) { - ;(doSet as SchedulerCb).id = -1 + ;(doSet as SchedulerJob).id = -1 queuePostRenderEffect(doSet, parentSuspense) } else { doSet() @@ -387,7 +368,7 @@ export const setRef = ( ref.value = value } if (value) { - ;(doSet as SchedulerCb).id = -1 + ;(doSet as SchedulerJob).id = -1 queuePostRenderEffect(doSet, parentSuspense) } else { doSet() @@ -1393,7 +1374,7 @@ function baseCreateRenderer( // in case the child component is also queued, remove it to avoid // double updating the same child component in the same flush. invalidateJob(instance.update) - // instance.update is the reactive effect runner. + // instance.update is the reactive effect. instance.update() } } else { @@ -1413,8 +1394,7 @@ function baseCreateRenderer( isSVG, optimized ) => { - // create reactive effect for rendering - instance.update = effect(function componentEffect() { + const componentUpdateFn = () => { if (!instance.isMounted) { let vnodeHook: VNodeHook | null | undefined const { el, props } = initialVNode @@ -1638,12 +1618,31 @@ function baseCreateRenderer( popWarningContext() } } - }, __DEV__ ? createDevEffectOptions(instance) : prodEffectOptions) + } + + // create reactive effect for rendering + const effect = new ReactiveEffect( + componentUpdateFn, + queueJob, + // allowRecurse + // #1801, #2043 component render effects should allow recursive updates + true + ) + + instance.update = effect.boundRun if (__DEV__) { - // @ts-ignore + effect.onTrack = instance.rtc + ? e => invokeArrayFns(instance.rtc!, e) + : void 0 + effect.onTrigger = instance.rtg + ? e => invokeArrayFns(instance.rtg!, e) + : void 0 + // @ts-ignore (for scheduler) instance.update.ownerInstance = instance } + + instance.update() } const updateComponentPreRender = ( @@ -2298,13 +2297,13 @@ function baseCreateRenderer( if (effects) { for (let i = 0; i < effects.length; i++) { - stop(effects[i]) + effects[i].stop() } } // update may be null if a component is unmounted before its async // setup has resolved. if (update) { - stop(update) + update.effect.stop() unmount(subTree, instance, parentSuspense, doRemove) } // unmounted hook diff --git a/packages/runtime-core/src/scheduler.ts b/packages/runtime-core/src/scheduler.ts index adaf3c26e84..510bfc5d1cd 100644 --- a/packages/runtime-core/src/scheduler.ts +++ b/packages/runtime-core/src/scheduler.ts @@ -3,9 +3,11 @@ import { isArray } from '@vue/shared' import { ComponentPublicInstance } from './componentPublicInstance' import { ComponentInternalInstance, getComponentName } from './component' import { warn } from './warning' -import { ReactiveEffect } from '@vue/reactivity' +import { ReactiveEffectRunner } from '@vue/reactivity' -export interface SchedulerJob extends Function, Partial { +export interface SchedulerJob extends Function, Partial { + id?: number + allowRecurse?: boolean /** * Attached by renderer.ts when setting up a component's render effect * Used to obtain component information when reporting max recursive updates. @@ -14,8 +16,7 @@ export interface SchedulerJob extends Function, Partial { ownerInstance?: ComponentInternalInstance } -export type SchedulerCb = Function & { id?: number } -export type SchedulerCbs = SchedulerCb | SchedulerCb[] +export type SchedulerJobs = SchedulerJob | SchedulerJob[] let isFlushing = false let isFlushPending = false @@ -23,12 +24,12 @@ let isFlushPending = false const queue: SchedulerJob[] = [] let flushIndex = 0 -const pendingPreFlushCbs: SchedulerCb[] = [] -let activePreFlushCbs: SchedulerCb[] | null = null +const pendingPreFlushCbs: SchedulerJob[] = [] +let activePreFlushCbs: SchedulerJob[] | null = null let preFlushIndex = 0 -const pendingPostFlushCbs: SchedulerCb[] = [] -let activePostFlushCbs: SchedulerCb[] | null = null +const pendingPostFlushCbs: SchedulerJob[] = [] +let activePostFlushCbs: SchedulerJob[] | null = null let postFlushIndex = 0 const resolvedPromise: Promise = Promise.resolve() @@ -37,7 +38,7 @@ let currentFlushPromise: Promise | null = null let currentPreFlushParentJob: SchedulerJob | null = null const RECURSION_LIMIT = 100 -type CountMap = Map +type CountMap = Map export function nextTick( this: ComponentPublicInstance | void, @@ -106,9 +107,9 @@ export function invalidateJob(job: SchedulerJob) { } function queueCb( - cb: SchedulerCbs, - activeQueue: SchedulerCb[] | null, - pendingQueue: SchedulerCb[], + cb: SchedulerJobs, + activeQueue: SchedulerJob[] | null, + pendingQueue: SchedulerJob[], index: number ) { if (!isArray(cb)) { @@ -130,11 +131,11 @@ function queueCb( queueFlush() } -export function queuePreFlushCb(cb: SchedulerCb) { +export function queuePreFlushCb(cb: SchedulerJob) { queueCb(cb, activePreFlushCbs, pendingPreFlushCbs, preFlushIndex) } -export function queuePostFlushCb(cb: SchedulerCbs) { +export function queuePostFlushCb(cb: SchedulerJobs) { queueCb(cb, activePostFlushCbs, pendingPostFlushCbs, postFlushIndex) } @@ -206,8 +207,8 @@ export function flushPostFlushCbs(seen?: CountMap) { } } -const getId = (job: SchedulerJob | SchedulerCb) => - job.id == null ? Infinity : job.id +const getId = (job: SchedulerJob): number => + job.id == null ? Infinity : job.id! function flushJobs(seen?: CountMap) { isFlushPending = false @@ -257,13 +258,13 @@ function flushJobs(seen?: CountMap) { } } -function checkRecursiveUpdates(seen: CountMap, fn: SchedulerJob | SchedulerCb) { +function checkRecursiveUpdates(seen: CountMap, fn: SchedulerJob) { if (!seen.has(fn)) { seen.set(fn, 1) } else { const count = seen.get(fn)! if (count > RECURSION_LIMIT) { - const instance = (fn as SchedulerJob).ownerInstance + const instance = fn.ownerInstance const componentName = instance && getComponentName(instance.type) warn( `Maximum recursive updates exceeded${ From b0447b35f7d9667b1030310b92b399b7f9c26aab Mon Sep 17 00:00:00 2001 From: Evan You Date: Thu, 24 Jun 2021 17:23:55 -0400 Subject: [PATCH 2/4] perf: avoid cost of passing bound runner to effect scheduler - introduces a minor breaking change where the scheduler no longer receives the bound runner --- packages/reactivity/__tests__/effect.spec.ts | 11 ++-- packages/reactivity/src/effect.ts | 54 ++++--------------- .../runtime-core/__tests__/scheduler.spec.ts | 14 +++-- packages/runtime-core/src/apiWatch.ts | 29 +++++----- packages/runtime-core/src/component.ts | 14 +++-- packages/runtime-core/src/renderer.ts | 28 +++++----- packages/runtime-core/src/scheduler.ts | 4 +- 7 files changed, 67 insertions(+), 87 deletions(-) diff --git a/packages/reactivity/__tests__/effect.spec.ts b/packages/reactivity/__tests__/effect.spec.ts index 89997abe357..3e19d37ebb5 100644 --- a/packages/reactivity/__tests__/effect.spec.ts +++ b/packages/reactivity/__tests__/effect.spec.ts @@ -590,12 +590,13 @@ describe('reactivity/effect', () => { }) it('scheduler', () => { - let runner: any, dummy - const scheduler = jest.fn(_runner => { - runner = _runner + let dummy + let run: any + const scheduler = jest.fn(() => { + run = runner }) const obj = reactive({ foo: 1 }) - effect( + const runner = effect( () => { dummy = obj.foo }, @@ -609,7 +610,7 @@ describe('reactivity/effect', () => { // should not run yet expect(dummy).toBe(1) // manually run - runner() + run() // should have run expect(dummy).toBe(2) }) diff --git a/packages/reactivity/src/effect.ts b/packages/reactivity/src/effect.ts index baab9f97b25..bac2e6ea6b7 100644 --- a/packages/reactivity/src/effect.ts +++ b/packages/reactivity/src/effect.ts @@ -9,7 +9,7 @@ type Dep = Set type KeyToDepMap = Map const targetMap = new WeakMap() -export type EffectScheduler = (job: () => void) => void +export type EffectScheduler = () => void export type DebuggerEvent = { effect: ReactiveEffect @@ -29,17 +29,6 @@ let activeEffect: ReactiveEffect | undefined export const ITERATE_KEY = Symbol(__DEV__ ? 'iterate' : '') export const MAP_KEY_ITERATE_KEY = Symbol(__DEV__ ? 'Map key iterate' : '') - -let uid = 0 - -export interface ReactiveEffectRunner { - (): any - id: number - active: boolean - allowRecurse: boolean - effect: ReactiveEffect -} - export class ReactiveEffect { active = true deps: Dep[] = [] @@ -91,27 +80,6 @@ export class ReactiveEffect { } } - /** - * Lazy initialized bound runner function that can be passed to a scheduler. - * Also attaches a few properties that are needed by @vue/runtime-core's - * scheduler. - * - * This is only needed when a scheduler is used so making it lazy provides - * decent memory savings. - */ - _boundRun?: ReactiveEffectRunner - get boundRun(): ReactiveEffectRunner { - if (this._boundRun) { - return this._boundRun - } - const run = (this._boundRun = this.run.bind(this) as ReactiveEffectRunner) - run.id = uid++ - run.active = this.active - run.allowRecurse = this.allowRecurse - run.effect = this - return run - } - cleanup() { const { deps } = this if (deps.length) { @@ -129,9 +97,6 @@ export class ReactiveEffect { this.onStop() } this.active = false - if (this._boundRun) { - this._boundRun.active = false - } } } } @@ -145,6 +110,11 @@ export interface ReactiveEffectOptions { onTrigger?: (event: DebuggerEvent) => void } +export interface ReactiveEffectRunner { + (): T + effect: ReactiveEffect +} + export function effect( fn: () => T, options?: ReactiveEffectOptions @@ -156,7 +126,9 @@ export function effect( if (!options || !options.lazy) { _effect.run() } - return _effect.boundRun + const runner = _effect.run.bind(_effect) as ReactiveEffectRunner + runner.effect = _effect + return runner } export function stop(runner: ReactiveEffectRunner) { @@ -314,12 +286,8 @@ export function triggerEffects( if (__DEV__ && effect.onTrigger) { effect.onTrigger(extend({ effect }, debuggerEventExtraInfo)) } - const { scheduler } = effect - if (scheduler) { - // optimization: avoid creating bound runner fn when scheduler expects - // no arguments. - // @ts-ignore - scheduler(scheduler.length ? effect.boundRun : null) + if (effect.scheduler) { + effect.scheduler() } else { effect.run() } diff --git a/packages/runtime-core/__tests__/scheduler.spec.ts b/packages/runtime-core/__tests__/scheduler.spec.ts index 5c686334416..92727e99537 100644 --- a/packages/runtime-core/__tests__/scheduler.spec.ts +++ b/packages/runtime-core/__tests__/scheduler.spec.ts @@ -1,4 +1,3 @@ -import { effect, stop } from '@vue/reactivity' import { queueJob, nextTick, @@ -576,20 +575,19 @@ describe('scheduler', () => { // simulate parent component that toggles child const job1 = () => { - stop(job2) + // @ts-ignore + job2.active = false } - job1.id = 0 // need the id to ensure job1 is sorted before job2 - // simulate child that's triggered by the same reactive change that // triggers its toggle - const job2 = effect(() => spy()) - expect(spy).toHaveBeenCalledTimes(1) + const job2 = () => spy() + expect(spy).toHaveBeenCalledTimes(0) queueJob(job1) queueJob(job2) await nextTick() - // should not be called again - expect(spy).toHaveBeenCalledTimes(1) + // should not be called + expect(spy).toHaveBeenCalledTimes(0) }) }) diff --git a/packages/runtime-core/src/apiWatch.ts b/packages/runtime-core/src/apiWatch.ts index 37b817eba53..ecea6588892 100644 --- a/packages/runtime-core/src/apiWatch.ts +++ b/packages/runtime-core/src/apiWatch.ts @@ -244,7 +244,7 @@ function doWatch( let cleanup: () => void let onInvalidate: InvalidateCbRegistrator = (fn: () => void) => { - cleanup = runner.onStop = () => { + cleanup = effect.onStop = () => { callWithErrorHandling(fn, instance, ErrorCodes.WATCH_CLEANUP) } } @@ -268,12 +268,12 @@ function doWatch( let oldValue = isMultiSource ? [] : INITIAL_WATCHER_VALUE const job: SchedulerJob = () => { - if (!runner.active) { + if (!effect.active) { return } if (cb) { // watch(source, cb) - const newValue = runner.run() + const newValue = effect.run() if ( deep || forceTrigger || @@ -300,7 +300,7 @@ function doWatch( } } else { // watchEffect - runner.run() + effect.run() } } @@ -326,32 +326,35 @@ function doWatch( } } - const runner = new ReactiveEffect(getter, scheduler) + const effect = new ReactiveEffect(getter, scheduler) if (__DEV__) { - runner.onTrack = onTrack - runner.onTrigger = onTrigger + effect.onTrack = onTrack + effect.onTrigger = onTrigger } - recordInstanceBoundEffect(runner, instance) + recordInstanceBoundEffect(effect, instance) // initial run if (cb) { if (immediate) { job() } else { - oldValue = runner.run() + oldValue = effect.run() } } else if (flush === 'post') { - queuePostRenderEffect(runner.boundRun, instance && instance.suspense) + queuePostRenderEffect( + effect.run.bind(effect), + instance && instance.suspense + ) } else { - runner.run() + effect.run() } return () => { - runner.stop() + effect.stop() if (instance) { - remove(instance.effects!, runner) + remove(instance.effects!, effect) } } } diff --git a/packages/runtime-core/src/component.ts b/packages/runtime-core/src/component.ts index b2d1ca1052b..c1af3b02b30 100644 --- a/packages/runtime-core/src/component.ts +++ b/packages/runtime-core/src/component.ts @@ -4,8 +4,7 @@ import { pauseTracking, resetTracking, shallowReadonly, - proxyRefs, - ReactiveEffectRunner + proxyRefs } from '@vue/reactivity' import { ComponentPublicInstance, @@ -58,6 +57,7 @@ import { currentRenderingInstance } from './componentRenderContext' import { startMeasure, endMeasure } from './profiling' import { convertLegacyRenderFn } from './compat/renderFn' import { globalCompatConfig, validateCompatConfig } from './compat/compatConfig' +import { SchedulerJob } from './scheduler' export type Data = Record @@ -216,9 +216,14 @@ export interface ComponentInternalInstance { */ subTree: VNode /** - * Bound effect runner + * Main update effect + * @internal + */ + effect: ReactiveEffect + /** + * Bound effect runner to be passed to schedulers */ - update: ReactiveEffectRunner + update: SchedulerJob /** * The render function that returns vdom tree. * @internal @@ -443,6 +448,7 @@ export function createComponentInstance( root: null!, // to be immediately set next: null, subTree: null!, // will be set synchronously right after creation + effect: null!, // will be set synchronously right after creation update: null!, // will be set synchronously right after creation render: null, proxy: null, diff --git a/packages/runtime-core/src/renderer.ts b/packages/runtime-core/src/renderer.ts index 707b75ddc54..b03beca5407 100644 --- a/packages/runtime-core/src/renderer.ts +++ b/packages/runtime-core/src/renderer.ts @@ -1621,15 +1621,17 @@ function baseCreateRenderer( } // create reactive effect for rendering - const effect = new ReactiveEffect( + const effect = (instance.effect = new ReactiveEffect( componentUpdateFn, - queueJob, - // allowRecurse - // #1801, #2043 component render effects should allow recursive updates - true - ) + () => queueJob(instance.update), + true /* allowRecurse */ + )) - instance.update = effect.boundRun + const update = (instance.update = effect.run.bind(effect) as SchedulerJob) + update.id = instance.uid + // allowRecurse + // #1801, #2043 component render effects should allow recursive updates + update.allowRecurse = true if (__DEV__) { effect.onTrack = instance.rtc @@ -1639,10 +1641,10 @@ function baseCreateRenderer( ? e => invokeArrayFns(instance.rtg!, e) : void 0 // @ts-ignore (for scheduler) - instance.update.ownerInstance = instance + update.ownerInstance = instance } - instance.update() + update() } const updateComponentPreRender = ( @@ -2282,7 +2284,7 @@ function baseCreateRenderer( unregisterHMR(instance) } - const { bum, effects, update, subTree, um } = instance + const { bum, effect, effects, update, subTree, um } = instance // beforeUnmount hook if (bum) { @@ -2302,8 +2304,10 @@ function baseCreateRenderer( } // update may be null if a component is unmounted before its async // setup has resolved. - if (update) { - update.effect.stop() + if (effect) { + effect.stop() + // so that scheduler will no longer invoke it + update.active = false unmount(subTree, instance, parentSuspense, doRemove) } // unmounted hook diff --git a/packages/runtime-core/src/scheduler.ts b/packages/runtime-core/src/scheduler.ts index 510bfc5d1cd..20b9f4cf8e4 100644 --- a/packages/runtime-core/src/scheduler.ts +++ b/packages/runtime-core/src/scheduler.ts @@ -3,10 +3,10 @@ import { isArray } from '@vue/shared' import { ComponentPublicInstance } from './componentPublicInstance' import { ComponentInternalInstance, getComponentName } from './component' import { warn } from './warning' -import { ReactiveEffectRunner } from '@vue/reactivity' -export interface SchedulerJob extends Function, Partial { +export interface SchedulerJob extends Function { id?: number + active?: boolean allowRecurse?: boolean /** * Attached by renderer.ts when setting up a component's render effect From 45e246494a504034325ab200e3ff06019df27cd2 Mon Sep 17 00:00:00 2001 From: Evan You Date: Thu, 24 Jun 2021 17:33:45 -0400 Subject: [PATCH 3/4] chore: move comments --- packages/reactivity/src/effect.ts | 16 +--------------- packages/runtime-core/src/scheduler.ts | 15 +++++++++++++++ 2 files changed, 16 insertions(+), 15 deletions(-) diff --git a/packages/reactivity/src/effect.ts b/packages/reactivity/src/effect.ts index bac2e6ea6b7..5f09b931560 100644 --- a/packages/reactivity/src/effect.ts +++ b/packages/reactivity/src/effect.ts @@ -43,21 +43,7 @@ export class ReactiveEffect { constructor( public fn: () => T, public scheduler: EffectScheduler | null = null, - /** - * Indicates whether the effect is allowed to recursively trigger itself - * when managed by the scheduler. - * - * By default, a job cannot trigger itself because some built-in method calls, - * e.g. Array.prototype.push actually performs reads as well (#1740) which - * can lead to confusing infinite loops. - * The allowed cases are component update functions and watch callbacks. - * Component update functions may update child component props, which in turn - * trigger flush: "pre" watch callbacks that mutates state that the parent - * relies on (#1801). Watch callbacks doesn't track its dependencies so if it - * triggers itself again, it's likely intentional and it is the user's - * responsibility to perform recursive state mutation that eventually - * stabilizes (#1727). - */ + // allow recursive self-invocation public allowRecurse = false ) {} diff --git a/packages/runtime-core/src/scheduler.ts b/packages/runtime-core/src/scheduler.ts index 20b9f4cf8e4..9c50040e549 100644 --- a/packages/runtime-core/src/scheduler.ts +++ b/packages/runtime-core/src/scheduler.ts @@ -7,6 +7,21 @@ import { warn } from './warning' export interface SchedulerJob extends Function { id?: number active?: boolean + /** + * Indicates whether the effect is allowed to recursively trigger itself + * when managed by the scheduler. + * + * By default, a job cannot trigger itself because some built-in method calls, + * e.g. Array.prototype.push actually performs reads as well (#1740) which + * can lead to confusing infinite loops. + * The allowed cases are component update functions and watch callbacks. + * Component update functions may update child component props, which in turn + * trigger flush: "pre" watch callbacks that mutates state that the parent + * relies on (#1801). Watch callbacks doesn't track its dependencies so if it + * triggers itself again, it's likely intentional and it is the user's + * responsibility to perform recursive state mutation that eventually + * stabilizes (#1727). + */ allowRecurse?: boolean /** * Attached by renderer.ts when setting up a component's render effect From 26f6265e6d9263a9dc95380ea9f19843f67c2d7c Mon Sep 17 00:00:00 2001 From: Evan You Date: Thu, 24 Jun 2021 17:38:34 -0400 Subject: [PATCH 4/4] test: fix edge case --- packages/reactivity/__tests__/effect.spec.ts | 12 ++++++------ packages/reactivity/src/effect.ts | 4 ++++ 2 files changed, 10 insertions(+), 6 deletions(-) diff --git a/packages/reactivity/__tests__/effect.spec.ts b/packages/reactivity/__tests__/effect.spec.ts index 3e19d37ebb5..d458a17d840 100644 --- a/packages/reactivity/__tests__/effect.spec.ts +++ b/packages/reactivity/__tests__/effect.spec.ts @@ -490,12 +490,12 @@ describe('reactivity/effect', () => { expect(conditionalSpy).toHaveBeenCalledTimes(2) }) - // it('should not double wrap if the passed function is a effect', () => { - // const runner = effect(() => {}) - // const otherRunner = effect(runner) - // expect(runner).not.toBe(otherRunner) - // expect(runner.raw).toBe(otherRunner.raw) - // }) + it('should not double wrap if the passed function is a effect', () => { + const runner = effect(() => {}) + const otherRunner = effect(runner) + expect(runner).not.toBe(otherRunner) + expect(runner.effect.fn).toBe(otherRunner.effect.fn) + }) it('should not run multiple times for a single mutation', () => { let dummy diff --git a/packages/reactivity/src/effect.ts b/packages/reactivity/src/effect.ts index 5f09b931560..d2733e93cd6 100644 --- a/packages/reactivity/src/effect.ts +++ b/packages/reactivity/src/effect.ts @@ -105,6 +105,10 @@ export function effect( fn: () => T, options?: ReactiveEffectOptions ): ReactiveEffectRunner { + if ((fn as ReactiveEffectRunner).effect) { + fn = (fn as ReactiveEffectRunner).effect.fn + } + const _effect = new ReactiveEffect(fn) if (options) { extend(_effect, options)