Improve getUpdateStatus performance (#16403) (#16874)

* Only update status when required

* Add missing Status.PARAMS

* Add requiresUpdateStatus to Vega vis

* Add requiresUpdateStatus for editors

* Fix stupid issue

* Create unit tests for getUpdateStatus method

src/core_plugins/input_control_vis/public/register_vis.js

@@ -7,7 +7,7 @@ import { ControlsTab } from './components/editor/controls_tab';
 import { OptionsTab } from './components/editor/options_tab';
 import { defaultFeedbackMessage } from 'ui/vis/default_feedback_message';
 import image from './images/icon-input-control.svg';
-
+import { Status } from 'ui/vis/update_status';
 
 function InputControlVisProvider(Private) {
   const VisFactory = Private(VisFactoryProvider);
@@ -20,6 +20,7 @@ function InputControlVisProvider(Private) {
     description: 'Create interactive controls for easy dashboard manipulation.',
     category: CATEGORY.OTHER,
     stage: 'lab',
+    requiresUpdateStatus: [Status.PARAMS, Status.TIME],
     feedbackMessage: defaultFeedbackMessage,
     visualization: VisController,
     visConfig: {

src/core_plugins/region_map/public/region_map_vis.js

@@ -8,6 +8,7 @@ import { VisTypesRegistryProvider } from 'ui/registry/vis_types';
 import { truncatedColorMaps } from 'ui/vislib/components/color/truncated_colormaps';
 import { mapToLayerWithId } from './util';
 import { RegionMapsVisualizationProvider } from './region_map_visualization';
+import { Status } from 'ui/vis/update_status';
 
 VisTypesRegistryProvider.register(function RegionMapProvider(Private, regionmapsConfig, config) {
   const VisFactory = Private(VisFactoryProvider);
@@ -40,6 +41,7 @@ VisTypesRegistryProvider.register(function RegionMapProvider(Private, regionmaps
         showAllShapes: true//still under consideration
       }
     },
+    requiresUpdateStatus: [Status.AGGS, Status.PARAMS, Status.RESIZE, Status.DATA, Status.UI_STATE],
     visualization: RegionMapsVisualization,
     editorConfig: {
       optionsTemplate: '<region_map-vis-params></region_map-vis-params>',

src/core_plugins/tagcloud/public/tag_cloud_vis.js

@@ -7,6 +7,7 @@ import { VisSchemasProvider } from 'ui/vis/editors/default/schemas';
 import tagCloudTemplate from 'plugins/tagcloud/tag_cloud_controller.html';
 import { VisTypesRegistryProvider } from 'ui/registry/vis_types';
 import image from './images/icon-tagcloud.svg';
+import { Status } from 'ui/vis/update_status';
 
 VisTypesRegistryProvider.register(function TagCloudProvider(Private) {
   const VisFactory = Private(VisFactoryProvider);
@@ -27,6 +28,7 @@ VisTypesRegistryProvider.register(function TagCloudProvider(Private) {
       },
       template: tagCloudTemplate,
     },
+    requiresUpdateStatus: [Status.PARAMS, Status.RESIZE],
     responseHandler: 'tabify',
     editorConfig: {
       collections: {
@@ -59,5 +61,3 @@ VisTypesRegistryProvider.register(function TagCloudProvider(Private) {
     }
   });
 });
-
-

src/core_plugins/tile_map/public/tile_map_vis.js

@@ -8,6 +8,7 @@ import { VisSchemasProvider } from 'ui/vis/editors/default/schemas';
 import { AggResponseGeoJsonProvider } from 'ui/agg_response/geo_json/geo_json';
 import image from './images/icon-tilemap.svg';
 import { VisTypesRegistryProvider } from 'ui/registry/vis_types';
+import { Status } from 'ui/vis/update_status';
 
 
 VisTypesRegistryProvider.register(function TileMapVisType(Private, getAppState, courier, config) {
@@ -36,6 +37,7 @@ VisTypesRegistryProvider.register(function TileMapVisType(Private, getAppState,
         wms: config.get('visualization:tileMap:WMSdefaults')
       }
     },
+    requiresUpdateStatus: [Status.AGGS, Status.PARAMS, Status.RESIZE, Status.DATA, Status.UI_STATE],
     responseConverter: geoJsonConverter,
     responseHandler: 'basic',
     visualization: CoordinateMapsVisualization,

src/core_plugins/vega/public/vega_type.js

@@ -2,6 +2,7 @@ import { VisTypesRegistryProvider } from 'ui/registry/vis_types';
 import { VisFactoryProvider } from 'ui/vis/vis_factory';
 import { CATEGORY } from 'ui/vis/vis_category';
 import { DefaultEditorSize } from 'ui/vis/editor_size';
+import { Status } from 'ui/vis/update_status';
 
 import { VegaRequestHandlerProvider } from './vega_request_handler';
 import { VegaVisualizationProvider } from './vega_visualization';
@@ -34,6 +35,7 @@ VisTypesRegistryProvider.register((Private) => {
       defaultSize: DefaultEditorSize.MEDIUM,
     },
     visualization: VegaVisualization,
+    requiresUpdateStatus: [Status.DATA, Status.RESIZE],
     requestHandler: vegaRequestHandler,
     responseHandler: 'none',
     options: { showIndexSelection: false },

src/ui/public/vis/update_status.js

@@ -1,5 +1,14 @@
 import { calculateObjectHash } from './lib/calculate_object_hash';
 
+const Status = {
+  AGGS: 'aggs',
+  DATA: 'data',
+  PARAMS: 'params',
+  RESIZE: 'resize',
+  TIME: 'time',
+  UI_STATE: 'uiState',
+};
+
 // adapted from https://github.com/isaacs/json-stringify-safe/blob/02cfafd45f06d076ac4bf0dd28be6738a07a72f9/stringify.js
 function serializer() {
   const stack = [];
@@ -23,7 +32,12 @@ function serializer() {
   };
 }
 
-const getUpdateStatus = ($scope) => {
+function getUpdateStatus(requiresUpdateStatus, $scope) {
+
+  // If the vis type doesn't need update status, skip all calculations
+  if (!requiresUpdateStatus) {
+    return {};
+  }
 
   if (!$scope._oldStatus) {
     $scope._oldStatus = {};
@@ -61,17 +75,36 @@ const getUpdateStatus = ($scope) => {
     return false;
   }
 
-  const time = $scope.vis.params.timeRange ? $scope.vis.params.timeRange : $scope.vis.API.timeFilter.getBounds();
-  const width = $scope.vis.size ? $scope.vis.size[0] : 0;
-  const height = $scope.vis.size ? $scope.vis.size[1] : 0;
-  return {
-    aggs: hasChangedUsingGenericHashComparison('aggs', $scope.vis.aggs),
-    data: hasDataChanged($scope.visData),
-    params: hasChangedUsingGenericHashComparison('param', $scope.vis.params),
-    resize: hasSizeChanged(width, height),
-    time: hasChangedUsingGenericHashComparison('time', time),
-    uiState: hasChangedUsingGenericHashComparison('uiState', $scope.uiState)
-  };
-};
+  const status = {};
 
-export { getUpdateStatus };
+  for (const requiredStatus of requiresUpdateStatus) {
+    // Calculate all required status updates for this visualization
+    switch (requiredStatus) {
+      case Status.AGGS:
+        status.aggs = hasChangedUsingGenericHashComparison('aggs', $scope.vis.aggs);
+        break;
+      case Status.DATA:
+        status.data = hasDataChanged($scope.visData);
+        break;
+      case Status.PARAMS:
+        status.params = hasChangedUsingGenericHashComparison('param', $scope.vis.params);
+        break;
+      case Status.RESIZE:
+        const width = $scope.vis.size ? $scope.vis.size[0] : 0;
+        const height = $scope.vis.size ? $scope.vis.size[1] : 0;
+        status.resize = hasSizeChanged(width, height);
+        break;
+      case Status.TIME:
+        const time = $scope.vis.params.timeRange ? $scope.vis.params.timeRange : $scope.vis.API.timeFilter.getBounds();
+        status.time = hasChangedUsingGenericHashComparison('time', time);
+        break;
+      case Status.UI_STATE:
+        status.uiState = hasChangedUsingGenericHashComparison('uiState', $scope.uiState);
+        break;
+    }
+  }
+
+  return status;
+}
+
+export { getUpdateStatus, Status };

src/ui/public/vis/update_status.test.js

@@ -0,0 +1,89 @@
+import { getUpdateStatus, Status } from './update_status';
+
+// Parts of the tests in this file are generated more dynamically, based on the
+// values inside the Status object.Make sure this object has one function per entry
+// in Status, that actually change on the passed $scope, what needs to be changed
+// so that we expect the getUpdateStatus function to actually detect a change.
+const changeFunctions = {
+  [Status.AGGS]: ($scope) => $scope.vis.aggs = { foo: 'new' },
+  [Status.DATA]: ($scope) => $scope.visData = { foo: 'new' },
+  [Status.PARAMS]: ($scope) => $scope.vis.params = { foo: 'new' },
+  [Status.RESIZE]: ($scope) => $scope.vis.size = [50, 50],
+  [Status.TIME]: ($scope) => $scope.vis.API.timeFilter.getBounds = () => [100, 100],
+  [Status.UI_STATE]: ($scope) => $scope.uiState = { foo: 'new' },
+};
+
+describe('getUpdateStatus', () => {
+
+  function getScope() {
+    return {
+      vis: {
+        aggs: {},
+        size: [100, 100],
+        params: {
+        },
+        API: {
+          timeFilter: {
+            getBounds: () => [50, 50]
+          }
+        }
+      },
+      uiState: {},
+      visData: {}
+    };
+  }
+
+  function initStatusCheckerAndChangeProperty(type, requiresUpdateStatus) {
+    const $scope = getScope();
+    // Call the getUpdateStatus function initially, so it can store it's current state
+    getUpdateStatus(requiresUpdateStatus, $scope);
+
+    // Get the change function for that specific change type
+    const changeFn = changeFunctions[type];
+    if (!changeFn) {
+      throw new Error(`Please implement the test change function for ${type}.`);
+    }
+
+    // Call that change function to manipulate the scope so it changed.
+    changeFn($scope);
+
+    return getUpdateStatus(requiresUpdateStatus, $scope);
+  }
+
+  it('should be a function', () => {
+    expect(typeof getUpdateStatus).toBe('function');
+  });
+
+  Object.entries(Status).forEach(([typeKey, typeValue]) => {
+    // This block automatically creates very simple tests for each of the Status
+    // keys, so we have simple tests per changed property.
+    // If it makes sense to test more specific behavior of a specific change detection
+    // please add additional tests for that.
+
+    it(`should detect changes for Status.${typeKey}`, () => {
+      // Check whether the required change type is not correctly determined
+      const status = initStatusCheckerAndChangeProperty(typeValue, [typeValue]);
+      expect(status[typeValue]).toBe(true);
+    });
+
+    it(`should not detect changes in other properties when changing Status.${typeKey}`, () => {
+      // Only change typeKey, but track changes for all status changes
+      const status = initStatusCheckerAndChangeProperty(typeValue, Object.values(Status));
+      Object.values(Status)
+        // Filter out the actual changed property so we only test for all other properties
+        .filter(stat => stat !== typeValue)
+        .forEach(otherProp => {
+          expect(status[otherProp]).toBeFalsy();
+        });
+    });
+
+    it(`should not detect changes if not requested for Status.${typeKey}`, () => {
+      const allOtherStatusProperties = Object.values(Status).filter(stat => stat !== typeValue);
+      // Change only the typeKey property, but do not listen for changes on it
+      // listen on all other status changes instead.
+      const status = initStatusCheckerAndChangeProperty(typeValue, allOtherStatusProperties);
+      // The typeValue check should be falsy, since we did not request tracking it.
+      expect(status[typeValue]).toBeFalsy();
+    });
+  });
+});

src/ui/public/visualize/visualization.js

@@ -80,7 +80,7 @@ uiModules
           .debounceTime(100)
           .switchMap(async ({ vis, visData, container }) => {
             vis.size = [container.width(), container.height()];
-            const status = getUpdateStatus($scope);
+            const status = getUpdateStatus(vis.type.requiresUpdateStatus, $scope);
             const renderPromise = visualization.render(visData, status);
             $scope.$apply();
             return renderPromise;

src/ui/public/visualize/visualization_editor.js

@@ -29,7 +29,7 @@ uiModules
 
         $scope.renderFunction = () => {
           if (!$scope.vis) return;
-          editor.render($scope.visData, $scope.searchSource, getUpdateStatus($scope), $scope.uiState);
+          editor.render($scope.visData, $scope.searchSource, getUpdateStatus(Editor.requiresUpdateStatus, $scope), $scope.uiState);
         };
 
         $scope.$on('render', (event) => {