perf(dashboard): Improve performance of complex dashboards #19064

kgabryje · 2022-03-08T13:33:21Z

SUMMARY

This PR improves the performance of dashboards with complex structure (multiple tabs, many levels of nesting, many charts, native filters).
The main culprit of poor performance was a function used for finding charts in filter's scope. getChartIdsInFilterScope was written in a very suboptimal way, which drastically slowed down loading charts - which was especially noticeable when opening a dashboard or switching tabs.
I've rewritten that function and made a few other improvements, which resulted in a much smoother experience.
The scope of the changes in this PR is related only to native filters and cross filters. Performance issues related to filter box were not addressed, as the filter box will soon be deprecated in favor of native filters.

The tests were performed on a dashboard with 100+ charts, 70+ tabs, up to 4 levels of nested tabs, up to 10 charts in a single tab, 9 native filters.

Dashboard used for testing: dashboard_export_20220309T112451.zip

BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF

Before:

Screen.Recording.2022-03-08.at.14.07.50.mov

After:

Screen.Recording.2022-03-08.at.14.06.22.mov

TESTING INSTRUCTIONS

Verify that building dashboards works like before
Verify that applying native filters works like before
Verify that applying cross filters works like before
Verify that applying filters from filter box works like before

ADDITIONAL INFORMATION

Has associated issue:
Required feature flags:
Changes UI
Includes DB Migration (follow approval process in SIP-59)
- Migration is atomic, supports rollback & is backwards-compatible
- Confirm DB migration upgrade and downgrade tested
- Runtime estimates and downtime expectations provided
Introduces new feature or API
Removes existing feature or API

https://app.shortcut.com/preset/story/38683/performance-issues-to-navigate-between-tabs-depending-on-the-amount-of-charts-nested-tabs-on-a-dashboard

codecov · 2022-03-08T18:19:46Z

Codecov Report

Merging #19064 (37008db) into master (f9c7405) will increase coverage by 0.00%.
The diff coverage is 45.00%.

❗ Current head 37008db differs from pull request most recent head f9ea592. Consider uploading reports for the commit f9ea592 to get more accurate results

@@           Coverage Diff           @@
##           master   #19064   +/-   ##
=======================================
  Coverage   66.50%   66.50%           
=======================================
  Files        1642     1644    +2     
  Lines       63442    63472   +30     
  Branches     6439     6452   +13     
=======================================
+ Hits        42191    42213   +22     
- Misses      19582    19589    +7     
- Partials     1669     1670    +1

Flag	Coverage Δ
javascript	`51.23% <40.00%> (+<0.01%)`	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files	Coverage Δ
...perset-frontend/src/addSlice/AddSliceContainer.tsx	`61.76% <ø> (ø)`
...ard/components/filterscope/FilterScopeSelector.jsx	`79.20% <0.00%> (ø)`
...ashboard/components/gridComponents/ChartHolder.jsx	`58.82% <ø> (ø)`
...end/src/dashboard/util/filterboxMigrationHelper.ts	`39.20% <0.00%> (ø)`
...end/src/dashboard/util/getChartIdsInFilterScope.ts	`0.00% <0.00%> (ø)`
...rontend/src/visualizations/TimeTable/TimeTable.jsx	`0.00% <0.00%> (ø)`
superset/databases/commands/create.py	`92.15% <ø> (+3.92%)`	⬆️
...src/dashboard/components/FiltersBadge/selectors.ts	`68.46% <14.28%> (-2.18%)`	⬇️
superset-frontend/src/views/CRUD/hooks.ts	`47.15% <33.33%> (ø)`
...frontend/src/SqlLab/components/SqlEditor/index.jsx	`50.84% <40.00%> (-0.60%)`	⬇️
... and 8 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update f9c7405...f9ea592. Read the comment docs.

rusackas · 2022-03-08T21:53:53Z

/testenv up

github-actions · 2022-03-08T21:55:46Z

@rusackas Ephemeral environment spinning up at http://34.209.37.190:8080. Credentials are admin/admin. Please allow several minutes for bootstrapping and startup.

rusackas · 2022-03-08T21:56:52Z

@graceguo-supercat is going to LOVE this PR 🚀

@kgabryje could you please export your test dashboard (if it uses sample data) and attach it here* and/or import it into the above ephemeral environment when it spins up?

*Fragility caveat that the import might get nuked with the ephemeral env, thus the request to upload/link here ;)

villebro

This is incredible work, a major achievement in improving dashboard performance! 👏 Changes look very straight forward and would actually serve as a great example for frontend development guidelines (use custom hooks to encapsulate complex state logic, memoize heavy computation and in the process avoid the need for JSON.stringify when referencing the results in dependency arrays etc). Also, not a blocker, but it would be great if getChartIdsInFilterScope could be converted to TS.

superset-frontend/src/dashboard/components/DashboardBuilder/DashboardContainer.tsx

superset-frontend/src/dashboard/util/activeDashboardFilters.js

jinghua-qa · 2022-03-09T08:34:06Z

@graceguo-supercat is going to LOVE this PR 🚀

@kgabryje could you please export your test dashboard (if it uses sample data) and attach it here* and/or import it into the above ephemeral environment when it spins up?

*Fragility caveat that the import might get nuked with the ephemeral env, thus the request to upload/link here ;)

Would be helpful if the perf test dashboard can be upload, currently the import dashboard feature seems not enable in the ephemeral env and i can not import the test dashboard

graceguo-supercat · 2022-03-09T08:46:04Z

@kgabryje Thank you for the perf improvement! just 1 question here: I saw both getChartIdsInFilterBoxScope and getChartIdsInFilterScope functions. So when to use each of them?

kgabryje · 2022-03-09T10:43:32Z

@kgabryje Thank you for the perf improvement! just 1 question here: I saw both getChartIdsInFilterBoxScope and getChartIdsInFilterScope functions. So when to use each of them?

@graceguo-supercat I kept getChartIdsInFilterBoxScope for legacy reasons. Refactoring the logic related to filter box would be rather risky - I decided not to do it since we're on the road of deprecating filter box in favour in native filters. I added a comment to getChartIdsInFilterBoxScope that redirects to the new getChartIdsInFilterScope.
So going forward, we should use only getChartIdsInFilterScope

kgabryje · 2022-03-09T10:44:48Z

@rusackas @jinghua-qa I added exported dashboard to the PR description 🙂 I'll spin up a new test env with VERSIONED_EXPORT enabled once docker builds

kgabryje · 2022-03-09T11:09:48Z

/testenv up FEATURE_VERSIONED_EXPORT=true FEATURE_DASHBOARD_CROSS_FILTERS=true

github-actions · 2022-03-09T11:14:11Z

@kgabryje Ephemeral environment spinning up at http://54.191.63.77:8080. Credentials are admin/admin. Please allow several minutes for bootstrapping and startup.

michael-s-molina

LGTM. Thanks for the awesome work @kgabryje. I left some non-blocking comments.

Given the critical nature of the components, I kindly ask that you wait for @jinghua-qa's approval before merging.

superset-frontend/src/dashboard/components/DashboardBuilder/DashboardContainer.tsx

superset-frontend/src/dashboard/components/FiltersBadge/selectors.ts

kgabryje · 2022-03-09T16:19:46Z

@kgabryje Ephemeral environment spinning up at http://54.191.63.77:8080. Credentials are admin/admin. Please allow several minutes for bootstrapping and startup.

@jinghua-qa This env is still up to date, the last changes were non-functional

rusackas

LGTM, and feels super snappy in testing. This PR is such a huge improvement!!

jinghua-qa

LGTM

github-actions · 2022-03-09T21:40:19Z

Ephemeral environment shutdown and build artifacts deleted.

) * perf(dashboard): Improve performance of filter indicators * Improve perf of cross filters * Rename old function * Fix undefined * fix type * fix tests * fix undefined * Address code review comments * Address code review comments

sadpandajoe · 2022-03-14T16:42:16Z

🏷️ preset:2022.9

* perf(dashboard): Improve performance of filter indicators * Improve perf of cross filters * Rename old function * Fix undefined * fix type * fix tests * fix undefined * Address code review comments * Address code review comments (cherry picked from commit 3c1fb94)

kgabryje added the need:qa-review Requires QA review label Mar 8, 2022

kgabryje requested review from rusackas, graceguo-supercat, villebro, geido, michael-s-molina and jinghua-qa March 8, 2022 13:33

pull-request-size bot added the size/L label Mar 8, 2022

kgabryje added 7 commits March 8, 2022 20:44

perf(dashboard): Improve performance of filter indicators

cff9bb9

Improve perf of cross filters

c5b416d

Rename old function

1c44345

Fix undefined

4221fbe

fix type

3973a14

fix tests

e4862bd

fix undefined

9ede2d3

kgabryje force-pushed the perf/dashboard-filters branch from 569e349 to 9ede2d3 Compare March 8, 2022 19:44

villebro approved these changes Mar 9, 2022

View reviewed changes

superset-frontend/src/dashboard/components/DashboardBuilder/DashboardContainer.tsx Outdated Show resolved Hide resolved

superset-frontend/src/dashboard/util/activeDashboardFilters.js Outdated Show resolved Hide resolved

Address code review comments

339b537

michael-s-molina approved these changes Mar 9, 2022

View reviewed changes

Address code review comments

f9ea592

kgabryje force-pushed the perf/dashboard-filters branch from 7a74e16 to f9ea592 Compare March 9, 2022 12:57

rusackas approved these changes Mar 9, 2022

View reviewed changes

villebro mentioned this pull request Mar 9, 2022

fix(dashboard): import handle missing excluded charts #19088

Merged

9 tasks

jinghua-qa approved these changes Mar 9, 2022

View reviewed changes

kgabryje merged commit 3c1fb94 into apache:master Mar 9, 2022

superset-github-bot bot added the preset:2022.9 label Mar 14, 2022

kgabryje added the lts-v1 label Mar 22, 2022

mistercrunch added 🍒 1.5.0 🍒 1.5.1 🍒 1.5.2 🍒 1.5.3 🏷️ bot A label used by `supersetbot` to keep track of which PR where auto-tagged with release labels 🚢 2.0.0 labels Mar 13, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

perf(dashboard): Improve performance of complex dashboards #19064

perf(dashboard): Improve performance of complex dashboards #19064

kgabryje commented Mar 8, 2022 •

edited

Loading

codecov bot commented Mar 8, 2022 •

edited

Loading

rusackas commented Mar 8, 2022

github-actions bot commented Mar 8, 2022

rusackas commented Mar 8, 2022

villebro left a comment

jinghua-qa commented Mar 9, 2022

graceguo-supercat commented Mar 9, 2022 •

edited

Loading

kgabryje commented Mar 9, 2022 •

edited

Loading

kgabryje commented Mar 9, 2022

kgabryje commented Mar 9, 2022

github-actions bot commented Mar 9, 2022

michael-s-molina left a comment

kgabryje commented Mar 9, 2022

rusackas left a comment

jinghua-qa left a comment

github-actions bot commented Mar 9, 2022

sadpandajoe commented Mar 14, 2022

perf(dashboard): Improve performance of complex dashboards #19064

perf(dashboard): Improve performance of complex dashboards #19064

Conversation

kgabryje commented Mar 8, 2022 • edited Loading

SUMMARY

BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF

TESTING INSTRUCTIONS

ADDITIONAL INFORMATION

codecov bot commented Mar 8, 2022 • edited Loading

Codecov Report

rusackas commented Mar 8, 2022

github-actions bot commented Mar 8, 2022

rusackas commented Mar 8, 2022

villebro left a comment

Choose a reason for hiding this comment

jinghua-qa commented Mar 9, 2022

graceguo-supercat commented Mar 9, 2022 • edited Loading

kgabryje commented Mar 9, 2022 • edited Loading

kgabryje commented Mar 9, 2022

kgabryje commented Mar 9, 2022

github-actions bot commented Mar 9, 2022

michael-s-molina left a comment

Choose a reason for hiding this comment

kgabryje commented Mar 9, 2022

rusackas left a comment

Choose a reason for hiding this comment

jinghua-qa left a comment

Choose a reason for hiding this comment

github-actions bot commented Mar 9, 2022

sadpandajoe commented Mar 14, 2022

kgabryje commented Mar 8, 2022 •

edited

Loading

codecov bot commented Mar 8, 2022 •

edited

Loading

graceguo-supercat commented Mar 9, 2022 •

edited

Loading

kgabryje commented Mar 9, 2022 •

edited

Loading