Closed cherdman closed 5 years ago
See the full results at https://webpagetest.org/result/190918_P5_28f85fa75f8be29a254107ae2a6c645e/.
This test was run using the following settings:
This profile was generated according to best practices:
development
mode with all browser extensions disabledWe're doing a good job memoizing our expensive functions. This is just one component example but the pattern is common. Here you can see the initial mount time, followed by the timing of additional mounts later on. Note that subsequent mounts are much faster than the initial one.
Immediately after our first render there is a cascading update:
We should investigate MagentoRouteHandler.componentDidMount
and HeadTag.componentDidMount
, the latter of which scheduled a cascading update.
Well it turns out I did mostly the wrong kind of investigation on this one.
Re-opening to dive back into React-specific performance (via the React Profiler).
The following audit was performed according to our audit guidelines against the release/4.0
branch.
📝 Application "site data" cleared before running this audit.
📝 Browser events such as DOMContentLoaded, First Paint, etc. not recorded here since we're in development mode and purposefully throttling the CPU.
We have three areas of significant CPU burn (horizontal red bars below):
I don't believe there is anything actionable about them, from left to right they are:
Here we can see the full timeline for Venia to mount (no user interactions performed).
Let's zoom in on each significant burst of activity. From left to right they are:
Here we can see Venia mounting for the first time.
Clicking the React Tree Reconciliation: Completed Root
segment and sorting by "Self Time (descending)" highlights components that are good targets for optimization:
Let's take a closer look at the ones that are an order of magnitude higher than the rest (>10ms):
Self Time | Component | Notes |
---|---|---|
23.4ms | Query (mount) |
This call occurs in the Footer here1 |
16.5ms | adapter_VeniaAdapter (mount) |
Here is the VeniaAdapter component. It has to initialize a lot of Apollo things, but perhaps that can be done more efficiently (or beforehand)? |
15.1ms | Route (mount) |
If we drill down into this one2 we can see that it is the Route call from Header3 |
12.5ms | Menu (mount) |
This is actually Menu from react-feather and it is expensive because it is wrapped in NavTrigger , a component that is connect ed to Redux. It may already be fixed on develop . |
1 We should consider making some GraphQL calls at build time. (Devil's advocate: how often is the app going to build relative to how often the GraphQL result will change?)
2
3 We should not lazy load (React.lazy
/ Suspense
) anything we know we need for the initial load.
The bulk of this activity is the componentDidMount
function on the HeadTag
, MagentoRouteHandler
, and Query
components:
HeadTag
There is a warning (⛔️) that HeadTag.componentDidMount
scheduled a cascading update. From Dan Abramov:
"Cascading" means an update inside an update. React first reconciles the tree by calling render, then commits DOM changes, then calls lifecycle hooks. If you call setState in componentDidMount, you are causing React to repeat the cycle: reconcile the tree, commit DOM changes, and call the hooks. This is wasteful.
Unfortunately this occurs in a third-party (react-head
) component.
MagentoRouteHandler
MagentoRouteHandler
's componentDidMount
does schedule a cascading update (leads to setting state here).
Query
The Query
component at 14.3 ms is the most concerning, but we already have plans to replace it with Apollo useQuery
hooks in #1657 .
This one is primarily the SearchBar
component mounting but I don't understand why Suspense
would have updated and told it to mount in the first place. Remember, there were no user interactions performed. The search bar was never opened.
Here's the relevant code from header.js:
<Suspense fallback={searchOpen ? suspenseFallback : null}>
<Route
render={({ history, location }) => (
<SearchBar
isOpen={searchOpen}
history={history}
location={location}
/>
)}
/>
</Suspense>
Provider
updateI believe Provider
refers to an internal component of react-apollo
's ApolloProvider
. We already have plans to replace this in #1657 .
Hopefully that will clear up the rest of the bursts of activity since they all start with a Provider
or Query
component update:
Flow:
Add to Cart
Once #1661 is complete we may see a marked improvement here (no GraphQL delay).
As far as the rest of the PDP, I see:
Suspense
> Options
update. We can shave time by resolving that.Items
update (10.3 ms) the first time you select an option (size, color)Header
update that includes a Suspense
update to SearchBar
MiniCart
gets three updates: two directly from Provider
and one from Provider
> App
> VeniaHeadProvider
> HeadProvider
📝 All interactions start with a single Valeria Two-Layer Tank already in the cart. Start from the homepage and `Empty Cache and Hard Reload". Then start recording.
This is after clicking the shopping cart icon to open the cart drawer.
Here is the timing chart before we get the details of the cart:
There's not much that jumps out as taking orders of magnitude longer than others, but it does seem suspect that some things are included at all:
VeniaHeadProvider
, HeadProvider
, and Footer
?Suspense
> Route
> SearchBar
CartTrigger
HeadProvider
> Container
These things are included under a Provider
update, which I believe stems from a call to getCartDetails
when we open the cart drawer. We may be able to eliminate some of these.
The UI that we'd most expect to get updated here is the MiniCart.
Notably, the bulk of the time spent updating MiniCart
is spent mounting Product
.
Again, three of the top six items are image / icon components. Improvement here will definitely speed things up across the board.
There's more than just the MiniCart getting updated here though. We should look into Provider
> Header
> ...
navTrigger_Trigger
SearchTrigger
Suspense
> Route
> SearchBar
Perhaps this will disappear when we move away from ApolloProvider
> Provider
.
We should definitely look into the SearchBar
though, that keeps appearing in every update.
This update looks good. It only updates the Product
and Kebab
.
Again I would only expect the MiniCart
to update here. I'm not sure why the Header
is updating at all.
This update looks good. Only Product
is updated.
📝 Checkout begins with a single Valeria Two-Layer Tank already in the cart.
I'm surprised to see the MiniCart
> Body
update here.
This update looks good.
It seems like we may be rendering twice when we submit the address. These two update trees are almost identical.
We do also get repeated Flow
> Form
updates here. There is one for each field in the form.
This update looks good.
Again we see a Flow
> Form
update for each field in the form.
The actual submission looks good.
Looks good.
There are two Flow
> Form
updates here. We can probably drop one.
The actual submit update looks good.
https://kentcdodds.com/blog/fix-the-slow-render-before-you-fix-the-re-render
https://reactrocket.com/post/react-redux-optimization/ https://itnext.io/redux-ruins-you-react-app-performance-you-are-doing-something-wrong-82e28ec96cf5 https://redux.js.org/faq/performance
Issues created and added to performance milestone https://github.com/magento/pwa-studio/milestone/20
What? Analyze Venia pages and components to see if they re-render unnecessarily or too frequently, or in general if they run repetitive code unintentially.
Why? React UIs are easy to quickly make, but in exchange, certain patterns can make React do more work than necessary to repaint the UI. This causes bad performance, slow response times due to "layout thrashing", and high energy consumption on battery-powered devices. Auditing our rendering and painting on the screen will allow us to identify possible real and perceptual performance gains, enabling us to improve actual user experience.
How?
Acceptance Criteria