Variables: Optimize "timeRangeUpdated" path (#52047)

.betterer.results

@@ -3512,8 +3512,7 @@ exports[`better eslint`] = {
       [0, 0, 0, "Unexpected any. Specify a different type.", "3"]
     ],
     "public/app/core/utils/dag.ts:5381": [
-      [0, 0, 0, "Unexpected any. Specify a different type.", "0"],
-      [0, 0, 0, "Unexpected any. Specify a different type.", "1"]
+      [0, 0, 0, "Unexpected any. Specify a different type.", "0"]
     ],
     "public/app/core/utils/deferred.ts:5381": [
       [0, 0, 0, "Unexpected any. Specify a different type.", "0"],

public/app/core/utils/dag.ts

@@ -118,7 +118,7 @@ export class Node {
 }
 
 export class Graph {
-  nodes: any = {};
+  nodes: Record<string, Node> = {};
 
   constructor() {}
 
@@ -189,6 +189,34 @@ export class Graph {
     return edges;
   }
 
+  descendants(nodes: Node[] | string[]): Set<Node> {
+    if (!nodes.length) {
+      return new Set();
+    }
+
+    const initialNodes = new Set(
+      isStringArray(nodes) ? nodes.map((n) => this.nodes[n]).filter((n) => n !== undefined) : nodes
+    );
+
+    return this.descendantsRecursive(initialNodes);
+  }
+
+  private descendantsRecursive(nodes: Set<Node>, descendants = new Set<Node>()): Set<Node> {
+    for (const node of nodes) {
+      const newDescendants = new Set<Node>();
+      for (const { inputNode } of node.inputEdges) {
+        if (inputNode && !descendants.has(inputNode)) {
+          descendants.add(inputNode);
+          newDescendants.add(inputNode);
+        }
+      }
+
+      this.descendantsRecursive(newDescendants, descendants);
+    }
+
+    return descendants;
+  }
+
   createEdge(): Edge {
     return new Edge();
   }
@@ -212,3 +240,7 @@ export const printGraph = (g: Graph) => {
     console.log(`${n.name}:\n - links to:   ${outputEdges}\n - links from: ${inputEdges}`);
   });
 };
+
+function isStringArray(arr: unknown[]): arr is string[] {
+  return arr.length > 0 && typeof arr[0] === 'string';
+}

public/app/core/utils/set.ts

@@ -0,0 +1,8 @@
+export function mapSet<T, R>(set: Set<T>, callback: (t: T) => R): Set<R> {
+  const newSet = new Set<R>();
+  for (const el of set) {
+    newSet.add(callback(el));
+  }
+
+  return newSet;
+}

public/app/features/dashboard/state/PanelModel.ts

@@ -19,6 +19,7 @@ import {
 } from '@grafana/data';
 import { getTemplateSrv, RefreshEvent } from '@grafana/runtime';
 import config from 'app/core/config';
+import { safeStringifyValue } from 'app/core/utils/explore';
 import { getNextRefIdChar } from 'app/core/utils/query';
 import { QueryGroupOptions } from 'app/types';
 import {
@@ -654,3 +655,18 @@ interface PanelOptionsCache {
   properties: any;
   fieldConfig: FieldConfigSource;
 }
+
+// For cases where we immediately want to stringify the panel model without cloning each property
+export function stringifyPanelModel(panel: PanelModel) {
+  const model: any = {};
+
+  Object.entries(panel)
+    .filter(
+      ([prop, val]) => !notPersistedProperties[prop] && panel.hasOwnProperty(prop) && !isEqual(val, defaults[prop])
+    )
+    .forEach(([k, v]) => {
+      model[k] = v;
+    });
+
+  return safeStringifyValue(model);
+}

public/app/features/variables/inspect/utils.test.ts

@@ -4,14 +4,9 @@ import { variableAdapters } from '../adapters';
 import { createCustomVariableAdapter } from '../custom/adapter';
 import { createDataSourceVariableAdapter } from '../datasource/adapter';
 import { createQueryVariableAdapter } from '../query/adapter';
+import { createGraph } from '../state/actions';
 
-import {
-  flattenPanels,
-  getAffectedPanelIdsForVariable,
-  getAllAffectedPanelIdsForVariableChange,
-  getDependenciesForVariable,
-  getPropsWithVariable,
-} from './utils';
+import { flattenPanels, getAllAffectedPanelIdsForVariableChange, getPanelVars, getPropsWithVariable } from './utils';
 
 describe('getPropsWithVariable', () => {
   it('when called it should return the correct graph', () => {
@@ -217,43 +212,12 @@ describe('getPropsWithVariable', () => {
   });
 });
 
-describe('getAffectedPanelIdsForVariable', () => {
-  describe('when called with a real world example with rows and repeats', () => {
-    it('then it should return correct panel ids', () => {
-      const panels = dashWithRepeatsAndRows.panels.map((panel: PanelModel) => ({
-        id: panel.id,
-        getSaveModel: () => panel,
-      }));
-      const result = getAffectedPanelIdsForVariable('query0', panels);
-      expect(result).toEqual([15, 16, 17, 11, 12, 13, 2, 5, 7, 6]);
-    });
-  });
-});
-
 variableAdapters.setInit(() => [
   createDataSourceVariableAdapter(),
   createCustomVariableAdapter(),
   createQueryVariableAdapter(),
 ]);
 
-describe('getDependenciesForVariable', () => {
-  describe('when called with a real world example with dependencies', () => {
-    it('then it should return correct dependencies', () => {
-      const {
-        templating: { list: variables },
-      } = dashWithTemplateDependenciesAndPanels;
-      const result = getDependenciesForVariable('ds_instance', variables, new Set());
-      expect([...result]).toEqual([
-        'ds',
-        'query_with_ds',
-        'depends_on_query_with_ds',
-        'depends_on_query_with_ds_regex',
-        'depends_on_all',
-      ]);
-    });
-  });
-});
-
 describe('getAllAffectedPanelIdsForVariableChange ', () => {
   describe('when called with a real world example with dependencies and panels', () => {
     it('then it should return correct panelIds', () => {
@@ -261,12 +225,11 @@ describe('getAllAffectedPanelIdsForVariableChange ', () => {
         panels: panelsAsJson,
         templating: { list: variables },
       } = dashWithTemplateDependenciesAndPanels;
-      const panels = panelsAsJson.map((panel: PanelModel) => ({
-        id: panel.id,
-        getSaveModel: () => panel,
-      }));
-      const result = getAllAffectedPanelIdsForVariableChange('ds_instance', variables, panels);
-      expect(result).toEqual([2, 3, 4, 5]);
+      const panelVarPairs = getPanelVars(panelsAsJson);
+      const varGraph = createGraph(variables);
+
+      const result = [...getAllAffectedPanelIdsForVariableChange(['ds_instance'], varGraph, panelVarPairs)];
+      expect(result).toEqual([5, 2, 4, 3]);
     });
   });
 
@@ -276,11 +239,9 @@ describe('getAllAffectedPanelIdsForVariableChange ', () => {
         panels: panelsAsJson,
         templating: { list: variables },
       } = dashWithTemplateDependenciesAndPanels;
-      const panels = panelsAsJson.map((panel: PanelModel) => ({
-        id: panel.id,
-        getSaveModel: () => panel,
-      }));
-      const result = getAllAffectedPanelIdsForVariableChange('depends_on_all', variables, panels);
+      const panelVarPairs = getPanelVars(panelsAsJson);
+      const varGraph = createGraph(variables);
+      const result = [...getAllAffectedPanelIdsForVariableChange(['depends_on_all'], varGraph, panelVarPairs)];
       expect(result).toEqual([2]);
     });
   });
@@ -291,11 +252,9 @@ describe('getAllAffectedPanelIdsForVariableChange ', () => {
         panels: panelsAsJson,
         templating: { list: variables },
       } = dashWithAllVariables;
-      const panels = panelsAsJson.map((panel: PanelModel) => ({
-        id: panel.id,
-        getSaveModel: () => panel,
-      }));
-      const result = getAllAffectedPanelIdsForVariableChange('unknown', variables, panels);
+      const panelVarPairs = getPanelVars(panelsAsJson);
+      const varGraph = createGraph(variables);
+      const result = [...getAllAffectedPanelIdsForVariableChange(['unknown'], varGraph, panelVarPairs)];
       expect(result).toEqual([2, 3]);
     });
   });

public/app/features/variables/inspect/utils.ts

@@ -1,4 +1,7 @@
 import { DataLinkBuiltInVars } from '@grafana/data';
+import { Graph } from 'app/core/utils/dag';
+import { mapSet } from 'app/core/utils/set';
+import { stringifyPanelModel } from 'app/features/dashboard/state/PanelModel';
 
 import { safeStringifyValue } from '../../../core/utils/explore';
 import { DashboardModel, PanelModel } from '../../dashboard/state';
@@ -51,10 +54,10 @@ export const createDependencyEdges = (variables: VariableModel[]): GraphEdge[] =
   return edges;
 };
 
-function getVariableName(expression: string) {
+export function getVariableName(expression: string) {
   const match = variableRegexExec(expression);
   if (!match) {
-    return null;
+    return undefined;
   }
   const variableName = match.slice(1).find((match) => match !== undefined);
   return variableName;
@@ -253,86 +256,32 @@ function createUnknownsNetwork(variables: VariableModel[], dashboard: DashboardM
 
   This doesn't take circular dependencies in consideration.
  */
+
 export function getAllAffectedPanelIdsForVariableChange(
-  variableId: string,
-  variables: VariableModel[],
-  panels: PanelModel[]
-): number[] {
-  const flattenedPanels = flattenPanels(panels);
-  let affectedPanelIds: number[] = getAffectedPanelIdsForVariable(variableId, flattenedPanels);
-  const affectedPanelIdsForAllVariables = getAffectedPanelIdsForVariable(
-    DataLinkBuiltInVars.includeVars,
-    flattenedPanels
-  );
-  affectedPanelIds = [...new Set([...affectedPanelIdsForAllVariables, ...affectedPanelIds])];
-
-  const dependencies = getDependenciesForVariable(variableId, variables, new Set());
-  for (const dependency of dependencies) {
-    const affectedPanelIdsForDependency = getAffectedPanelIdsForVariable(dependency, flattenedPanels);
-    affectedPanelIds = [...new Set([...affectedPanelIdsForDependency, ...affectedPanelIds])];
+  variableIds: string[],
+  variableGraph: Graph,
+  panelsByVar: Record<string, Set<number>>
+): Set<number> {
+  const allDependencies = mapSet(variableGraph.descendants(variableIds), (n) => n.name);
+  allDependencies.add(DataLinkBuiltInVars.includeVars);
+  for (const id of variableIds) {
+    allDependencies.add(id);
   }
 
+  const affectedPanelIds = getDependentPanels([...allDependencies], panelsByVar);
   return affectedPanelIds;
 }
 
-export function getDependenciesForVariable(
-  variableId: string,
-  variables: VariableModel[],
-  deps: Set<string>
-): Set<string> {
-  if (!variables.length) {
-    return deps;
-  }
-
-  for (const variable of variables) {
-    if (variable.name === variableId) {
-      continue;
-    }
-
-    const depends = variableAdapters.get(variable.type).dependsOn(variable, { name: variableId });
-    if (!depends) {
-      continue;
-    }
-
-    deps.add(variable.name);
-    deps = getDependenciesForVariable(variable.name, variables, deps);
-  }
-
-  return deps;
-}
-
-export function getAffectedPanelIdsForVariable(variableId: string, panels: PanelModel[]): number[] {
-  if (!panels.length) {
-    return [];
-  }
-
-  const affectedPanelIds: number[] = [];
-  const repeatRegex = new RegExp(`"repeat":"${variableId}"`);
-  for (const panel of panels) {
-    const panelAsJson = safeStringifyValue(panel.getSaveModel());
-
-    // check for repeats that don't use variableRegex
-    const repeatMatches = panelAsJson.match(repeatRegex);
-    if (repeatMatches?.length) {
-      affectedPanelIds.push(panel.id);
-      continue;
-    }
-
-    const matches = panelAsJson.match(variableRegex);
-    if (!matches) {
-      continue;
-    }
-
-    for (const match of matches) {
-      const variableName = getVariableName(match);
-      if (variableName === variableId) {
-        affectedPanelIds.push(panel.id);
-        break;
-      }
+// Return an array of panel IDs depending on variables
+export function getDependentPanels(variables: string[], panelsByVarUsage: Record<string, Set<number>>) {
+  const thePanels: number[] = [];
+  for (const varId of variables) {
+    if (panelsByVarUsage[varId]) {
+      thePanels.push(...panelsByVarUsage[varId]);
     }
   }
 
-  return affectedPanelIds;
+  return new Set(thePanels);
 }
 
 export interface UsagesToNetwork {
@@ -419,3 +368,24 @@ export function flattenPanels(panels: PanelModel[]): PanelModel[] {
 
   return result;
 }
+
+// Accepts an array of panel models, and returns an array of panel IDs paired with
+// the names of any template variables found
+export function getPanelVars(panels: PanelModel[]) {
+  const panelsByVar: Record<string, Set<number>> = {};
+  for (const p of panels) {
+    const jsonString = stringifyPanelModel(p);
+    const repeats = [...jsonString.matchAll(/"repeat":"([^"]+)"/g)].map((m) => m[1]);
+    const varRegexMatches = jsonString.match(variableRegex)?.map((m) => getVariableName(m)) ?? [];
+    const varNames = [...repeats, ...varRegexMatches];
+    for (const varName of varNames) {
+      if (varName! in panelsByVar) {
+        panelsByVar[varName!].add(p.id);
+      } else {
+        panelsByVar[varName!] = new Set([p.id]);
+      }
+    }
+  }
+
+  return panelsByVar;
+}

public/app/features/variables/state/actions.ts

@@ -36,7 +36,7 @@ import {
   isMulti,
   isQuery,
 } from '../guard';
-import { getAllAffectedPanelIdsForVariableChange } from '../inspect/utils';
+import { getAllAffectedPanelIdsForVariableChange, getPanelVars } from '../inspect/utils';
 import { cleanPickerState } from '../pickers/OptionsPicker/reducer';
 import { alignCurrentWithMulti } from '../shared/multiOptions';
 import {
@@ -549,7 +549,7 @@ export const setOptionAsCurrent = (
   };
 };
 
-const createGraph = (variables: VariableModel[]) => {
+export const createGraph = (variables: VariableModel[]) => {
   const g = new Graph();
 
   variables.forEach((v) => {
@@ -594,9 +594,14 @@ export const variableUpdated = (
     const variables = getVariablesByKey(rootStateKey, state);
     const g = createGraph(variables);
     const panels = state.dashboard?.getModel()?.panels ?? [];
+    const panelVars = getPanelVars(panels);
+
     const event: VariablesChangedEvent = isAdHoc(variableInState)
       ? { refreshAll: true, panelIds: [] } // for adhoc variables we don't know which panels that will be impacted
-      : { refreshAll: false, panelIds: getAllAffectedPanelIdsForVariableChange(variableInState.id, variables, panels) };
+      : {
+          refreshAll: false,
+          panelIds: Array.from(getAllAffectedPanelIdsForVariableChange([variableInState.id], g, panelVars)),
+        };
 
     const node = g.getNode(variableInState.name);
     let promises: Array<Promise<any>> = [];
@@ -643,7 +648,7 @@ export const onTimeRangeUpdated =
     }) as VariableWithOptions[];
 
     const variableIds = variablesThatNeedRefresh.map((variable) => variable.id);
-    const promises = variablesThatNeedRefresh.map((variable: VariableWithOptions) =>
+    const promises = variablesThatNeedRefresh.map((variable) =>
       dispatch(timeRangeUpdated(toKeyedVariableIdentifier(variable)))
     );
 
@@ -678,8 +683,8 @@ export const templateVarsChangedInUrl =
   async (dispatch, getState) => {
     const update: Array<Promise<any>> = [];
     const dashboard = getState().dashboard.getModel();
-    const panelIds = new Set<number>();
     const variables = getVariablesByKey(key, getState());
+
     for (const variable of variables) {
       const key = `var-${variable.name}`;
       if (!vars.hasOwnProperty(key)) {
@@ -706,24 +711,29 @@ export const templateVarsChangedInUrl =
         }
       }
 
-      // for adhoc variables we don't know which panels that will be impacted
-      if (!isAdHoc(variable)) {
-        getAllAffectedPanelIdsForVariableChange(variable.id, variables, dashboard?.panels ?? []).forEach((id) =>
-          panelIds.add(id)
-        );
-      }
-
       const promise = variableAdapters.get(variable.type).setValueFromUrl(variable, value);
       update.push(promise);
     }
 
+    const filteredVars = variables.filter((v) => {
+      const key = `var-${v.name}`;
+      return vars.hasOwnProperty(key) && isVariableUrlValueDifferentFromCurrent(v, vars[key].value) && !isAdHoc(v);
+    });
+    const varGraph = createGraph(variables);
+    const panelVars = getPanelVars(dashboard?.panels ?? []);
+    const affectedPanels = getAllAffectedPanelIdsForVariableChange(
+      filteredVars.map((v) => v.id),
+      varGraph,
+      panelVars
+    );
+
     if (update.length) {
       await Promise.all(update);
 
       events.publish(
         new VariablesChangedInUrl({
-          refreshAll: panelIds.size === 0,
-          panelIds: Array.from(panelIds),
+          refreshAll: affectedPanels.size === 0,
+          panelIds: Array.from(affectedPanels),
         })
       );
     }