Common Performance Anti-Patterns in Calypso

calypso-performance-antipatterns.md

Anti-Patterns

• too many rerenders, mostly because of inefficient selectors
• computing something just to throw it away
• initializing everything during Calypso boot

Be aware how Redux update cycle works

• action dispatch calls reducer
• reducer produces a new state (almost always)
• mapStateToProps of all connected components is called
• the new props are shallow-compared with the previous props
• if they differ, setState is called
• React rerenders the connected component

Corollary

1. your ‘mapStateToProps` and the selectors therein will be called thousands times, and most of the time the input and output is the same as the previous one — proper memoization is critical
2. identical input must produce identical (===) output, otherwise the shallow comparison gets confused.

Selectors that return different output on identical inputs

function getMappedDomains( state ) {
  return state.domains.filter( domain => domain.isMapped )
}

function getDecoratedSettings( state ) {
  const { settings } = state.me;
  return {
    ...settings,
    computedProperty: getComputedProperty( settings )
 } );
}

Selectors that are not properly memoized

const getNotReallyMemoizedDomain = createSelector(
  ( state, siteId ) => {
    const domain = getPrimaryDomain( state, siteId );
    return expensivelyDecorateDomain( domain );
  },
  ( state, siteId ) => [ getPrimaryDomain( state, siteId ) ]
);

// The memoized selector forgets too easily:
getNotReallyMemoizedFoo( state, 123 );
getNotReallyMemoizedFoo( state, 456 );
getNotReallyMemoizedFoo( state, 123 );

Understand how exactly your memoization function works

• how it caches the return values
• when it forgets the cached values
• if it ever forgets the cached values (memory leaks!)
• treeSelect, createSelector, lodash/memoize

Shallow compare killers

function mapStateToProps( state, { siteId } ) {
  const query = {
    type: 'draft',
    siteId
  };
  
  return {
    query,
    posts: getPostsForQuery( state, query )
  };
}

function mapStateToProps( state ) {
  return {
    canUser: capability => canCurrentUser( state, capability )
  };
}

lib/compare-props can help:

connect(
  mapStateToProps,
  mapDispatchToProps,
  null,
  {
    areStatePropsEqual: compareProps( { deep: [ 'query' ] } )
  }
)

Don't compute something just to throw it away

<Popover isVisible={ false }>
  <h3>{ translate( 'I am the very model of an inefficient popover' ) }</h3>
  <ul>
    { items.map( item => <li>{ item.label }</li> ) }
  </ul>
<Popover>

The Popover and Dialog components are poorly designed: the most intuitive and elegant usage is very inefficient.

Things get even worse when 30+ invisible popovers install a scroll listener to update the positioning of their (nonexistent) DOM elements.

Somewhat better

<Popover isVisible={ false }>
  <PopoverContent />
</Popover>

or

isPopoverVisible && <Popover />

a popover like this can even be async loaded on first appearance!

Don't initialize everything during Calypso boot

Calypso spends 0.7s initializing itself. Big part of that time is wasted.

This happens in some data layer handlers:

import schemaValidator from 'is-my-json-valid';
import schema from './schema';

export const validator = schemaValidator( schema );

And guided tours initialize like this:

export const MyGuidedTour = makeTour(
  <Tour name="myGuidedTour" version="20170816">
    <Step name="init" arrow="top-left" when={ and( isDesktop, isNotNewUser ) }>
      <p>
        { translate( 'Did you know?' ) }
      </p>
      <ButtonRow>
	<Quit primary>{ translate( 'Got it, thanks!' ) }</Quit>
      </ButtonRow>
      <Link href="https://en.support.wordpress.com/learn-more">
        { translate( 'Learn more.' ) }
      </Link>
    </Step>
  </Tour>
);