Skip to content

Commit

Permalink
WIP: Allow createSelector to store a cache
Browse files Browse the repository at this point in the history
`createSelector` has a big problem: it invalidates its cache way too
frequently. It does this whenever dependants change, but it's not
comparing those dependants against the called parameters - it's applying
them "globally" within the selector such that even when the underlying
data in `state` is the same, because the selector was invoked with
different parameters it generates different dependants and thinks things
are different.

This change caches the dependants per cache key so we don't need to
clear so often. This introduces memory bloat because we're storing those
cached values throughout the session.
  • Loading branch information
dmsnell committed Jan 15, 2018
1 parent 0ba1a17 commit 787a31a
Show file tree
Hide file tree
Showing 2 changed files with 17 additions and 19 deletions.
33 changes: 15 additions & 18 deletions client/lib/create-selector/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@
* External dependencies
*/

import { memoize } from 'lodash';
import shallowEqual from 'react-pure-render/shallowEqual';

/**
Expand Down Expand Up @@ -88,28 +87,26 @@ export default function createSelector(
getDependants = DEFAULT_GET_DEPENDANTS,
getCacheKey = DEFAULT_GET_CACHE_KEY
) {
const memoizedSelector = memoize( selector, getCacheKey );
let lastDependants;
const cache = new Map();
const dependants = new Map();

if ( Array.isArray( getDependants ) ) {
getDependants = makeSelectorFromArray( getDependants );
}

return Object.assign(
function( state, ...args ) {
let currentDependants = getDependants( state, ...args );
if ( ! Array.isArray( currentDependants ) ) {
currentDependants = [ currentDependants ];
}

if ( lastDependants && ! shallowEqual( currentDependants, lastDependants ) ) {
memoizedSelector.cache.clear();
}
return ( ...args ) => {
const cacheKey = getCacheKey( ...args );
const lastDependants = dependants.get( cacheKey );
let currentDependants = getDependants( ...args );
if ( ! Array.isArray( currentDependants ) ) {
currentDependants = [ currentDependants ];
}

lastDependants = currentDependants;
if ( ! lastDependants || ! shallowEqual( currentDependants, lastDependants ) ) {
cache.set( cacheKey, selector( ...args ) );
dependants.set( cacheKey, currentDependants );
}

return memoizedSelector( state, ...args );
},
{ memoizedSelector }
);
return cache.get( cacheKey );
};
}
3 changes: 2 additions & 1 deletion client/state/stats/lists/utils.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
* External dependencies
*/

import deterministicStringify from 'json-stable-stringify';
import {
sortBy,
toPairs,
Expand Down Expand Up @@ -139,7 +140,7 @@ export function buildExportArray( data, parent = null ) {
* @return {String} Serialized stats query
*/
export function getSerializedStatsQuery( query = {} ) {
return JSON.stringify( sortBy( toPairs( query ), pair => pair[ 0 ] ) );
return deterministicStringify( query );
}

/**
Expand Down

0 comments on commit 787a31a

Please sign in to comment.