Skip to content

refactor(reactivity, runtime-core): improve performance-critical code #13274

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: vapor
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 3 additions & 7 deletions packages/reactivity/__tests__/computed.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ import {
} from '../src'
import type { ComputedRef, ComputedRefImpl } from '../src/computed'
import { pauseTracking, resetTracking } from '../src/effect'
import { SubscriberFlags } from '../src/system'
import { ReactiveFlags } from '../src/system'

describe('reactivity/computed', () => {
it('should return updated value', () => {
Expand Down Expand Up @@ -467,12 +467,8 @@ describe('reactivity/computed', () => {
const c2 = computed(() => c1.value) as unknown as ComputedRefImpl

c2.value
expect(
c1.flags & (SubscriberFlags.Dirty | SubscriberFlags.PendingComputed),
).toBe(0)
expect(
c2.flags & (SubscriberFlags.Dirty | SubscriberFlags.PendingComputed),
).toBe(0)
expect(c1.flags & (ReactiveFlags.Dirty | ReactiveFlags.Pending)).toBe(0)
expect(c2.flags & (ReactiveFlags.Dirty | ReactiveFlags.Pending)).toBe(0)
})

it('should chained computeds dirtyLevel update with first computed effect', () => {
Expand Down
4 changes: 2 additions & 2 deletions packages/reactivity/__tests__/effect.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ import {
stop,
toRaw,
} from '../src/index'
import { type Dependency, endBatch, startBatch } from '../src/system'
import { type ReactiveNode, endBatch, startBatch } from '../src/system'

describe('reactivity/effect', () => {
it('should run the passed function once (wrapped by a effect)', () => {
Expand Down Expand Up @@ -1178,7 +1178,7 @@ describe('reactivity/effect', () => {
})

describe('dep unsubscribe', () => {
function getSubCount(dep: Dependency | undefined) {
function getSubCount(dep: ReactiveNode | undefined) {
let count = 0
let sub = dep!.subs
while (sub) {
Expand Down
66 changes: 32 additions & 34 deletions packages/reactivity/__tests__/effectScope.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,13 +2,15 @@ import { nextTick, watch, watchEffect } from '@vue/runtime-core'
import {
type ComputedRef,
EffectScope,
ReactiveEffect,
computed,
effect,
effectScope,
getCurrentScope,
onScopeDispose,
reactive,
ref,
setCurrentScope,
} from '../src'

describe('reactivity/effect/scope', () => {
Expand All @@ -20,7 +22,7 @@ describe('reactivity/effect/scope', () => {

it('should accept zero argument', () => {
const scope = effectScope()
expect(scope.effects.length).toBe(0)
expect(getEffectsCount(scope)).toBe(0)
})

it('should return run value', () => {
Expand All @@ -29,7 +31,8 @@ describe('reactivity/effect/scope', () => {

it('should work w/ active property', () => {
const scope = effectScope()
scope.run(() => 1)
const src = computed(() => 1)
scope.run(() => src.value)
expect(scope.active).toBe(true)
scope.stop()
expect(scope.active).toBe(false)
Expand All @@ -47,7 +50,7 @@ describe('reactivity/effect/scope', () => {
expect(dummy).toBe(7)
})

expect(scope.effects.length).toBe(1)
expect(getEffectsCount(scope)).toBe(1)
})

it('stop', () => {
Expand All @@ -60,7 +63,7 @@ describe('reactivity/effect/scope', () => {
effect(() => (doubled = counter.num * 2))
})

expect(scope.effects.length).toBe(2)
expect(getEffectsCount(scope)).toBe(2)

expect(dummy).toBe(0)
counter.num = 7
Expand All @@ -87,9 +90,8 @@ describe('reactivity/effect/scope', () => {
})
})

expect(scope.effects.length).toBe(1)
expect(scope.scopes!.length).toBe(1)
expect(scope.scopes![0]).toBeInstanceOf(EffectScope)
expect(getEffectsCount(scope)).toBe(1)
expect(scope.deps?.nextDep?.dep).toBeInstanceOf(EffectScope)

expect(dummy).toBe(0)
counter.num = 7
Expand Down Expand Up @@ -117,7 +119,7 @@ describe('reactivity/effect/scope', () => {
})
})

expect(scope.effects.length).toBe(1)
expect(getEffectsCount(scope)).toBe(1)

expect(dummy).toBe(0)
counter.num = 7
Expand All @@ -142,13 +144,13 @@ describe('reactivity/effect/scope', () => {
effect(() => (dummy = counter.num))
})

expect(scope.effects.length).toBe(1)
expect(getEffectsCount(scope)).toBe(1)

scope.run(() => {
effect(() => (doubled = counter.num * 2))
})

expect(scope.effects.length).toBe(2)
expect(getEffectsCount(scope)).toBe(2)

counter.num = 7
expect(dummy).toBe(7)
Expand All @@ -166,21 +168,21 @@ describe('reactivity/effect/scope', () => {
effect(() => (dummy = counter.num))
})

expect(scope.effects.length).toBe(1)
expect(getEffectsCount(scope)).toBe(1)

scope.stop()

expect(getEffectsCount(scope)).toBe(0)

scope.run(() => {
effect(() => (doubled = counter.num * 2))
})

expect('[Vue warn] cannot run an inactive effect scope.').toHaveBeenWarned()

expect(scope.effects.length).toBe(0)
expect(getEffectsCount(scope)).toBe(1)

counter.num = 7
expect(dummy).toBe(0)
expect(doubled).toBe(undefined)
expect(doubled).toBe(14)
})

it('should fire onScopeDispose hook', () => {
Expand Down Expand Up @@ -224,9 +226,9 @@ describe('reactivity/effect/scope', () => {
it('should dereference child scope from parent scope after stopping child scope (no memleaks)', () => {
const parent = effectScope()
const child = parent.run(() => effectScope())!
expect(parent.scopes!.includes(child)).toBe(true)
expect(parent.deps?.dep).toBe(child)
child.stop()
expect(parent.scopes!.includes(child)).toBe(false)
expect(parent.deps).toBeUndefined()
})

it('test with higher level APIs', async () => {
Expand Down Expand Up @@ -290,21 +292,7 @@ describe('reactivity/effect/scope', () => {

parentScope.run(() => {
const childScope = effectScope(true)
childScope.on()
childScope.off()
expect(getCurrentScope()).toBe(parentScope)
})
})

it('calling on() and off() multiple times inside an active scope should not break currentScope', () => {
const parentScope = effectScope()
parentScope.run(() => {
const childScope = effectScope(true)
childScope.on()
childScope.on()
childScope.off()
childScope.off()
childScope.off()
setCurrentScope(setCurrentScope(childScope))
expect(getCurrentScope()).toBe(parentScope)
})
})
Expand Down Expand Up @@ -372,7 +360,17 @@ describe('reactivity/effect/scope', () => {
expect(watcherCalls).toBe(3)
expect(cleanupCalls).toBe(1)

expect(scope.effects.length).toBe(0)
expect(scope.cleanups.length).toBe(0)
expect(getEffectsCount(scope)).toBe(0)
expect(scope.cleanupsLength).toBe(0)
})
})

function getEffectsCount(scope: EffectScope): number {
let n = 0
for (let dep = scope.deps; dep !== undefined; dep = dep.nextDep) {
if (dep.dep instanceof ReactiveEffect) {
n++
}
}
return n
}
76 changes: 0 additions & 76 deletions packages/reactivity/__tests__/watch.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,40 +3,12 @@ import {
type Ref,
WatchErrorCodes,
type WatchOptions,
type WatchScheduler,
computed,
onWatcherCleanup,
ref,
watch,
} from '../src'

const queue: (() => void)[] = []

// a simple scheduler for testing purposes
let isFlushPending = false
const resolvedPromise = /*@__PURE__*/ Promise.resolve() as Promise<any>
const nextTick = (fn?: () => any) =>
fn ? resolvedPromise.then(fn) : resolvedPromise

const scheduler: WatchScheduler = (job, isFirstRun) => {
if (isFirstRun) {
job()
} else {
queue.push(job)
flushJobs()
}
}

const flushJobs = () => {
if (isFlushPending) return
isFlushPending = true
resolvedPromise.then(() => {
queue.forEach(job => job())
queue.length = 0
isFlushPending = false
})
}

describe('watch', () => {
test('effect', () => {
let dummy: any
Expand Down Expand Up @@ -147,54 +119,6 @@ describe('watch', () => {
expect(dummy).toBe(30)
})

test('nested calls to baseWatch and onWatcherCleanup', async () => {
let calls: string[] = []
let source: Ref<number>
let copyist: Ref<number>
const scope = new EffectScope()

scope.run(() => {
source = ref(0)
copyist = ref(0)
// sync by default
watch(
() => {
const current = (copyist.value = source.value)
onWatcherCleanup(() => calls.push(`sync ${current}`))
},
null,
{},
)
// with scheduler
watch(
() => {
const current = copyist.value
onWatcherCleanup(() => calls.push(`post ${current}`))
},
null,
{ scheduler },
)
})

await nextTick()
expect(calls).toEqual([])

scope.run(() => source.value++)
expect(calls).toEqual(['sync 0'])
await nextTick()
expect(calls).toEqual(['sync 0', 'post 0'])
calls.length = 0

scope.run(() => source.value++)
expect(calls).toEqual(['sync 1'])
await nextTick()
expect(calls).toEqual(['sync 1', 'post 1'])
calls.length = 0

scope.stop()
expect(calls).toEqual(['sync 2', 'post 2'])
})

test('once option should be ignored by simple watch', async () => {
let dummy: any
const source = ref(0)
Expand Down
7 changes: 3 additions & 4 deletions packages/reactivity/src/arrayInstrumentations.ts
Original file line number Diff line number Diff line change
@@ -1,9 +1,8 @@
import { isArray } from '@vue/shared'
import { TrackOpTypes } from './constants'
import { ARRAY_ITERATE_KEY, track } from './dep'
import { pauseTracking, resetTracking } from './effect'
import { isProxy, isShallow, toRaw, toReactive } from './reactive'
import { endBatch, startBatch } from './system'
import { endBatch, setActiveSub, startBatch } from './system'

/**
* Track array iteration and return:
Expand Down Expand Up @@ -320,10 +319,10 @@ function noTracking(
method: keyof Array<any>,
args: unknown[] = [],
) {
pauseTracking()
startBatch()
const prevSub = setActiveSub()
const res = (toRaw(self) as any)[method].apply(self, args)
setActiveSub(prevSub)
endBatch()
resetTracking()
return res
}
Loading