-
Notifications
You must be signed in to change notification settings - Fork 13.1k
Alerting: Alerts page performance improvements #113391
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
base: main
Are you sure you want to change the base?
Conversation
| const handleDrawerOpen = useCallback(() => { | ||
| setIsDrawerOpen(true); | ||
| }; | ||
| }, []); | ||
|
|
||
| const handleDrawerClose = () => { | ||
| const handleDrawerClose = useCallback(() => { | ||
| setIsDrawerOpen(false); | ||
| }; | ||
| }, []); |
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.
To avoid OpenDrawerIconButton re-rendering
| useEffect(() => { | ||
| const transformData = (newState: typeof runner.state) => { | ||
| if (newState.data?.state !== 'Done' || !newState.data?.series) { | ||
| return; | ||
| } | ||
|
|
||
| // Get the groupBy from the scene directly to avoid having groupByVariable in the dependency array | ||
| let currentGroupByKeys: string[] = []; | ||
| const groupByVariable = sceneGraph.lookupVariable(VARIABLES.groupBy, runner); | ||
|
|
||
| if (groupByVariable && sceneUtils.isGroupByVariable(groupByVariable)) { | ||
| const value = groupByVariable.getValue(); | ||
| if (Array.isArray(value)) { | ||
| currentGroupByKeys = value.map((value) => String(value)); | ||
| } | ||
| } | ||
|
|
||
| const { series } = newState.data; | ||
| // Use transition for non-blocking update | ||
| startTransition(() => { | ||
| setRows(convertToWorkbenchRows(series, currentGroupByKeys)); | ||
| }); |
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.
I changed the way we convert time series to table rows. This process can be quite expensive for many rows and it's quite tricky to limit the number of recalculations relying only on const { data } = runner.useState(); because there are many things that are changing and triggering rerenders (filters, grouping, timerange)
The new approach is more performant, but also more complex.
Let me now what you think!
| let currentGroupByKeys: string[] = []; | ||
| const groupByVariable = sceneGraph.lookupVariable(VARIABLES.groupBy, runner); | ||
|
|
||
| if (groupByVariable && sceneUtils.isGroupByVariable(groupByVariable)) { | ||
| const value = groupByVariable.getValue(); | ||
| if (Array.isArray(value)) { | ||
| currentGroupByKeys = value.map((value) => String(value)); | ||
| } | ||
| } |
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 code is run asynchronously (it's a callback when called when the new time series data is available) so using groupByKeys from line 20 could lead to reruning this effect multiple times and odd issues
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.
I had written these tests first, for the old conversion algorithm. Then I swapped the algorithm to ensure it works the same way
|
|
||
| // Single-pass implementation: 30x faster than previous approach | ||
| // Builds tree structure in one pass through data, avoiding intermediate row objects | ||
| export function convertToWorkbenchRows(series: DataFrame[], groupBy: string[] = []): WorkbenchRow[] { |
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 algorithm seem to be faster. I run a few benchmarks to compare its performance with the old one and I feel the results are worth the complexity
| Test | Old (ops/sec) | Old (avg time) | New (ops/sec) | New (avg time) | Speedup |
|---|---|---|---|---|---|
| No grouping | 11.26 | 88.790 ms | 384.62 | 2.600 ms | 34.2x |
| Single groupBy (grafana_folder) | 6.55 | 152.710 ms | 59.31 | 16.860 ms | 9.1x |
| Double groupBy (grafana_folder, alertstate) | 6.13 | 163.020 ms | 43.55 | 22.960 ms | 7.1x |
| } | ||
|
|
||
| // Get required field value arrays (direct columnar access) | ||
| const alertnameIndex = fieldIndex.get('alertname'); |
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.
Could these index values ever be 0? If so these will be falsey so may have to explicitly check if they're undefined?
|
|
||
| const getStyles = (theme: GrafanaTheme2) => ({ | ||
| iconButton: css({ | ||
| transform: 'rotate(180deg)', |
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.
Could we avoid calling useStyles on every render by using inline styling as the css is so simple?
Overview
This PR introduces significant performance optimizations for the Alerting Triage page, focusing on reducing CPU overhead during data transformation and visualization rendering. The changes include lazy-loading visualization panels and implementing a more efficient data transformation algorithm.
Changes Summary
Performance Optimizations
Lazy-loading visualization panels - Uses IntersectionObserver to render expensive visualization components only when they approach the viewport, preventing unnecessary CPU operations for off-screen elements.
Optimized data transformation - Implements a new single-pass algorithm that is approximately 30x faster than the previous approach when processing large datasets.
Reduced redundant transformations - Uses
subscribeToState()instead ofuseState()to transform data only when it actually changes, avoiding 2-3 unnecessary transformation calls per update.Files Changed
Modified Files
public/app/features/alerting/unified/triage/Workbench.tsxisLoadingprop supportpublic/app/features/alerting/unified/triage/scene/AlertRuleSummary.tsxAlertRuleSummaryViz(contains expensive hooks) andAlertRuleSummary(lazy-loading wrapper)public/app/features/alerting/unified/triage/scene/Workbench.tsxdataTransform.ts)subscribeToState()instead ofuseState()for more efficient state managementuseTransitionhook for non-blocking updates during data transformationNew Files
public/app/features/alerting/unified/triage/scene/dataTransform.tsconvertToWorkbenchRows()public/app/features/alerting/unified/triage/scene/dataTransform.test.tsTechnical Details
IntersectionObserver Implementation
Data Transformation Algorithm
State Management
useState()withsubscribeToState()to avoid unnecessary re-rendersuseTransition()for non-blocking updates during expensive transformations