Skip to content
This repository has been archived by the owner on Dec 11, 2019. It is now read-only.

appStore calls diff on the state without first checking to see if there are changes #10195

Closed
bridiver opened this issue Jul 28, 2017 · 2 comments

Comments

@bridiver
Copy link
Collaborator

bridiver commented Jul 28, 2017

Test plan

#10498 (comment)


diff is a relatively expensive operation on the state and we should short-circuit emitChanges if lastEmittedState === appState

@bridiver bridiver added the perf label Jul 28, 2017
@NejcZdovc NejcZdovc added the hackathon Legacy label for a hackaton. label Jul 29, 2017
@NejcZdovc NejcZdovc self-assigned this Aug 15, 2017
@NejcZdovc NejcZdovc added this to the 0.21.x (Nightly Channel) milestone Aug 15, 2017
@NejcZdovc
Copy link
Contributor

@bbondy I putted this into 0.21, but please move it into earlier branches if needed.

NejcZdovc added a commit to NejcZdovc/browser-laptop that referenced this issue Aug 15, 2017
Resolves brave#10195

Auditors: @bridiver @bbondy

Test Plan:
- open two windows
- add bookmark in one
- bookmark should appear in both bookmark toolbars
@NejcZdovc NejcZdovc modified the milestones: 0.19.x (Beta Channel), 0.21.x (Nightly Channel) Aug 15, 2017
NejcZdovc added a commit that referenced this issue Aug 15, 2017
First quick check if lastEmittedState and appState are different
NejcZdovc added a commit that referenced this issue Aug 15, 2017
Resolves #10195

Auditors: @bridiver @bbondy

Test Plan:
- open two windows
- add bookmark in one
- bookmark should appear in both bookmark toolbars
NejcZdovc added a commit that referenced this issue Aug 15, 2017
Resolves #10195

Auditors: @bridiver @bbondy

Test Plan:
- open two windows
- add bookmark in one
- bookmark should appear in both bookmark toolbars
NejcZdovc added a commit that referenced this issue Aug 15, 2017
Resolves #10195

Auditors: @bridiver @bbondy

Test Plan:
- open two windows
- add bookmark in one
- bookmark should appear in both bookmark toolbars
@luixxiul
Copy link
Contributor

added release-notes/include since every improvement of perf is worth mentioning on the release note

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

5 participants