Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: improve performance in account list and account connection components #23933

Merged
merged 5 commits into from
Apr 16, 2024

Conversation

FrederikBolding
Copy link
Member

@FrederikBolding FrederikBolding commented Apr 10, 2024

Description

Improves performance in the account list and "account connection" components by no longer leveraging getAddressConnectedSubjectMap. The selector in question is very computationally expensive to use and thus were causing performance problems for users with many accounts and many connected sites. This PR proposes using getPermittedAccountsForCurrentTab where possible and also introduces a memoized selector isAccountConnectedToCurrentTab that performantly can check for a given address.

From local testing this seems to improve the experience.

Open in GitHub Codespaces

Related issues

Fixes #23913

@metamaskbot metamaskbot added the INVALID-PR-TEMPLATE PR's body doesn't match template label Apr 10, 2024
@FrederikBolding FrederikBolding force-pushed the fb/improve-account-connection-perf branch from 00b7bdd to acded48 Compare April 10, 2024 11:54
Copy link

codecov bot commented Apr 10, 2024

Codecov Report

Attention: Patch coverage is 46.15385% with 7 lines in your changes are missing coverage. Please review.

Project coverage is 67.52%. Comparing base (d15cc54) to head (a6e4dfc).
Report is 60 commits behind head on develop.

Files Patch % Lines
...ted-status-indicator/connected-status-indicator.js 0.00% 4 Missing ⚠️
...s/multichain/connected-status/connected-status.tsx 0.00% 2 Missing ⚠️
ui/selectors/permissions.js 80.00% 1 Missing ⚠️
Additional details and impacted files
@@             Coverage Diff             @@
##           develop   #23933      +/-   ##
===========================================
- Coverage    67.54%   67.52%   -0.02%     
===========================================
  Files         1245     1245              
  Lines        48864    48869       +5     
  Branches     12744    12742       -2     
===========================================
- Hits         33003    32995       -8     
- Misses       15861    15874      +13     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@metamaskbot
Copy link
Collaborator

Builds ready [e80f64e]
Page Load Metrics (1550 ± 513 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint743611618541
domContentLoaded1091302010
load60267115501068513
domInteractive1091302010
Bundle size diffs [🚨 Warning! Bundle size has increased!]
  • background: 0 Bytes (0.00%)
  • ui: -236 Bytes (-0.00%)
  • common: 145 Bytes (0.00%)

@NidhiKJha NidhiKJha self-assigned this Apr 12, 2024
@metamaskbot
Copy link
Collaborator

Builds ready [a6e4dfc]
Page Load Metrics (1465 ± 542 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint6355717811857
domContentLoaded105026126
load51275914651129542
domInteractive105026126
Bundle size diffs [🚨 Warning! Bundle size has increased!]
  • background: 0 Bytes (0.00%)
  • ui: -236 Bytes (-0.00%)
  • common: 145 Bytes (0.00%)

@NidhiKJha NidhiKJha marked this pull request as ready for review April 15, 2024 08:36
@NidhiKJha NidhiKJha requested review from a team as code owners April 15, 2024 08:36
Copy link
Member

@NidhiKJha NidhiKJha left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM! Verified locally, everything works

@NidhiKJha NidhiKJha added the needs-assets-ux-review A shared label between the Assets and UX team to flag PRs ready for consolidated team review. label Apr 15, 2024
@NidhiKJha NidhiKJha merged commit d40321c into develop Apr 16, 2024
68 of 70 checks passed
@NidhiKJha NidhiKJha deleted the fb/improve-account-connection-perf branch April 16, 2024 15:02
@github-actions github-actions bot locked and limited conversation to collaborators Apr 16, 2024
@metamaskbot metamaskbot added the release-11.16.0 Issue or pull request that will be included in release 11.16.0 label Apr 16, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
INVALID-PR-TEMPLATE PR's body doesn't match template needs-assets-ux-review A shared label between the Assets and UX team to flag PRs ready for consolidated team review. release-11.16.0 Issue or pull request that will be included in release 11.16.0 team-wallet-ux
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Bug]: Clicking the Accounts list has a serious delay in response.
5 participants