Skip to content

Commit

Permalink
perf: improve unsub perf for deps with massive amount of subs
Browse files Browse the repository at this point in the history
close #12696
  • Loading branch information
yyx990803 committed Oct 11, 2022
1 parent 738f4b3 commit 8880b55
Show file tree
Hide file tree
Showing 4 changed files with 43 additions and 11 deletions.
32 changes: 26 additions & 6 deletions src/core/observer/dep.ts
Original file line number Diff line number Diff line change
@@ -1,9 +1,19 @@
import { remove } from '../util/index'
import config from '../config'
import { DebuggerOptions, DebuggerEventExtraInfo } from 'v3'

let uid = 0

const pendingCleanupDeps: Dep[] = []

export const cleanupDeps = () => {
for (let i = 0; i < pendingCleanupDeps.length; i++) {
const dep = pendingCleanupDeps[i]
dep.subs = dep.subs.filter(s => s)
dep._pending = false
}
pendingCleanupDeps.length = 0
}

/**
* @internal
*/
Expand All @@ -21,7 +31,9 @@ export interface DepTarget extends DebuggerOptions {
export default class Dep {
static target?: DepTarget | null
id: number
subs: Array<DepTarget>
subs: Array<DepTarget | null>
// pending subs cleanup
_pending = false

constructor() {
this.id = uid++
Expand All @@ -33,7 +45,15 @@ export default class Dep {
}

removeSub(sub: DepTarget) {
remove(this.subs, sub)
// #12696 deps with massive amount of subscribers are extremely slow to
// clean up in Chromium
// to workaround this, we unset the sub for now, and clear them on
// next scheduler flush.
this.subs[this.subs.indexOf(sub)] = null
if (!this._pending) {
this._pending = true
pendingCleanupDeps.push(this)
}
}

depend(info?: DebuggerEventExtraInfo) {
Expand All @@ -50,23 +70,23 @@ export default class Dep {

notify(info?: DebuggerEventExtraInfo) {
// stabilize the subscriber list first
const subs = this.subs.slice()
const subs = this.subs.filter(s => s) as DepTarget[]
if (__DEV__ && !config.async) {
// subs aren't sorted in scheduler if not running async
// we need to sort them now to make sure they fire in correct
// order
subs.sort((a, b) => a.id - b.id)
}
for (let i = 0, l = subs.length; i < l; i++) {
const sub = subs[i]
if (__DEV__ && info) {
const sub = subs[i]
sub.onTrigger &&
sub.onTrigger({
effect: subs[i],
...info
})
}
subs[i].update()
sub.update()
}
}
}
Expand Down
3 changes: 2 additions & 1 deletion src/core/observer/scheduler.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import type Watcher from './watcher'
import config from '../config'
import Dep from './dep'
import Dep, { cleanupDeps } from './dep'
import { callHook, activateChildComponent } from '../instance/lifecycle'

import { warn, nextTick, devtools, inBrowser, isIE } from '../util/index'
Expand Down Expand Up @@ -121,6 +121,7 @@ function flushSchedulerQueue() {
// call component updated and activated hooks
callActivatedHooks(activatedQueue)
callUpdatedHooks(updatedQueue)
cleanupDeps()

// devtool hook
/* istanbul ignore if */
Expand Down
8 changes: 7 additions & 1 deletion src/shared/util.ts
Original file line number Diff line number Diff line change
Expand Up @@ -133,7 +133,13 @@ export const isReservedAttribute = makeMap('key,ref,slot,slot-scope,is')
* Remove an item from an array.
*/
export function remove(arr: Array<any>, item: any): Array<any> | void {
if (arr.length) {
const len = arr.length
if (len) {
// fast path for the only / last item
if (item === arr[len - 1]) {
arr.length = len - 1
return
}
const index = arr.indexOf(item)
if (index > -1) {
return arr.splice(index, 1)
Expand Down
11 changes: 8 additions & 3 deletions test/unit/modules/observer/dep.spec.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import Dep from 'core/observer/dep'
import Dep, { cleanupDeps } from 'core/observer/dep'

describe('Dep', () => {
let dep
Expand All @@ -24,8 +24,13 @@ describe('Dep', () => {

describe('removeSub()', () => {
it('should remove sub', () => {
dep.subs.push(null)
dep.removeSub(null)
const sub = {}
dep.subs.push(sub)
dep.removeSub(sub)
expect(dep.subs.includes(sub)).toBe(false)

// nulled subs are cleared on next flush
cleanupDeps()
expect(dep.subs.length).toBe(0)
})
})
Expand Down

0 comments on commit 8880b55

Please sign in to comment.