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

Misc Jetpack React performance enhancements #4469

Merged
merged 5 commits into from
Jul 22, 2016

Conversation

gravityrail
Copy link
Contributor

@gravityrail gravityrail commented Jul 20, 2016

A branch where I'm experimenting with performance issues with Jetpack React.

Baseline:

Loading the Jetpack dashboard takes a pretty long time. On my dev environment, Jetpack dashboard transfers 3.6MB of data, issues 82 DB requests, 125 individual asset requests, 6 back-end API requests to WPCOM, takes ~6-8 seconds of back-end time and and takes 20 - 25 seconds to load on a moderate broadband connection. These stats don't even include back-end time spent within individual API requests to the site.

Things in this branch:

  • Change timestamp to version number for admin.js and other dashboard assets. Developers who want these assets refreshed can just disable caching. Saves: ~3mb per page load.
  • Allow storing NULL response for post_by_email_address, so that we don't try to retrieve this value from WPCOM on every page load. Saves: 1 WPCOM request on every dashboard load (for users without post by email address).
  • Cache the output of Jetpack::get_connected_user_data( $user_id ) for 1 day (saves 1 WPCOM request for every dashboard load)
  • In-progress: Load stats via an API call rather than inline during page load. Despite caching of 5 minutes for stats, this basically incurs 4-5 additional WPCOM requests on every load of the Jetpack dashboard, since I'm guessing most times people come back to the dashboard after more than 5 minutes has passed.

WIP results:

Before After
Remote requests 2-6 0 - 6 (mostly 0!)
Download size 3.6 MB 0.424 MB
Back-end time 8s 0.6s

The upside of all this: For regular users, this should see the load time of the dashboard drop from over 10 seconds to 1-2 seconds, particularly if their cache is primed.

@gravityrail gravityrail changed the title Use JETPACK__VERSION instead of time() to allow JS to be cached Misc Jetpack React performance enhancements Jul 20, 2016
@gravityrail gravityrail added this to the 4.3 milestone Jul 20, 2016
@gravityrail
Copy link
Contributor Author

@eliorivero - as discussed, could you please finish the work I started in 9b42d1a to fetch stats from the API after page load? This will save 4 calls to WPCOM from most requests.

Separately, we should look at making fewer requests during that call, either by combining the 4 requests into 1 or by only fetching the data that's being actively viewed (e.g. just the daily, monthly or yearly stats).

@jeherve jeherve added the Admin Page React-powered dashboard under the Jetpack menu label Jul 21, 2016
@eliorivero
Copy link
Contributor

@gravityrail I've updated the PR with the commit to make Stats data load after page load. It will be cached once it's correctly fetch in the JS object we use to store initial data so when user switches tabs and backs to view Stats they don't have to be fetch again.

On page load, the property Initial_Status.stats.data is initially set to false.
Once the Stats data are properly fetch, it's populated and cached for later use.
@gravityrail
Copy link
Contributor Author

@eliorivero works great! A couple of things:

  • probably should have a spinner state (didn't see one) while it's loading
  • might be worth setting the height of the stats area while it's loading so the page layout doesn't jump downwards once it's loaded

What do you think? Should we merge this and address those in a separate PR?

@eliorivero
Copy link
Contributor

@gravityrail we need to introduce skeleton loading in many views that load remote data so we'll surely do that in a separate PR. When the skeleton loading for Stats is introduced, it will take the height of the stats area so that will be addressed too.

@eliorivero eliorivero added [Status] Needs Review To request a review from Crew. Label will be renamed soon. and removed [Status] In Progress labels Jul 21, 2016
@gravityrail gravityrail added [Status] Ready to Merge Go ahead, you can push that green button! and removed [Status] Needs Review To request a review from Crew. Label will be renamed soon. labels Jul 22, 2016
@gravityrail gravityrail merged commit c08f051 into master Jul 22, 2016
@gravityrail gravityrail deleted the fix/performance-with-jetpack-react branch July 22, 2016 18:17
@kraftbj kraftbj removed the [Status] Ready to Merge Go ahead, you can push that green button! label Oct 13, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Admin Page React-powered dashboard under the Jetpack menu [Focus] Performance
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants