[SIEM] Fix unnecessary re-renders on the Overview page by patrykkopycinski · Pull Request #56587 · elastic/kibana

patrykkopycinski · 2020-02-02T15:20:11Z

Summary

With help of why-did-you-render I was able to track and fix some of the unnecessary re-renders in our app.
As a first step, I've focused on the Overview page as it's our main page

Before:
On the initial load of Overview page we have ~155 unnecessary re-renders

After:
I was able to reduce this number to 14.
I've decided to not track redux connect wrappers as in 7.7 we will migrate to hooks.
I think I have found bug in charts library that causes unnecessary re-renders, I will confirm that this week
WithSource will be resolved after me migrate to #51926

Checklist

Use ~~strikethroughs~~ to remove checklist items you don't feel are applicable to this PR.

This was checked for cross-browser compatibility, including a check against IE11
Any text added follows EUI's writing guidelines, uses sentence case text and includes i18n support
Documentation was added for features that require explanation or tutorials
Unit or functional tests were updated or added to match the most common scenarios
This was checked for keyboard-only and screenreader accessibility

For maintainers

This was checked for breaking API changes and was labeled appropriately
This includes a feature addition or change that requires a release note and was labeled appropriately

elasticmachine · 2020-02-02T15:20:13Z

Pinging @elastic/siem (Team:SIEM)

…ep-equal

FrankHassanabad · 2020-02-03T16:02:49Z

x-pack/legacy/plugins/siem/package.json

    "@types/js-yaml": "^3.12.1",
-    "@types/react-beautiful-dnd": "^11.0.4"
+    "@types/react-beautiful-dnd": "^11.0.4",
+    "@welldone-software/why-did-you-render": "^4.0.0"


I would move this up to x-pack/package.json as we are suppose to keep our package.json within SIEM to a minimum as per the last time we talked to core team members.

FrankHassanabad · 2020-02-03T16:03:45Z

typings/fast_deep_equal.d.ts

+ */
+
+declare const equal: (a: any, b: any) => boolean;
+export = equal;


~~I would add a comment where this came from.~~

~~Looks like it is from one of these files?~~

~~epoberezkin/fast-deep-equal@f0e3dc1~~

~~and why we have to have it here and cannot use regular TypeScript without additional "cruft" and duplication for the next maintainer.~~

Ok, I see this above:

/* TODO: Remove after https://github.com/epoberezkin/fast-deep-equal/pull/48 is merged and fast-deep-equal version is bumped */

I think you are good!

…ep-equal

XavierM · 2020-02-05T15:11:46Z

...cy/plugins/siem/public/pages/detection_engine/components/signals_histogram_panel/helpers.tsx

  }, []);
 };

+export const memoFormatSignalsData = memoizeOne(formatSignalsData, deepEqual);


instead of doing this here, can we do the memoization where we do the fetch? So when other engineers will use the hooks will be already memoized.

XavierM · 2020-02-05T15:13:51Z

x-pack/legacy/plugins/siem/public/pages/home/index.tsx

-                    render={({ location, match }) => (
-                      <HostsContainer location={location} url={match.url} />
-                    )}
+                    render={({ match }) => <HostsContainer url={match.url} />}


I am wondering here since we have the new version of react-router, we might be able to use their hook to get the value inside of the component without the props.

XavierM · 2020-02-05T15:16:49Z

x-pack/legacy/plugins/siem/public/routes.tsx

 import { HomePage } from './pages/home';
 import { ManageRoutesSpy } from './utils/route/manage_spy_routes';

+/* Uncomment only during debugging */


do we want to keep it like that or adding it into a read.me. So it is easy to find it later on and maybe explain there how to find re-render problems and how to resolve them.

XavierM

I reviewed the code, I did not see anything weird there. However, give me some time to test it locally

…ep-equal # Conflicts: # x-pack/legacy/plugins/siem/public/pages/detection_engine/components/signals_histogram_panel/index.tsx

andrew-goldstein · 2020-02-06T00:36:39Z

x-pack/legacy/plugins/siem/public/components/charts/barchart.tsx

  );
 };

-BarChartComponent.displayName = 'BarChartComponent';


do the recent tooling changes make explicitly setting displayName unnecessary in cases like this?

Yes with @patrykkopycinski npm library

the library wasn't added @XavierM :( but the good news is that if we declare component as
const component:React.FC<>
and then declare
const MemoComponent = React.memo(Component)
it will get the displayName automatically

andrew-goldstein · 2020-02-06T00:41:55Z

x-pack/legacy/plugins/siem/public/components/flyout/index.tsx

  return { dataProviders, show, width };
 };

-export const Flyout = connect(mapStateToProps, {


🦅 this is a subtle one! looking forward to your PR that updates our redux deps!

andrew-goldstein · 2020-02-06T00:53:48Z

x-pack/legacy/plugins/siem/public/components/timeline/timeline.tsx

 import { getOr, isEmpty } from 'lodash/fp';
 import React from 'react';
 import styled from 'styled-components';
+import deepEqual from 'fast-deep-equal//react';


it appears the // may be missing es6, i.e.:

import deepEqual from 'fast-deep-equal/es6/react';

andrew-goldstein

Thank you @patrykkopycinski for the performance improvements in this PR, and for the knowledge sharing and demo with the team today! 🙏
LGTM 🚀

…ibana into chore/siem-deep-equal

patrykkopycinski · 2020-02-06T09:34:41Z

after reverting changes for using deepEqual in memo() we're back to ~220 re-renders

XavierM

LGTM, thank you for all the hard work. I still think that it will be nice to see the change when we move away from our render props. I will bet the number will go down

…ep-equal # Conflicts: # package.json # packages/kbn-ui-shared-deps/package.json # x-pack/legacy/plugins/siem/public/components/auto_sizer/index.tsx # x-pack/legacy/plugins/siem/public/components/charts/areachart.tsx # x-pack/legacy/plugins/siem/public/components/charts/barchart.tsx # x-pack/legacy/plugins/siem/public/components/drag_and_drop/draggable_wrapper.tsx # x-pack/legacy/plugins/siem/public/components/events_viewer/events_viewer.tsx # x-pack/legacy/plugins/siem/public/components/events_viewer/index.tsx # x-pack/legacy/plugins/siem/public/components/flyout/index.tsx # x-pack/legacy/plugins/siem/public/components/flyout/pane/index.tsx # x-pack/legacy/plugins/siem/public/components/page/hosts/authentications_table/index.tsx # x-pack/legacy/plugins/siem/public/components/page/hosts/hosts_table/index.tsx # x-pack/legacy/plugins/siem/public/components/page/hosts/uncommon_process_table/index.tsx # x-pack/legacy/plugins/siem/public/components/page/network/network_dns_table/index.tsx # x-pack/legacy/plugins/siem/public/components/page/network/network_http_table/index.tsx # x-pack/legacy/plugins/siem/public/components/page/network/network_top_countries_table/index.tsx # x-pack/legacy/plugins/siem/public/components/page/network/network_top_n_flow_table/index.tsx # x-pack/legacy/plugins/siem/public/components/page/network/tls_table/index.tsx # x-pack/legacy/plugins/siem/public/components/page/network/users_table/index.tsx # x-pack/legacy/plugins/siem/public/components/search_bar/index.tsx # x-pack/legacy/plugins/siem/public/components/timeline/index.tsx # x-pack/legacy/plugins/siem/public/components/timeline/query_bar/index.tsx # x-pack/legacy/plugins/siem/public/components/timeline/refetch_timeline.tsx # x-pack/legacy/plugins/siem/public/components/url_state/use_url_state.tsx # x-pack/legacy/plugins/siem/public/containers/global_time/index.tsx # x-pack/legacy/plugins/siem/public/pages/home/index.tsx # x-pack/legacy/plugins/siem/public/pages/hosts/details/details_tabs.tsx # x-pack/legacy/plugins/siem/public/pages/hosts/details/index.tsx # x-pack/legacy/plugins/siem/public/pages/hosts/hosts.tsx # x-pack/legacy/plugins/siem/public/pages/hosts/hosts_tabs.tsx # x-pack/legacy/plugins/siem/public/pages/network/navigation/network_routes.tsx # x-pack/legacy/plugins/siem/public/pages/overview/overview.tsx # yarn.lock

…ep-equal # Conflicts: # x-pack/legacy/plugins/siem/public/components/events_viewer/events_viewer.tsx # x-pack/legacy/plugins/siem/public/components/events_viewer/index.tsx

kibanamachine · 2020-02-25T12:36:46Z

💚 Build Succeeded

continuous-integration/kibana-ci/pull-request
Commit: 90ab4dd

History

💔 Build #28731 failed 92f292e
💔 Build #28711 failed f041e65
💔 Build #27324 failed 99c62f9
💚 Build #24860 succeeded 3ee3dd8
💔 Build #24813 failed aa2dca1

To update your PR or re-run it, just comment with:
@elasticmachine merge upstream

…re/files-and-filetree * 'master' of github.com:elastic/kibana: (174 commits) [SIEM] Fix unnecessary re-renders on the Overview page (elastic#56587) Don't mutate error message (elastic#58452) Fix service map popover transaction duration (elastic#58422) [ML] Adding filebeat config to file dataviz (elastic#58152) [Uptime] Improve refresh handling when generating test data (elastic#58285) [Logs / Metrics UI] Remove path prefix from ViewSourceConfigur… (elastic#58238) [ML] Functional tests - adjust classification model memory (elastic#58445) [ML] Use event.timezone instead of beat.timezone in file upload (elastic#58447) [Logs UI] Unskip and stabilitize log column configuration tests (elastic#58392) [Telemetry] Separate the license retrieval from the stats in the usage collectors (elastic#57332) hide welcome screen for cloud (elastic#58371) Move src/legacy/ui/public/notify/app_redirect to kibana_legacy (elastic#58127) [ML] Functional tests - stabilize typing during df analytics creation (elastic#58227) fix short url in spaces (elastic#58313) [SIEM] Upgrades cypress to version 4.0.2 (elastic#58400) [Index management] Move to new platform "plugins" folder (elastic#58109) [kbn/optimizer] disable parallelization in terser plugin (elastic#58396) [Uptime] Delete useless try...catch blocks (elastic#58263) [Uptime] Use scripted metric for snapshot calculation (elastic#58247) (elastic#58389) [APM] Stabilize agent configuration API (elastic#57767) ... # Conflicts: # src/plugins/console/public/application/containers/editor/legacy/console_editor/editor.tsx

kibanamachine · 2020-02-26T14:08:46Z

Looks like this PR has a backport PR but it still hasn't been merged. Please merge it ASAP to keep the branches relatively in sync.

patrykkopycinski added 2 commits February 2, 2020 15:24

[SIEM] WIP

59ddb37

add why-did-you-render

112a64b

patrykkopycinski added chore Team:SIEM v8.0.0 release_note:skip Skip the PR/issue when compiling release notes v7.7.0 labels Feb 2, 2020

patrykkopycinski requested review from FrankHassanabad, andrew-goldstein, angorayc and rylnd February 2, 2020 15:20

patrykkopycinski self-assigned this Feb 2, 2020

patrykkopycinski added 5 commits February 2, 2020 16:26

cleanup

fbcf91b

Merge branch 'master' of github.com:elastic/kibana into chore/siem-de…

7999e0e

…ep-equal

cleanup

1b40bf4

Merge branch 'master' of github.com:elastic/kibana into chore/siem-de…

1044ea6

…ep-equal

fix typings

d9f5dd5

FrankHassanabad reviewed Feb 3, 2020

View reviewed changes

patrykkopycinski added 2 commits February 3, 2020 17:12

Merge branch 'master' of github.com:elastic/kibana into chore/siem-de…

68a57e9

…ep-equal

update tests

cadabd6

patrykkopycinski mentioned this pull request Feb 3, 2020

fix: replace PureComponent with shouldComponentUpdate elastic/elastic-charts#534

Merged

5 tasks

XavierM reviewed Feb 5, 2020

View reviewed changes

patrykkopycinski added 3 commits February 6, 2020 01:28

Merge branch 'master' of github.com:elastic/kibana into chore/siem-de…

3544d45

…ep-equal # Conflicts: # x-pack/legacy/plugins/siem/public/pages/detection_engine/components/signals_histogram_panel/index.tsx

cleanup

abe7967

cleanup

a681c58

andrew-goldstein reviewed Feb 6, 2020

View reviewed changes

patrykkopycinski and others added 2 commits February 6, 2020 02:11

Merge branch 'master' into chore/siem-deep-equal

876805e

bump charts

839f609

andrew-goldstein approved these changes Feb 6, 2020

View reviewed changes

Merge branch 'chore/siem-deep-equal' of github.com:patrykkopycinski/k…

aa2dca1

…ibana into chore/siem-deep-equal

patrykkopycinski requested a review from a team as a code owner February 6, 2020 01:12

update tests

3ee3dd8

XavierM approved these changes Feb 6, 2020

View reviewed changes

jbudz approved these changes Feb 24, 2020

View reviewed changes

patrykkopycinski added 3 commits February 25, 2020 09:09

Merge branch 'master' of github.com:elastic/kibana into chore/siem-de…

f041e65

…ep-equal # Conflicts: # x-pack/legacy/plugins/siem/public/components/events_viewer/events_viewer.tsx # x-pack/legacy/plugins/siem/public/components/events_viewer/index.tsx

Update tsconfig.json

92f292e

fix tests

90ab4dd

patrykkopycinski merged commit 418c44a into elastic:master Feb 25, 2020

patrykkopycinski deleted the chore/siem-deep-equal branch February 25, 2020 14:00

patrykkopycinski mentioned this pull request Feb 25, 2020

[7.x] [SIEM] Fix unnecessary re-renders on the Overview page (#56587) #58474

Merged

patrykkopycinski added a commit to patrykkopycinski/kibana that referenced this pull request Feb 25, 2020

[SIEM] Fix unnecessary re-renders on the Overview page (elastic#56587)

bb0a512

kibanamachine added the backport missing Added to PRs automatically when the are determined to be missing a backport. label Feb 26, 2020

patrykkopycinski added a commit that referenced this pull request Feb 26, 2020

[SIEM] Fix unnecessary re-renders on the Overview page (#56587) (#58474)

4c054c4

kibanamachine removed the backport missing Added to PRs automatically when the are determined to be missing a backport. label Feb 26, 2020

Conversation

patrykkopycinski commented Feb 2, 2020

Summary

Checklist

For maintainers

Uh oh!

elasticmachine commented Feb 2, 2020

Uh oh!

Choose a reason for hiding this comment

Uh oh!

FrankHassanabad Feb 3, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

XavierM Feb 5, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

XavierM left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

patrykkopycinski Feb 6, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

andrew-goldstein left a comment

Choose a reason for hiding this comment

Uh oh!

patrykkopycinski commented Feb 6, 2020

Uh oh!

XavierM left a comment

Choose a reason for hiding this comment

Uh oh!

kibanamachine commented Feb 25, 2020

💚 Build Succeeded

History

Uh oh!

kibanamachine commented Feb 26, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

FrankHassanabad Feb 3, 2020 •

edited

Loading

XavierM Feb 5, 2020 •

edited

Loading

patrykkopycinski Feb 6, 2020 •

edited

Loading