Optimize makeGetStatus (#11211)

* Optimize makeGetStatus

Because `ImmutableList.filter` always returns a new object and `createSelector`
memoizes based on object identity, the selector returned by `makeGetStatus`
would *always* execute.

To avoid that, we wrap `getFilters` into a new memoizer that memoizes based on
deep equality, thus returning the same object as long as the filters haven't
changed, allowing the memoization of `makeGetStatus` to work.

Furthermore, we memoize the compiled regexs instead of recomputing them each
time the selector is called.

* Fix memoized result being cleared too often

* Make notifications use memoized getFiltersRegex
This commit is contained in:
ThibG 2019-06-30 00:12:38 +02:00 committed by Eugen Rochko
parent ccc7fe3e1d
commit f895bf1984
2 changed files with 31 additions and 12 deletions

View File

@ -11,7 +11,7 @@ import { saveSettings } from './settings';
import { defineMessages } from 'react-intl'; import { defineMessages } from 'react-intl';
import { List as ImmutableList } from 'immutable'; import { List as ImmutableList } from 'immutable';
import { unescapeHTML } from '../utils/html'; import { unescapeHTML } from '../utils/html';
import { getFilters, regexFromFilters } from '../selectors'; import { getFiltersRegex } from '../selectors';
export const NOTIFICATIONS_UPDATE = 'NOTIFICATIONS_UPDATE'; export const NOTIFICATIONS_UPDATE = 'NOTIFICATIONS_UPDATE';
export const NOTIFICATIONS_UPDATE_NOOP = 'NOTIFICATIONS_UPDATE_NOOP'; export const NOTIFICATIONS_UPDATE_NOOP = 'NOTIFICATIONS_UPDATE_NOOP';
@ -43,13 +43,13 @@ export function updateNotifications(notification, intlMessages, intlLocale) {
const showInColumn = getState().getIn(['settings', 'notifications', 'shows', notification.type], true); const showInColumn = getState().getIn(['settings', 'notifications', 'shows', notification.type], true);
const showAlert = getState().getIn(['settings', 'notifications', 'alerts', notification.type], true); const showAlert = getState().getIn(['settings', 'notifications', 'alerts', notification.type], true);
const playSound = getState().getIn(['settings', 'notifications', 'sounds', notification.type], true); const playSound = getState().getIn(['settings', 'notifications', 'sounds', notification.type], true);
const filters = getFilters(getState(), { contextType: 'notifications' }); const filters = getFiltersRegex(getState(), { contextType: 'notifications' });
let filtered = false; let filtered = false;
if (notification.type === 'mention') { if (notification.type === 'mention') {
const dropRegex = regexFromFilters(filters.filter(filter => filter.get('irreversible'))); const dropRegex = filters[0];
const regex = regexFromFilters(filters); const regex = filters[1];
const searchIndex = notification.status.spoiler_text + '\n' + unescapeHTML(notification.status.content); const searchIndex = notification.status.spoiler_text + '\n' + unescapeHTML(notification.status.content);
if (dropRegex && dropRegex.test(searchIndex)) { if (dropRegex && dropRegex.test(searchIndex)) {

View File

@ -1,5 +1,5 @@
import { createSelector } from 'reselect'; import { createSelector } from 'reselect';
import { List as ImmutableList } from 'immutable'; import { List as ImmutableList, is } from 'immutable';
import { me } from '../initial_state'; import { me } from '../initial_state';
const getAccountBase = (state, id) => state.getIn(['accounts', id], null); const getAccountBase = (state, id) => state.getIn(['accounts', id], null);
@ -36,12 +36,10 @@ const toServerSideType = columnType => {
} }
}; };
export const getFilters = (state, { contextType }) => state.get('filters', ImmutableList()).filter(filter => contextType && filter.get('context').includes(toServerSideType(contextType)) && (filter.get('expires_at') === null || Date.parse(filter.get('expires_at')) > (new Date())));
const escapeRegExp = string => const escapeRegExp = string =>
string.replace(/[.*+?^${}()|[\]\\]/g, '\\$&'); // $& means the whole matched string string.replace(/[.*+?^${}()|[\]\\]/g, '\\$&'); // $& means the whole matched string
export const regexFromFilters = filters => { const regexFromFilters = filters => {
if (filters.size === 0) { if (filters.size === 0) {
return null; return null;
} }
@ -63,6 +61,27 @@ export const regexFromFilters = filters => {
}).join('|'), 'i'); }).join('|'), 'i');
}; };
// Memoize the filter regexps for each valid server contextType
const makeGetFiltersRegex = () => {
let memo = {};
return (state, { contextType }) => {
if (!contextType) return ImmutableList();
const serverSideType = toServerSideType(contextType);
const filters = state.get('filters', ImmutableList()).filter(filter => filter.get('context').includes(serverSideType) && (filter.get('expires_at') === null || Date.parse(filter.get('expires_at')) > (new Date())));
if (!memo[serverSideType] || !is(memo[serverSideType].filters, filters)) {
const dropRegex = regexFromFilters(filters.filter(filter => filter.get('irreversible')));
const regex = regexFromFilters(filters);
memo[serverSideType] = { filters: filters, results: [dropRegex, regex] };
}
return memo[serverSideType].results;
};
};
export const getFiltersRegex = makeGetFiltersRegex();
export const makeGetStatus = () => { export const makeGetStatus = () => {
return createSelector( return createSelector(
[ [
@ -70,10 +89,10 @@ export const makeGetStatus = () => {
(state, { id }) => state.getIn(['statuses', state.getIn(['statuses', id, 'reblog'])]), (state, { id }) => state.getIn(['statuses', state.getIn(['statuses', id, 'reblog'])]),
(state, { id }) => state.getIn(['accounts', state.getIn(['statuses', id, 'account'])]), (state, { id }) => state.getIn(['accounts', state.getIn(['statuses', id, 'account'])]),
(state, { id }) => state.getIn(['accounts', state.getIn(['statuses', state.getIn(['statuses', id, 'reblog']), 'account'])]), (state, { id }) => state.getIn(['accounts', state.getIn(['statuses', state.getIn(['statuses', id, 'reblog']), 'account'])]),
getFilters, getFiltersRegex,
], ],
(statusBase, statusReblog, accountBase, accountReblog, filters) => { (statusBase, statusReblog, accountBase, accountReblog, filtersRegex) => {
if (!statusBase) { if (!statusBase) {
return null; return null;
} }
@ -84,12 +103,12 @@ export const makeGetStatus = () => {
statusReblog = null; statusReblog = null;
} }
const dropRegex = (accountReblog || accountBase).get('id') !== me && regexFromFilters(filters.filter(filter => filter.get('irreversible'))); const dropRegex = (accountReblog || accountBase).get('id') !== me && filtersRegex[0];
if (dropRegex && dropRegex.test(statusBase.get('reblog') ? statusReblog.get('search_index') : statusBase.get('search_index'))) { if (dropRegex && dropRegex.test(statusBase.get('reblog') ? statusReblog.get('search_index') : statusBase.get('search_index'))) {
return null; return null;
} }
const regex = (accountReblog || accountBase).get('id') !== me && regexFromFilters(filters); const regex = (accountReblog || accountBase).get('id') !== me && filtersRegex[1];
const filtered = regex && regex.test(statusBase.get('reblog') ? statusReblog.get('search_index') : statusBase.get('search_index')); const filtered = regex && regex.test(statusBase.get('reblog') ? statusReblog.get('search_index') : statusBase.get('search_index'));
return statusBase.withMutations(map => { return statusBase.withMutations(map => {