Skip to content

Commit 47ba7ab

Browse files
authored
perf(dashboards): Fix slow re-render of ECharts objects (#103375)
This PR is part of the effort to improve how fast the Widget Builder opens and runs. One of the reasons why it's slow is that the `Dashboard` component (i.e., the literal dashboard and all its charts) underneath the builder re-renders when it opens. This takes a long time because each ECharts object takes ~50ms to re-draw. You can see this phase in the screenshot, there are 5 charts on that page, so ~250ms delay. <img width="524" height="316" alt="Screenshot 2025-11-14 at 11 19 10 AM" src="https://github.com/user-attachments/assets/3dfc4b7f-492c-400c-bc29-0ac692b36842" /> On re-render, `echarts-for-react` calls `componentDidUpdate` and in there, `echarts.setOption` which re-draws the chart. Drawing is expensive because ECharts needs to measure the DOM a lot (to calculate overlap between labels) and because it might be calling a bunch of `requestAnimationFrame` to run animations from ZRender. <img width="1173" height="384" alt="Screenshot 2025-11-14 at 11 20 53 AM" src="https://github.com/user-attachments/assets/b907b040-e8b0-47e2-85aa-9b97af9aa15f" /> This PR makes two major changes: 1. Turn animations off completely on Dashboards charts. We don't have any animations configured, but even still ZRender will call `requestAnimationFrame` to schedule animations sometimes. Not a major win, but still a win 2. Allow ECharts to merge chart options. In ECharts, calling `setOption`, by default, will compare the objects and only update and re-draw things that changed. This is _much_ more efficient than re-drawing (see above). In `BaseChart`, we've set `notMerge` to `true` by default, which forces ECharts to re-draw! I've set `notMerge` to `false` for Dashboards, so ECharts will merge the options There's no "After" profile, because that section disappears completely. TADA, a huge amount of work gone. I'm not comfortable turning off `notMerge` for all charts because in some cases it _might_ cause UI problems, but in Dashboards this seems to work well. I thought for a long time about doing some hardcore memoizing of the props to ECharts, but that's too brittle and annoying, and there are too many contexts that cause hard to avoid re-renders.
1 parent a358900 commit 47ba7ab

File tree

3 files changed

+17
-2
lines changed

3 files changed

+17
-2
lines changed

static/app/components/charts/barChart.tsx

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -81,6 +81,12 @@ export function BarChart({
8181
}, [xAxis]);
8282

8383
return (
84-
<BaseChart {...props} ref={ref} xAxis={xAxisOptions} series={transformedSeries} />
84+
<BaseChart
85+
{...props}
86+
animation={animation}
87+
ref={ref}
88+
xAxis={xAxisOptions}
89+
series={transformedSeries}
90+
/>
8591
);
8692
}

static/app/components/charts/baseChart.tsx

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -135,6 +135,10 @@ export interface BaseChartProps {
135135
* This is to pass series to BaseChart bypassing the wrappers like LineChart, AreaChart etc.
136136
*/
137137
additionalSeries?: SeriesOption[];
138+
/**
139+
* Whether animations are enabled for the entire chart. If animations are enabled overall, this will cause ZRender to occasionally attempt to run animations and call `requestAnimationFrame` which might cause UI stutter.
140+
*/
141+
animation?: boolean;
138142
/**
139143
* If true, ignores height value and auto-scales chart to fit container height.
140144
*/
@@ -332,6 +336,7 @@ const DEFAULT_Y_AXIS = {};
332336
const DEFAULT_X_AXIS = {};
333337

334338
function BaseChart({
339+
animation,
335340
brush,
336341
colors,
337342
grid,
@@ -564,6 +569,7 @@ function BaseChart({
564569

565570
return {
566571
...options,
572+
animation,
567573
useUTC: utc,
568574
color: color as string[],
569575
grid: Array.isArray(grid) ? grid.map(Grid) : Grid(grid),
@@ -580,6 +586,7 @@ function BaseChart({
580586
brush,
581587
};
582588
}, [
589+
animation,
583590
chartId,
584591
color,
585592
resolvedSeries,

static/app/views/dashboards/widgetCard/chart.tsx

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -303,6 +303,8 @@ function WidgetCardChart(props: WidgetCardChartProps) {
303303
};
304304

305305
const chartOptions = {
306+
animation: false, // Turn off all chart animations. This turns off all ZRender hooks that might `requestAnimationFrame`
307+
notMerge: false, // Enable ECharts option merging. Chart components are only re-drawn if they've changed
306308
autoHeightResize: shouldResize ?? true,
307309
useMultilineDate: true,
308310
grid: {
@@ -661,7 +663,7 @@ function getChartComponent(chartProps: any, widget: Widget): React.ReactNode {
661663

662664
switch (widget.displayType) {
663665
case 'bar':
664-
return <BarChart {...chartProps} stacked={stacked} animation={false} />;
666+
return <BarChart {...chartProps} stacked={stacked} />;
665667
case 'area':
666668
case 'top_n':
667669
return <AreaChart stacked {...chartProps} />;

0 commit comments

Comments
 (0)