GraphNG: fix mem leaks & avoid plot re-inits (#38017)

* State timeline: fix mem leak caused by excessive plot re-init

* Update PlotTooltipInterpolator type

* Do not reference config object in the setCursor hook

* fix excessive BarChart re-init caused by shallow diff of text config

* one less error

Co-authored-by: Dominik Prokop <dominik.prokop@grafana.com>
Co-authored-by: Ryan McKinley <ryantxu@gmail.com>
This commit is contained in:
Leon Sorokin 2021-08-18 13:43:53 -05:00 committed by GitHub
parent 63a85a4ac0
commit 49b129b110
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
7 changed files with 86 additions and 53 deletions

View File

@ -27,6 +27,11 @@ import { UPlotChart } from '../uPlot/Plot';
*/
export const FIXED_UNIT = '__fixed';
/**
* @internal -- not a public API
*/
export type PropDiffFn<T extends any = any> = (prev: T, next: T) => boolean;
export interface GraphNGProps extends Themeable2 {
frames: DataFrame[];
structureRev?: number; // a number that will change when the frames[] structure changes
@ -39,14 +44,18 @@ export interface GraphNGProps extends Themeable2 {
onLegendClick?: (event: GraphNGLegendEvent) => void;
children?: (builder: UPlotConfigBuilder, alignedFrame: DataFrame) => React.ReactNode;
prepConfig: (alignedFrame: DataFrame, allFrames: DataFrame[], getTimeRange: () => TimeRange) => UPlotConfigBuilder;
propsToDiff?: string[];
propsToDiff?: Array<string | PropDiffFn>;
preparePlotFrame?: (frames: DataFrame[], dimFields: XYFieldMatchers) => DataFrame;
renderLegend: (config: UPlotConfigBuilder) => React.ReactElement | null;
}
function sameProps(prevProps: any, nextProps: any, propsToDiff: string[] = []) {
function sameProps(prevProps: any, nextProps: any, propsToDiff: Array<string | PropDiffFn> = []) {
for (const propName of propsToDiff) {
if (nextProps[propName] !== prevProps[propName]) {
if (typeof propName === 'function') {
if (!propName(prevProps, nextProps)) {
return false;
}
} else if (nextProps[propName] !== prevProps[propName]) {
return false;
}
}

View File

@ -49,11 +49,8 @@ export class UPlotConfigBuilder {
private frame: DataFrame | undefined = undefined;
// to prevent more than one threshold per scale
private thresholds: Record<string, UPlotThresholdOptions> = {};
/**
* Custom handler for closest datapoint and series lookup. Technicaly returns uPlots setCursor hook
* that sets tooltips state.
*/
tooltipInterpolator: PlotTooltipInterpolator | undefined = undefined;
// Custom handler for closest datapoint and series lookup
private tooltipInterpolator: PlotTooltipInterpolator | undefined = undefined;
prepData: PrepData | undefined = undefined;
@ -154,6 +151,10 @@ export class UPlotConfigBuilder {
this.tooltipInterpolator = interpolator;
}
getTooltipInterpolator() {
return this.tooltipInterpolator;
}
setPrepData(prepData: PrepData) {
this.prepData = (frame) => {
this.frame = frame;

View File

@ -93,25 +93,31 @@ export const TooltipPlugin: React.FC<TooltipPluginProps> = ({
// Add uPlot hooks to the config, or re-add when the config changed
useLayoutEffect(() => {
if (config.tooltipInterpolator) {
const tooltipInterpolator = config.getTooltipInterpolator();
if (tooltipInterpolator) {
// Custom toolitp positioning
config.addHook('setCursor', (u) => {
config.tooltipInterpolator!(setFocusedSeriesIdx, setFocusedPointIdx, (clear) => {
if (clear) {
setCoords(null);
return;
}
tooltipInterpolator(
setFocusedSeriesIdx,
setFocusedPointIdx,
(clear) => {
if (clear) {
setCoords(null);
return;
}
const bbox = plotCtx.getCanvasBoundingBox();
if (!bbox) {
return;
}
const bbox = plotCtx.getCanvasBoundingBox();
if (!bbox) {
return;
}
const { x, y } = positionTooltip(u, bbox);
if (x !== undefined && y !== undefined) {
setCoords({ x, y });
}
})(u);
const { x, y } = positionTooltip(u, bbox);
if (x !== undefined && y !== undefined) {
setCoords({ x, y });
}
},
u
);
});
} else {
config.addHook('setLegend', (u) => {

View File

@ -34,8 +34,9 @@ export abstract class PlotConfigBuilder<P, T> {
export type PlotTooltipInterpolator = (
updateActiveSeriesIdx: (sIdx: number | null) => void,
updateActiveDatapointIdx: (dIdx: number | null) => void,
updateTooltipPosition: (clear?: boolean) => void
) => (u: uPlot) => void;
updateTooltipPosition: (clear?: boolean) => void,
u: uPlot
) => void;
export interface PlotSelection {
min: number;

View File

@ -12,6 +12,7 @@ import {
} from '@grafana/ui';
import { BarChartOptions } from './types';
import { preparePlotConfigBuilder, preparePlotFrame } from './utils';
import { PropDiffFn } from '../../../../../packages/grafana-ui/src/components/GraphNG/GraphNG';
/**
* @alpha
@ -20,7 +21,14 @@ export interface BarChartProps
extends BarChartOptions,
Omit<GraphNGProps, 'prepConfig' | 'propsToDiff' | 'renderLegend' | 'theme'> {}
const propsToDiff: string[] = ['orientation', 'barWidth', 'groupWidth', 'stacking', 'showValue', 'text'];
const propsToDiff: Array<string | PropDiffFn> = [
'orientation',
'barWidth',
'groupWidth',
'stacking',
'showValue',
(prev: BarChartProps, next: BarChartProps) => next.text?.valueSize === prev.text?.valueSize,
];
export const BarChart: React.FC<BarChartProps> = (props) => {
const theme = useTheme2();

View File

@ -272,22 +272,22 @@ export function getConfig(opts: BarsOptions, theme: GrafanaTheme2) {
const interpolateTooltip: PlotTooltipInterpolator = (
updateActiveSeriesIdx,
updateActiveDatapointIdx,
updateTooltipPosition
updateTooltipPosition,
u
) => {
return (u: uPlot) => {
let found: Rect | undefined;
let cx = u.cursor.left! * devicePixelRatio;
let cy = u.cursor.top! * devicePixelRatio;
let found: Rect | undefined;
let cx = u.cursor.left! * devicePixelRatio;
let cy = u.cursor.top! * devicePixelRatio;
qt.get(cx, cy, 1, 1, (o) => {
if (pointWithin(cx, cy, o.x, o.y, o.x + o.w, o.y + o.h)) {
found = o;
}
});
qt.get(cx, cy, 1, 1, (o) => {
if (pointWithin(cx, cy, o.x, o.y, o.x + o.w, o.y + o.h)) {
found = o;
}
});
if (found) {
// prettier-ignore
if (found !== hovered) {
if (found) {
// prettier-ignore
if (found !== hovered) {
barMark.style.display = '';
barMark.style.left = found.x / devicePixelRatio + 'px';
barMark.style.top = found.y / devicePixelRatio + 'px';
@ -298,16 +298,15 @@ export function getConfig(opts: BarsOptions, theme: GrafanaTheme2) {
updateActiveDatapointIdx(hovered.didx);
updateTooltipPosition();
}
} else if (hovered !== undefined) {
updateActiveSeriesIdx(hovered!.sidx);
updateActiveDatapointIdx(hovered!.didx);
updateTooltipPosition();
hovered = undefined;
barMark.style.display = 'none';
} else {
updateTooltipPosition(true);
}
};
} else if (hovered !== undefined) {
updateActiveSeriesIdx(hovered!.sidx);
updateActiveDatapointIdx(hovered!.didx);
updateTooltipPosition();
hovered = undefined;
barMark.style.display = 'none';
} else {
updateTooltipPosition(true);
}
};
let alignedTotals: AlignedData | null = null;

View File

@ -126,7 +126,7 @@ export const preparePlotConfigBuilder: UPlotConfigPrepFn<TimelineOptions> = ({
updateActiveSeriesIdx,
updateActiveDatapointIdx,
updateTooltipPosition
) => (u: uPlot) => {
) => {
if (shouldChangeHover) {
if (hoveredSeriesIdx != null) {
updateActiveSeriesIdx(hoveredSeriesIdx);
@ -271,7 +271,7 @@ export function prepareTimelineFields(
let isTimeseries = false;
let changed = false;
const fields: Field[] = [];
for (const field of frame.fields) {
for (let field of frame.fields) {
switch (field.type) {
case FieldType.time:
isTimeseries = true;
@ -281,8 +281,17 @@ export function prepareTimelineFields(
case FieldType.number:
case FieldType.boolean:
case FieldType.string:
// magic value for join() to leave nulls alone
(field.config.custom = field.config.custom ?? {}).spanNulls = -1;
field = {
...field,
config: {
...field.config,
custom: {
...field.config.custom,
// magic value for join() to leave nulls alone
spanNulls: -1,
},
},
};
if (mergeValues) {
let merged = unsetSameFutureValues(field.values.toArray());