Improve app load times using code splitting #701
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
We now have basic code-splitting in place for a handful of components. This means that (e.g.) the code for the main interactive page is not fetched for display of the splash page. This reduces (initial) bundle sizes and should make for quicker load time.
Useful for understanding bundle contents and making decisions on where to code split / lazy load.
Lazily loading these components means they won't be fetched & displayed until after the main page has rendered. This allows the tree & map to appear faster than previously (smaller bundle to fetch & less JS to parse and render).
|
Thanks for the all the detailed work here @jameshadfield. I'm always a fan of performance improvements. Generally in favor of page loads. I looked through code changes and did a fair amount of testing. There does seem to be an improvement, though I seemed to be getting more like ~1s rather than ~2s, but I wasn't being entirely rigorous with Chrome's dev tools. I don't like the idea of a blank map panel during load. This is a more a rhetorical question for you: how much additional logic overhead is this bringing? Looking at the code it doesn't look bad at all. Please feel free to merge. |
trvrb
approved these changes
Feb 9, 2019
jameshadfield
added a commit
that referenced
this pull request
Feb 27, 2019
After code splitting (PR #701) the `transition` prototype was, depending on the dataset, not set on `Selection()` objects. See d3/d3-selection#185 for some background info. This commit closes #708
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Currently loading auspice (e.g. nextstrain.org/zika) involves fetching & parsing a JS bundle containing all the relevant app code (324kb minimised), then fetching the dataset JSON(s).
Code-splitting is a feature where the app code (bundle) can be split into separate chunks. This allows a smaller initial fetch+parse and can improve load times for apps. This PR implements code splitting for a number of the components (see below) and improves load time. The time until nextstrain.org/flu/seasonal/h3n2/ha/3y is fully displayed has reduced from 7.9s to 6.3s (average from 4 runs on my machine using the dev server vs live server) and it "feels" faster than this because the map/tree displays and can be interacted with before the entropy/frequency panels).
This image shows the current live version (top) vs this PR (bottom):
The components split into separate bundles:
I tried splitting out the Map component, which sped up loading even more but looked worse (in my opinion) as there was a white panel next to the tree for ~0.5s.
This PR is available for testing here: https://nextstrain-dev.herokuapp.com
Furthermore, we can now analyse the composition of each bundle using
auspice build --analyzeBundlewhich results in an interactive display (screenshot below). I'll make a separate issue for the improvements that this has flagged up.