perf: use an interstitial page to load popup.html; load scripts using defered script tags#26555
Merged
davidmurdoch merged 4 commits intodevelopfrom Aug 28, 2024
Merged
perf: use an interstitial page to load popup.html; load scripts using defered script tags#26555davidmurdoch merged 4 commits intodevelopfrom
popup.html; load scripts using defered script tags#26555davidmurdoch merged 4 commits intodevelopfrom
Conversation
Contributor
|
CLA Signature Action: All authors have signed the CLA. You may need to manually re-run the blocking PR check if it doesn't pass in a few minutes. |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## develop #26555 +/- ##
===========================================
+ Coverage 69.96% 70.11% +0.15%
===========================================
Files 1405 1412 +7
Lines 48996 49246 +250
Branches 13697 13769 +72
===========================================
+ Hits 34280 34527 +247
- Misses 14716 14719 +3 ☔ View full report in Codecov by Sentry. |
Collaborator
Builds ready [4d2b359]
Page Load Metrics (1655 ± 61 ms)
Bundle size diffs
|
This change drastically improves the performance of the extension but greatly reduces perceived performance because the browser won't display the popup until the DOMContentLoaded event. A work around for the reduction in perceived performance is in a follow up PR. Co-authored-by: Mark Stacey <markjstacey@gmail.com>
…s HTML file, `popup-init.html`, that then redirects to the slower `popup.html` Co-authored-by: Mark Stacey <markjstacey@gmail.com>
4d2b359 to
fcf6a91
Compare
Collaborator
Builds ready [fcf6a91]
Page Load Metrics (1629 ± 52 ms)
Bundle size diffs
|
popup.html; load scripts use defered script tagspopup.html; load scripts using defered script tags
HowardBraham
previously approved these changes
Aug 23, 2024
matthewwalsh0
previously approved these changes
Aug 23, 2024
itsyoboieltr
previously approved these changes
Aug 26, 2024
83f0c20
Firefox is very slow to render the `popup-init.html` redirect, and it renders the page only partially anyway, making the UX feel very janky. It also doesn't delay opening the popup until `DOMContentLoaded`, like chrome does, so the issue `popup-init.html` solves doesn't do anything anyway.
83f0c20 to
22281d7
Compare
|
HowardBraham
approved these changes
Aug 27, 2024
matthewwalsh0
approved these changes
Aug 27, 2024
itsyoboieltr
approved these changes
Aug 28, 2024
Collaborator
Builds ready [22281d7]
Page Load Metrics (2053 ± 149 ms)
Bundle size diffs
|
Collaborator
|
Missing release label release-12.2.0 on PR. Adding release label release-12.2.0 on PR and removing other release labels(release-12.5.0), as PR was cherry-picked in branch 12.2.0. |
This file contains hidden or 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
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
5 participants
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.
This PR removes
load-*.jshelpers and load scripts using thedeferproperty instead.This change drastically improves the performance of the extension but greatly reduces perceived performance because the browser won't display the popup until the DOMContentLoaded event.
But improve perceived performance is mostly regained by initially loading an asset-less HTML file,
popup-init.html, that then redirects to the slower (but much faster than before this PR)popup.html.This was initially authored by @Gudahtt, I've just updated and optimized it to work with the both build systems.
One change I did not make was moving scripts to the
head. I don't think putting the scripts in the head does anything in our case1 other than potentially require that we wait forDOMContentLoadedbefore querying for the app container.Note: In Firefox we continue to use
popup.html. Firefox is very slow to render thepopup-init.htmlredirect, and it renders the page only partially anyway, making the UX feel very janky. It also doesn't delay opening the popup untilDOMContentLoaded, like Chrome does, so the issuepopup-init.htmlsolves doesn't do anything anyway.Here is side-by-side video comparison of
developvspopup-defer-scripts.This video displays the elapsed time from clicking the MetaMask Fox to Lock Screen render.
The end of the video is a frame-by-frame comparison of the new "jank" this PR introduces. The develop branch renders the fox immediately, whereas the popup-defer-scripts branch renders an empty page for about 15 milliseconds.
These stats aren't representative of real world performance, but are intended to illustrate relative performance and perceived performance.
Both builds were created by running
yarn build dist.develop-vs-propup-defer-scripts.mp4
Manual Testing
yarn distCloses #25721
Footnotes
our HTML files are very tiny, and thus will be loaded all in one go, we don't have to worry about packets being lost and retransmitted over the network, delaying the browser's preload scanner. ↩