[SIEM] Upgrades react-redux and utilize React.memo for performance gains #43029

FrankHassanabad · 2019-08-09T15:18:50Z

Summary

Upgrades react-redux so we can use React.memo with hooks and connected components.
Adds displayName to all the components that are React.memo as well as all components that are pure components.

Using React Profiler you can see that when doing something as innocuous as dragging the divider bar of the Timeline you will get updates of the different tables:

This is a result of redux making changes based on the location of the timeline. As this occurs, the UrlState begins updating its self and all children which is not what we want.

You will see differences now in the snapshots such as this which adds naming to it:

For the rest of us profiling we will have friendly names in our profiling tools to be able to trace what is going on very quickly

Before where a name didn't exist and it just showed Memo or pure:

Now that a name exists you can trace to the code quickly to the component name:

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~~

…r for perf improvements

elasticmachine · 2019-08-09T15:19:54Z

Pinging @elastic/siem

andrew-goldstein · 2019-08-09T15:40:48Z

x-pack/legacy/plugins/siem/public/components/url_state/__snapshots__/index.test.tsx.snap

@@ -353,7 +353,7 @@ exports[`UrlStateContainer mounts and renders 1`] = `
                        }
                      }
                    >
-                      <withRouter(Connect(UrlStateContainer))
+                      <withRouter(Connect(Component))


FYI adriantoine/enzyme-to-json#137

Thanks, I just read through it. I don't think it's worth doing the workaround to get the name back, if you're ok with that as sooner or later they will fix the bug.

stephmilovic

Noticeably better! Checked it out in the 3 main browsers, ran tests. Great pairing sesh yall! 🤠 LGTM once CI passes 👍

elasticmachine · 2019-08-09T16:22:42Z

💚 Build Succeeded

continuous-integration/kibana-ci/pull-request

…traces as well as within snapshots

elasticmachine · 2019-08-09T21:55:06Z

💚 Build Succeeded

continuous-integration/kibana-ci/pull-request

andrew-goldstein · 2019-08-09T22:58:01Z

x-pack/legacy/plugins/siem/public/components/page/manage_query.tsx

@@ -36,6 +37,6 @@ export function manageQuery<T>(WrappedComponent: React.ComponentClass<T> | React
      return <WrappedComponent {...otherProps} />;
    }
  }
-
+  ManageQuery.displayName = `ManageQuery (${WrappedComponent.displayName || 'Unknown'})`;


this works really well in the React profiler 👍

andrew-goldstein

Thanks for the perf improvement, and for improving the developer experience when using the profiler! It's waaaaay easier now to read the flame graph when, for example, connected components are rendered.

I verified the UrlStateContainer performance improvement by running the React profiler against master first, and then comparing this PR to the baseline.

LGTM 🚀

elasticmachine · 2019-08-10T01:44:56Z

💚 Build Succeeded

continuous-integration/kibana-ci/pull-request

…ins (elastic#43029) ## Summary * Upgrades react-redux so we can use React.memo with hooks and connected components. * Adds displayName to all the components that are React.memo as well as all components that are `pure` components. Using React Profiler you can see that when doing something as innocuous as dragging the divider bar of the Timeline you will get updates of the different tables: <img width="1547" alt="Screen Shot 2019-08-08 at 1 25 22 PM" src="https://user-images.githubusercontent.com/1151048/62789872-19efad80-ba87-11e9-92b5-eb9666c7c07a.png"> This is a result of redux making changes based on the location of the timeline. As this occurs, the UrlState begins updating its self and all children which is not what we want. You will see differences now in the snapshots such as this which adds naming to it: <img width="1582" alt="Screen Shot 2019-08-09 at 2 34 06 PM" src="https://user-images.githubusercontent.com/1151048/62808504-7a95df00-bab5-11e9-89c1-910b3908c705.png"> For the rest of us profiling we will have friendly names in our profiling tools to be able to trace what is going on very quickly Before where a name didn't exist and it just showed `Memo` or `pure`: <img width="221" alt="Screen Shot 2019-08-09 at 11 55 08 AM" src="https://user-images.githubusercontent.com/1151048/62809192-79fe4800-bab7-11e9-993b-2e7abadbb691.png"> Now that a name exists you can trace to the code quickly to the component name: <img width="1570" alt="Screen Shot 2019-08-09 at 3 08 42 PM" src="https://user-images.githubusercontent.com/1151048/62809235-9a2e0700-bab7-11e9-8711-bae7d263d487.png"> ### 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](https://github.com/elastic/kibana/blob/master/CONTRIBUTING.md#cross-browser-compatibility)~~ ~~- [ ] Any text added follows [EUI's writing guidelines](https://elastic.github.io/eui/#/guidelines/writing), uses sentence case text and includes [i18n support](https://github.com/elastic/kibana/blob/master/packages/kbn-i18n/README.md)~~ ~~- [ ] [Documentation](https://github.com/elastic/kibana/blob/master/CONTRIBUTING.md#writing-documentation) was added for features that require explanation or tutorials~~ ~~- [ ] [Unit or functional tests](https://github.com/elastic/kibana/blob/master/CONTRIBUTING.md#cross-browser-compatibility) were updated or added to match the most common scenarios~~ ~~- [ ] This was checked for [keyboard-only and screenreader accessibility](https://developer.mozilla.org/en-US/docs/Learn/Tools_and_testing/Cross_browser_testing/Accessibility#Accessibility_testing_checklist)~~ ### For maintainers ~~- [ ] This was checked for breaking API changes and was [labeled appropriately](https://github.com/elastic/kibana/blob/master/CONTRIBUTING.md#release-notes-process)~~ ~~- [ ] This includes a feature addition or change that requires a release note and was [labeled appropriately](https://github.com/elastic/kibana/blob/master/CONTRIBUTING.md#release-notes-process)~~

…ins (#43029) (#43070) ## Summary * Upgrades react-redux so we can use React.memo with hooks and connected components. * Adds displayName to all the components that are React.memo as well as all components that are `pure` components. Using React Profiler you can see that when doing something as innocuous as dragging the divider bar of the Timeline you will get updates of the different tables: <img width="1547" alt="Screen Shot 2019-08-08 at 1 25 22 PM" src="https://user-images.githubusercontent.com/1151048/62789872-19efad80-ba87-11e9-92b5-eb9666c7c07a.png"> This is a result of redux making changes based on the location of the timeline. As this occurs, the UrlState begins updating its self and all children which is not what we want. You will see differences now in the snapshots such as this which adds naming to it: <img width="1582" alt="Screen Shot 2019-08-09 at 2 34 06 PM" src="https://user-images.githubusercontent.com/1151048/62808504-7a95df00-bab5-11e9-89c1-910b3908c705.png"> For the rest of us profiling we will have friendly names in our profiling tools to be able to trace what is going on very quickly Before where a name didn't exist and it just showed `Memo` or `pure`: <img width="221" alt="Screen Shot 2019-08-09 at 11 55 08 AM" src="https://user-images.githubusercontent.com/1151048/62809192-79fe4800-bab7-11e9-993b-2e7abadbb691.png"> Now that a name exists you can trace to the code quickly to the component name: <img width="1570" alt="Screen Shot 2019-08-09 at 3 08 42 PM" src="https://user-images.githubusercontent.com/1151048/62809235-9a2e0700-bab7-11e9-8711-bae7d263d487.png"> ### 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](https://github.com/elastic/kibana/blob/master/CONTRIBUTING.md#cross-browser-compatibility)~~ ~~- [ ] Any text added follows [EUI's writing guidelines](https://elastic.github.io/eui/#/guidelines/writing), uses sentence case text and includes [i18n support](https://github.com/elastic/kibana/blob/master/packages/kbn-i18n/README.md)~~ ~~- [ ] [Documentation](https://github.com/elastic/kibana/blob/master/CONTRIBUTING.md#writing-documentation) was added for features that require explanation or tutorials~~ ~~- [ ] [Unit or functional tests](https://github.com/elastic/kibana/blob/master/CONTRIBUTING.md#cross-browser-compatibility) were updated or added to match the most common scenarios~~ ~~- [ ] This was checked for [keyboard-only and screenreader accessibility](https://developer.mozilla.org/en-US/docs/Learn/Tools_and_testing/Cross_browser_testing/Accessibility#Accessibility_testing_checklist)~~ ### For maintainers ~~- [ ] This was checked for breaking API changes and was [labeled appropriately](https://github.com/elastic/kibana/blob/master/CONTRIBUTING.md#release-notes-process)~~ ~~- [ ] This includes a feature addition or change that requires a release note and was [labeled appropriately](https://github.com/elastic/kibana/blob/master/CONTRIBUTING.md#release-notes-process)~~

…p-metrics-selectall * 'master' of github.com:elastic/kibana: (22 commits) [Code]: downgrade the log level of error message from subprocess (elastic#42925) [Code] Cancel clone/update job in the middle if disk space over the watermark (elastic#42890) Add Kibana App specific URL to the help menu (elastic#34739) (elastic#42580) [Maps] refactor createShapeFilterWithMeta to support more than just polygons (elastic#43042) Skip flaky es_ui_shared/request tests. Pass uiSettings to all data plugin services (elastic#42159) [SIEM] Upgrades react-redux and utilize React.memo for performance gains (elastic#43029) [skip-ci][Maps] add maki icon sheet to docs (elastic#43063) Adding "style-src 'unsafe-inline' 'self'" to default CSP rules (elastic#41305) Update dependency commander to v3 (elastic#43041) Update dependency @percy/agent to ^0.10.0 (elastic#40517) [Maps] only show top hits checkbox if index has date fields (elastic#43056) run chained_controls on Firefox to catch regression (elastic#43044) fixing issue with dashboard csv download (elastic#42964) Expose task manager as plugin instead of server argument (elastic#42966) Expose createRouter from HttpService, prepare handlers for context introduction (elastic#42686) [Code] disk watermark supports percentage and absolute modes (elastic#42987) [apps/dashboard] skip part of filtering tests on FF (elastic#43047) [ML] Kibana management jobs list (elastic#42570) [ML] Fix check for watcher being enabled (elastic#43025) ...

Upgrades our react-redux and uses React.memo for the UrlStateContaine…

c2eb8f9

…r for perf improvements

FrankHassanabad added the release_note:enhancement label Aug 9, 2019

FrankHassanabad changed the title ~~[SIEM] Upgrade react-redux and utilize React.memo for performance gains~~ [SIEM] Upgrades react-redux and utilize React.memo for performance gains Aug 9, 2019

FrankHassanabad added v7.4.0 v8.0.0 Team:SIEM labels Aug 9, 2019

FrankHassanabad self-assigned this Aug 9, 2019

andrew-goldstein reviewed Aug 9, 2019

View reviewed changes

stephmilovic reviewed Aug 9, 2019

View reviewed changes

stephmilovic approved these changes Aug 9, 2019

View reviewed changes

Added displayName to all the components so we can read them in stack …

f6ee3d4

…traces as well as within snapshots

andrew-goldstein reviewed Aug 9, 2019

View reviewed changes

andrew-goldstein approved these changes Aug 9, 2019

View reviewed changes

FrankHassanabad added 2 commits August 9, 2019 18:02

Added more displayName

7c95053

More displayName settings for pure

a6f1292

FrankHassanabad merged commit fe3422c into elastic:master Aug 10, 2019

FrankHassanabad deleted the use-memo-url-state branch August 10, 2019 01:46

FrankHassanabad mentioned this pull request Aug 10, 2019

[7.x] [SIEM] Upgrades react-redux and utilize React.memo for performance gains (#43029) #43070

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[SIEM] Upgrades react-redux and utilize React.memo for performance gains #43029

[SIEM] Upgrades react-redux and utilize React.memo for performance gains #43029

FrankHassanabad commented Aug 9, 2019 •

edited

elasticmachine commented Aug 9, 2019

andrew-goldstein Aug 9, 2019

FrankHassanabad Aug 9, 2019

stephmilovic left a comment

elasticmachine commented Aug 9, 2019

elasticmachine commented Aug 9, 2019

andrew-goldstein Aug 9, 2019

andrew-goldstein left a comment

elasticmachine commented Aug 10, 2019

[SIEM] Upgrades react-redux and utilize React.memo for performance gains #43029

[SIEM] Upgrades react-redux and utilize React.memo for performance gains #43029

Conversation

FrankHassanabad commented Aug 9, 2019 • edited

Summary

Checklist

For maintainers

elasticmachine commented Aug 9, 2019

andrew-goldstein Aug 9, 2019

Choose a reason for hiding this comment

FrankHassanabad Aug 9, 2019

Choose a reason for hiding this comment

stephmilovic left a comment

Choose a reason for hiding this comment

elasticmachine commented Aug 9, 2019

💚 Build Succeeded

elasticmachine commented Aug 9, 2019

💚 Build Succeeded

andrew-goldstein Aug 9, 2019

Choose a reason for hiding this comment

andrew-goldstein left a comment

Choose a reason for hiding this comment

elasticmachine commented Aug 10, 2019

💚 Build Succeeded

FrankHassanabad commented Aug 9, 2019 •

edited