mitodl / micromasters

Portal for learners and course teams to access MITx Micromasters® programs
https://mm.mit.edu
BSD 3-Clause "New" or "Revised" License
30 stars 17 forks source link

Investigate slow page loads #2163

Open pdpinch opened 7 years ago

pdpinch commented 7 years ago

get a snapshot of performance now and then look into why it is so slow.

alicewriteswrongs commented 7 years ago

I wrote a patch which can be used to get a production environment running locally (minified builds and so on). This is more useful for performance testing and getting relevant numbers than a development build - the patch is available here: https://github.com/aliceriot/mm_patches/blob/master/mock_production_mode.patch

I'm trying to use this to get some baseline numbers about our application's performance (mainly looking at the dashboard code right now) using the chrome 'Timeline' developer feature.

It looks like, on the dashboard, all of our JavaScript and CSS is done loading by about 700ms or so, and then we get first paint (anything rendered on the screen) 1740ms, which is a pretty long time. The interim looks to be mainly occupied with evaluating the dashboard.js bundle.

One thing we might try is these two webpack plugins:

https://babeljs.io/docs/plugins/transform-react-inline-elements/ https://babeljs.io/docs/plugins/transform-react-constant-elements/

I'm going to install them and see if there's a difference.

alicewriteswrongs commented 7 years ago

I wanted to get some more reliable numbers for the time until the document is loaded. Chrome makes it easy to get this information in the timeline tool.

on master, with just the mock-production patch applied:

time to 'loaded' event (seconds)
2.5
2.75
2.37
2.43
2.5
2.83
2.58
3.04
2.39
2.55
average: 2.59

This gives us a baseline we can compare things to.

alicewriteswrongs commented 7 years ago

First I wanted to investigate the effect of adding the two babel transforms I mentioned above. Here are some load times, on a branch where those plugins are added and configured, and the mock-production patch has been applied:

time to 'loaded' event (seconds)
2.99
2.71
2.85
2.86
2.93
3.04
2.46
2.57
2.51
2.53
average: 2.745

so that's not an improvement! darn.

probably this isn't statistically significant - the chrome profiler has options for CPU throttling to simulate slower devices. I'm going to run another set of tests with a CPU throttling on.

alicewriteswrongs commented 7 years ago

time to loaded event with chrome's 'low end device' throttling enabled:

master babel react optimizations
6.84 6.09
8.18 7.60
7.28 6.64
8.24 7.61
7.33 6.59
7.57 6.33
7.03 6.70
7.04 6.73
7.14 6.48
8.37 6.69
average 7.502 6.746

I think that demonstrates there's some gain to those babel plugins, I'll open a PR to add them.

alicewriteswrongs commented 7 years ago

I wanted to also investigate switching to webpack 2. Here's a quick comparison of the output of a production build.

master:

total 3.4M
drwxr-xr-x 2 alice users  12K Jan  3 16:20 ./
drwxr-xr-x 6 alice users 4.0K May 24  2016 ../
-rw-r--r-- 1 alice  1000 1.4M Jan  3 16:19 common-394f898fa507c27070fd.js
-rw-r--r-- 1 alice  1000 1.2M Jan  3 16:19 dashboard-8aa4f6165519a2a40098.js
-rw-r--r-- 1 alice  1000 2.6K Jan  3 16:19 financial_aid-e3391cbebde3187e1184.js
-rw-r--r-- 1 alice  1000  60K Jan  3 16:19 public-e2c9b0fad874149be5a3.js
-rw-r--r-- 1 alice  1000  25K Jan  3 16:19 sentry_client-793a74458dc40169dcc8.js
-rw-r--r-- 1 alice  1000 664K Jan  3 16:19 style-2f29252d70b5af881293.js
-rw-r--r-- 1 alice  1000 137K Jan  3 16:19 style_public-1ae82f1c5ed0532221b2.js
-rw-r--r-- 1 alice  1000 4.1K Jan  3 16:19 zendesk_widget-94d83c7c5b6da6e78b1e.js

on my webpack 2 branch (https://github.com/mitodl/micromasters/compare/ap/webpack-2):

total 3.5M
drwxr-xr-x 2 alice users  12K Jan  3 16:16 ./
drwxr-xr-x 6 alice users 4.0K May 24  2016 ../
-rw-r--r-- 1 alice  1000 1.4M Jan  3 16:14 common-96c220a057b057f357ee.js
-rw-r--r-- 1 alice  1000 1.2M Jan  3 16:14 dashboard-26cb629f7c535a17a516.js
-rw-r--r-- 1 alice  1000 2.6K Jan  3 16:14 financial_aid-acabd8f28770984827b4.js
-rw-r--r-- 1 alice  1000  59K Jan  3 16:14 public-de6e9d980d8e499572a5.js
-rw-r--r-- 1 alice  1000  25K Jan  3 16:14 sentry_client-ed4d5a358dc66fefdb5e.js
-rw-r--r-- 1 alice  1000 670K Jan  3 16:14 style-1ac288f61b22136a5727.js
-rw-r--r-- 1 alice  1000 147K Jan  3 16:14 style_public-23eca547f17a0336295d.js
-rw-r--r-- 1 alice  1000 4.1K Jan  3 16:14 zendesk_widget-1e9e440ee13e0393adcf.js

not really better! doing some more poking to see if there are further optimizations I can get working.

noisecapella commented 7 years ago

Hmm... maybe it would be worth seeing if we can do a bit of manual tree shaking. Maybe there are overly broad imports that can be narrowed? Not sure what the return on invested time is there though

alicewriteswrongs commented 7 years ago

yeah, something I'm thinking about is doing some further splitting for specific features - searchkit, for instance, is the single biggest library in the bundle, and there's no reason for users who don't visit the search page to download it.

alicewriteswrongs commented 7 years ago

I got react-router splitting figured out! 🎊 🎆

this is what it gives us right away:

total 3.5M
drwxr-xr-x 2 alice users  12K Jan  4 14:42 ./
drwxr-xr-x 6 alice users 4.0K May 24  2016 ../
-rw-r--r-- 1 alice  1000 221K Jan  4 14:41 0-d0110f7d84da28f971f1.js
-rw-r--r-- 1 alice  1000  22K Jan  4 14:41 1-fb0d2a222d014784d286.js
-rw-r--r-- 1 alice  1000 332K Jan  4 14:41 2-b6215f409937e78df991.js
-rw-r--r-- 1 alice  1000 136K Jan  4 14:41 3-bb43f8734c32bc980103.js
-rw-r--r-- 1 alice  1000  13K Jan  4 14:41 4-5cfdc6491a738c3de47a.js
-rw-r--r-- 1 alice  1000 1.4M Jan  4 14:41 common-31df3a90822a778472fe.js
-rw-r--r-- 1 alice  1000 504K Jan  4 14:41 dashboard-f81b893da90886546d44.js
-rw-r--r-- 1 alice  1000  23K Jan  4 14:41 financial_aid-332851875722ba5ebcfa.js
-rw-r--r-- 1 alice  1000  59K Jan  4 14:41 public-372eb70e5734ce90169e.js
-rw-r--r-- 1 alice  1000  25K Jan  4 14:41 sentry_client-d1ea86239a382868fd94.js
-rw-r--r-- 1 alice  1000 670K Jan  4 14:41 style-0eb3c4c46fb756393ddd.js
-rw-r--r-- 1 alice  1000 147K Jan  4 14:41 style_public-83c95d782078e16b368b.js
-rw-r--r-- 1 alice  1000 4.1K Jan  4 14:41 zendesk_widget-085dd83e44b462d1199e.js

the files that are called something like 0-d0110f7d84da28f971f1.js are the files for each route. So dashboard.js ends up being quite a bit smaller, and the user will incrementally pull down the JS they need as they go, instead of just downloading the whole app at once.

I haven't figured out how to give the route splits meaningful names yet, but once I do that and iron out any issues I'll open a PR.

alicewriteswrongs commented 7 years ago

There's one other thing I thought of on the ride in today that I might look into today or tomorrow, which is seeing if there's a significant change in bundle size from moving some of our css import statements for dependency styles out of style.js and into the modules where the relevant dependency is used - might shuffle things around a little bit, and reduce the size of the stylesheet that any particular page receives.

noisecapella commented 7 years ago

Yeah, I noticed that the stylesheets were a surprisingly large chunk of our data

alicewriteswrongs commented 7 years ago

yeah, in particular it looks like we're including bootstrap on every page (it's imported in style.js) even though we only use it on the financial aid review interface.

noisecapella commented 7 years ago

We'll have to be careful removing that, it affects a whole lot of default styles for the other pages

alicewriteswrongs commented 7 years ago

true

alicewriteswrongs commented 7 years ago

One other thing we should probably investigate is removing some of the data files we include in JavaScript, like for country -> region and so on. There might be a noticeable impact from doing the work to move those into the backend.

pdpinch commented 7 years ago

What percentage of our JavaScript footprint is the list of countries and regions?

alicewriteswrongs commented 7 years ago

I just merged #2354, which should reduce unnecessary code in production a little bit.

One thing that we should do (following on from #2247 ) is implement a spinner or something similar to show while the bundle request is in-flight, which will let us have a better ux on slow connections for implementing route-based splitting for the rest of the app (i.e. splitting out the personal tabs from dashboard, settings, etc). Right now, without that added, doing full splitting doesn't have a great UX on a slow connection, since while the request for the next module is in-flight it is still possible for the user to interact with the application and to transition to other routes, and then when the first request comes back the associated transition will go through. We should implement something which prevents the user from interacting with the app while these requests are in-flight to get around this (will also probably make it much clearer what is going on).

alicewriteswrongs commented 7 years ago

this is how we'e doing on this right now, btw:

filesizes

pdpinch commented 7 years ago

So common's gotten a bit smaller, but dashboard has more than doubled?

Does dashboard include /learners and /learner ?

alicewriteswrongs commented 7 years ago

I haven't yet done the work to fully enable route based splitting, so those numbers shouldn't be compared to the next ones up (which were taken with full route-based splitting enabled) but based on these numbers.

alicewriteswrongs commented 7 years ago

so it's about the same

alicewriteswrongs commented 7 years ago

and /learners is not included in the dashboard bundle.

alicewriteswrongs commented 7 years ago

we should really enable full route-based splitting at some point, so I filed a separate issue for that (#3017)