-
Notifications
You must be signed in to change notification settings - Fork 21
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
Performance: Limit data layer state updates #724
Conversation
const baseStateUpdate = this.state.data ? this.state.data : { ...emptyPanelData, timeRange: timeRange.state.value }; | ||
// Skip unnessary state updates | ||
if ( | ||
alertState === this.state.data?.alertState && |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This condition is quite hard to reason about, as it assumes that one knows that the alert state is a reference to an state taken from the alert states results. I wonder if it wouldn't be easier if we didn't set that particular alert state reference in the state but always create a new object, and here instead rely on object equality rather than reference equalit? WDYT?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@dprokop pushed update that switched to isEqual instead, for some reason I thought alertState was an enum :)
…tes-performance-updates
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Gave it another look (thanks for updating the alerts state quality check!) and it looks good. I've also tested some test dashboards with annotations and it works great.
🚀 PR was released in |
Various performance fixes
📦 Published PR as canary version:
4.22.0--canary.724.9079775010.0
✨ Test out this PR locally via:
npm install @grafana/scenes@4.22.0--canary.724.9079775010.0 # or yarn add @grafana/scenes@4.22.0--canary.724.9079775010.0