Enable downsampling in echarts #24311
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
bramkragten
reviewed
Feb 19, 2025
| @@ -492,8 +492,8 @@ export class StatisticsChart extends LitElement { | |||
| : this.hass.localize( | |||
| `ui.components.statistics_charts.statistic_types.${type}` | |||
| ), | |||
| symbol: "circle", | |||
| symbolSize: 0, | |||
| symbol: "none", | |||
There was a problem hiding this comment.
So stats chart points can not be highlighted anymore now?
There was a problem hiding this comment.
Yes? Although the size is set to 0... But then we have this code that sets it to none for bands, that would not be needed anymore now then?
There was a problem hiding this comment.
Indeed we can remove this or enable points again for stats.
symbolSize: 0 means they haven't had points since the migration and no one has complained so I don't think we need points for stats. Also the smoothing and the nature of the data makes it odd to have points
|
I tested it on desktop chrome & firefox and on iOS. I really feel the difference its so smooth now 🎉 |
bramkragten
approved these changes
Feb 19, 2025
bramkragten
pushed a commit
that referenced
this pull request
Feb 21, 2025
* Enable downsampling in echarts * remove unneeded symbol set
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Proposed change
Was finally able to look into what Chartjs did differently before. As I suspected, they have downsampling by default while in echarts it is disabled by default. This is the main reason for the performance difference.
There are a few sampling algorithms to choose from.
minmaxlooks good to me butlttbis also good.This change makes echarts 2x faster than ChartJS on my test data. 7 charts on the same page, 5 of them in same card, 1 chart with 85k datapoints.
Updates take
ChartsJS:
150msecharts:
70msdown from300msPerformance can be further improved by setting
symbol: "none"for history charts but then points can't be highlighted.Type of change
Example configuration
Additional information
Checklist
If user exposed functionality or configuration variables are added/changed: