perf: use redux state patches #26738
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add patch store.
github-actions
bot
added
the
team-confirmations
Support raw state update. Fix decrypt test.
Increase driver timeout.
Builds ready [1e27752]
Page Load Metrics (1675 ± 82 ms)
Bundle size diffs [🚨 Warning! Bundle size has increased!]
|
Builds ready [3c4e8ae]
Page Load Metrics (1767 ± 80 ms)
Bundle size diffs [🚨 Warning! Bundle size has increased!]
|
matthewwalsh0
added
the
team-tiger
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## develop #26738 +/- ##
===========================================
- Coverage 70.02% 69.99% -0.04%
===========================================
Files 1443 1445 +2
Lines 50164 50226 +62
Branches 14039 14045 +6
===========================================
+ Hits 35126 35151 +25
- Misses 15038 15075 +37 ☔ View full report in Codecov by Sentry. |
Builds ready [b5f22e8]
Page Load Metrics (1741 ± 99 ms)
Bundle size diffs [🚨 Warning! Bundle size has increased!]
|
Builds ready [b5968fe]
Page Load Metrics (1700 ± 74 ms)
Bundle size diffs [🚨 Warning! Bundle size has increased!]
|
|
Builds ready [54bbdf5]
Page Load Metrics (1848 ± 148 ms)
Bundle size diffs [🚨 Warning! Bundle size has increased!]
|
cryptotavares
previously approved these changes
Sep 18, 2024
Comment on lines
+5613
to
+5614
| * Intentionally not using immer as a temporary measure to avoid | ||
| * freezing the resulting state and requiring further fixes |
There was a problem hiding this comment.
was just thinking about why are we not using immer for applying the patches... I guess this explains it 😅
|
cryptotavares
approved these changes
Sep 19, 2024
pedronfigueiredo
approved these changes
Sep 19, 2024
Builds ready [86ab71d]
Page Load Metrics (1935 ± 116 ms)
Bundle size diffs [🚨 Warning! Bundle size has increased!]
|
metamaskbot
added
the
release-12.6.0
metamaskbot
added
release-12.5.0
|
Missing release label release-12.5.0 on PR. Adding release label release-12.5.0 on PR and removing other release labels(release-12.6.0), as PR was added to branch 12.5.0 when release was cut. |
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
Currently, the background page / service worker subscribes to state changes from every controller and on each state change (debounced to 200ms), sends the full flattened state to the UI where it is stored in the
metamaskduck.The primary drawback of this is that Redux and React default to shallow equality comparisons meaning any object and array state is always detected as new, resulting in frequent unnecessary renders unless deep equality is explicitly used through mechanisms such as
createDeepEqualSelector.This PR aims to resolve this by instead sending state patches to the UI, and only for the top level state properties that no longer have shallow equality. These patches are then applied to the MetaMask duck in the UI, meaning it is now incrementally updated and any object and array references are only updated when the data itself has changed.
This logic is encapsulated within the new
PatchStoreclass, with an alternate instance being created for each UI connection.The patches are currently intentionally generated and applied without
immergiven this has the side effect of freezing the associated state, which triggers multiple errors due to pre-existing instances of direct state mutation in the background and UI code.Related issues
Fixes: #2987
Manual testing steps
No functional changes, but high level sanity checks would be beneficial given risk.
Screenshots/Recordings
Before
After
Pre-merge author checklist
Pre-merge reviewer checklist