Skip to content

Commit

Permalink
Revert "perf: avoid unnecessary re-renders when computed property val…
Browse files Browse the repository at this point in the history
…ue did not change (#7824)"

This reverts commit 653aac2.
  • Loading branch information
yyx990803 committed Sep 18, 2018
1 parent 2ae4e29 commit 6b1d431
Show file tree
Hide file tree
Showing 4 changed files with 51 additions and 153 deletions.
13 changes: 9 additions & 4 deletions src/core/instance/state.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@

import config from '../config'
import Watcher from '../observer/watcher'
import { pushTarget, popTarget } from '../observer/dep'
import Dep, { pushTarget, popTarget } from '../observer/dep'
import { isUpdatingChildComponent } from './lifecycle'

import {
Expand Down Expand Up @@ -164,7 +164,7 @@ export function getData (data: Function, vm: Component): any {
}
}

const computedWatcherOptions = { computed: true }
const computedWatcherOptions = { lazy: true }

function initComputed (vm: Component, computed: Object) {
// $flow-disable-line
Expand Down Expand Up @@ -244,8 +244,13 @@ function createComputedGetter (key) {
return function computedGetter () {
const watcher = this._computedWatchers && this._computedWatchers[key]
if (watcher) {
watcher.depend()
return watcher.evaluate()
if (watcher.dirty) {
watcher.evaluate()
}
if (Dep.target) {
watcher.depend()
}
return watcher.value
}
}
}
Expand Down
101 changes: 37 additions & 64 deletions src/core/observer/watcher.js
Original file line number Diff line number Diff line change
Expand Up @@ -29,11 +29,10 @@ export default class Watcher {
id: number;
deep: boolean;
user: boolean;
computed: boolean;
lazy: boolean;
sync: boolean;
dirty: boolean;
active: boolean;
dep: Dep;
deps: Array<Dep>;
newDeps: Array<Dep>;
depIds: SimpleSet;
Expand All @@ -58,16 +57,16 @@ export default class Watcher {
if (options) {
this.deep = !!options.deep
this.user = !!options.user
this.computed = !!options.computed
this.lazy = !!options.lazy
this.sync = !!options.sync
this.before = options.before
} else {
this.deep = this.user = this.computed = this.sync = false
this.deep = this.user = this.lazy = this.sync = false
}
this.cb = cb
this.id = ++uid // uid for batching
this.active = true
this.dirty = this.computed // for computed watchers
this.dirty = this.lazy // for lazy watchers
this.deps = []
this.newDeps = []
this.depIds = new Set()
Expand All @@ -90,12 +89,9 @@ export default class Watcher {
)
}
}
if (this.computed) {
this.value = undefined
this.dep = new Dep()
} else {
this.value = this.get()
}
this.value = this.lazy
? undefined
: this.get()
}

/**
Expand Down Expand Up @@ -166,24 +162,8 @@ export default class Watcher {
*/
update () {
/* istanbul ignore else */
if (this.computed) {
// A computed property watcher has two modes: lazy and activated.
// It initializes as lazy by default, and only becomes activated when
// it is depended on by at least one subscriber, which is typically
// another computed property or a component's render function.
if (this.dep.subs.length === 0) {
// In lazy mode, we don't want to perform computations until necessary,
// so we simply mark the watcher as dirty. The actual computation is
// performed just-in-time in this.evaluate() when the computed property
// is accessed.
this.dirty = true
} else {
// In activated mode, we want to proactively perform the computation
// but only notify our subscribers when the value has indeed changed.
this.getAndInvoke(() => {
this.dep.notify()
})
}
if (this.lazy) {

This comment has been minimized.

Copy link
@LeeYunhang

LeeYunhang Nov 19, 2018

Hi, Evan You:
I want to know how does computed property notify observers. It seems hasn't any function call. Thank you. 👍

this.dirty = true
} else if (this.sync) {
this.run()
} else {
Expand All @@ -197,54 +177,47 @@ export default class Watcher {
*/
run () {
if (this.active) {
this.getAndInvoke(this.cb)
}
}

getAndInvoke (cb: Function) {
const value = this.get()
if (
value !== this.value ||
// Deep watchers and watchers on Object/Arrays should fire even
// when the value is the same, because the value may
// have mutated.
isObject(value) ||
this.deep
) {
// set new value
const oldValue = this.value
this.value = value
this.dirty = false
if (this.user) {
try {
cb.call(this.vm, value, oldValue)
} catch (e) {
handleError(e, this.vm, `callback for watcher "${this.expression}"`)
const value = this.get()
if (
value !== this.value ||
// Deep watchers and watchers on Object/Arrays should fire even
// when the value is the same, because the value may
// have mutated.
isObject(value) ||
this.deep
) {
// set new value
const oldValue = this.value
this.value = value
if (this.user) {
try {
this.cb.call(this.vm, value, oldValue)
} catch (e) {
handleError(e, this.vm, `callback for watcher "${this.expression}"`)
}
} else {
this.cb.call(this.vm, value, oldValue)
}
} else {
cb.call(this.vm, value, oldValue)
}
}
}

/**
* Evaluate and return the value of the watcher.
* This only gets called for computed property watchers.
* Evaluate the value of the watcher.
* This only gets called for lazy watchers.
*/
evaluate () {
if (this.dirty) {
this.value = this.get()
this.dirty = false
}
return this.value
this.value = this.get()
this.dirty = false
}

/**
* Depend on this watcher. Only for computed property watchers.
* Depend on all deps collected by this watcher.
*/
depend () {
if (this.dep && Dep.target) {
this.dep.depend()
let i = this.deps.length
while (i--) {
this.deps[i].depend()
}
}

Expand Down
36 changes: 0 additions & 36 deletions test/unit/features/options/computed.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -216,40 +216,4 @@ describe('Options computed', () => {
})
expect(() => vm.a).toThrowError('rethrow')
})

// #7767
it('should avoid unnecessary re-renders', done => {
const computedSpy = jasmine.createSpy('computed')
const updatedSpy = jasmine.createSpy('updated')
const vm = new Vue({
data: {
msg: 'bar'
},
computed: {
a () {
computedSpy()
return this.msg !== 'foo'
}
},
template: `<div>{{ a }}</div>`,
updated: updatedSpy
}).$mount()

expect(vm.$el.textContent).toBe('true')
expect(computedSpy.calls.count()).toBe(1)
expect(updatedSpy.calls.count()).toBe(0)

vm.msg = 'baz'
waitForUpdate(() => {
expect(vm.$el.textContent).toBe('true')
expect(computedSpy.calls.count()).toBe(2)
expect(updatedSpy.calls.count()).toBe(0)
}).then(() => {
vm.msg = 'foo'
}).then(() => {
expect(vm.$el.textContent).toBe('false')
expect(computedSpy.calls.count()).toBe(3)
expect(updatedSpy.calls.count()).toBe(1)
}).then(done)
})
})
54 changes: 5 additions & 49 deletions test/unit/modules/observer/watcher.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -144,70 +144,26 @@ describe('Watcher', () => {
}).then(done)
})

it('computed mode, lazy', done => {
let getterCallCount = 0
it('lazy mode', done => {
const watcher = new Watcher(vm, function () {
getterCallCount++
return this.a + this.b.d
}, null, { computed: true })

expect(getterCallCount).toBe(0)
expect(watcher.computed).toBe(true)
}, null, { lazy: true })
expect(watcher.lazy).toBe(true)
expect(watcher.value).toBeUndefined()
expect(watcher.dirty).toBe(true)
expect(watcher.dep).toBeTruthy()

const value = watcher.evaluate()
expect(getterCallCount).toBe(1)
expect(value).toBe(5)
watcher.evaluate()
expect(watcher.value).toBe(5)
expect(watcher.dirty).toBe(false)

// should not get again if not dirty
watcher.evaluate()
expect(getterCallCount).toBe(1)

vm.a = 2
waitForUpdate(() => {
expect(getterCallCount).toBe(1)
expect(watcher.value).toBe(5)
expect(watcher.dirty).toBe(true)

const value = watcher.evaluate()
expect(getterCallCount).toBe(2)
expect(value).toBe(6)
watcher.evaluate()
expect(watcher.value).toBe(6)
expect(watcher.dirty).toBe(false)
}).then(done)
})

it('computed mode, activated', done => {
let getterCallCount = 0
const watcher = new Watcher(vm, function () {
getterCallCount++
return this.a + this.b.d
}, null, { computed: true })

// activate by mocking a subscriber
const subMock = jasmine.createSpyObj('sub', ['update'])
watcher.dep.addSub(subMock)

const value = watcher.evaluate()
expect(getterCallCount).toBe(1)
expect(value).toBe(5)

vm.a = 2
waitForUpdate(() => {
expect(getterCallCount).toBe(2)
expect(subMock.update).toHaveBeenCalled()

// since already computed, calling evaluate again should not trigger
// getter
watcher.evaluate()
expect(getterCallCount).toBe(2)
}).then(done)
})

it('teardown', done => {
const watcher = new Watcher(vm, 'b.c', spy)
watcher.teardown()
Expand Down

2 comments on commit 6b1d431

@LightBoom
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi, Evan You:
Why revert,can you explain it. It seams that the feature can be useful in avoiding unnecessary re-renders. Thank you.

@godky
Copy link

@godky godky commented on 6b1d431 Apr 18, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi, Evan You:
Why revert,can you explain it. It seams that the feature can be useful in avoiding unnecessary re-renders. Thank you.

There is the answer in #8446

Please sign in to comment.