byId[] fixes to reduce invalidation (#239)

* Don't update 'pendingById' if no changes.

'pendingById' isn't frequently updated, but using it as a proof-of-concept to fix how reducers should be written to avoid unnecessary updates.

ImmutableJS apparently does all of this for us, but there are cons to using it as well, so using own wrappers for now.

* Don't update 'byId' if no changes + add 'selectClaimWithId'

## Ticket
116 Claim store optimization ideas (reducing unnecessary renders)

## Changes
- Ignore things like `confirmations` so that already-fetched claims aren't invalidated and causes re-rendering. The `stringify` might look expensive, but the amount of avoided re-renders outweighs it. There might be faster ways to compare, though.

- With `byId[claimId]` references more stable now, memoized selectors can now use 'selectClaimWithId' to pick a specific claim to depend on, instead of 'byId' which changes on every update.

* Fix memo: selectMyChannelClaims, selectActiveChannelClaim

## Issue
These should never recalculate after `channel_list` has been fetched, but they do because of poor selector dependency.

## Change
With the `byId` changes from the previous commit, we are now able to memoize these selectors correctly.
This commit is contained in:
infinite-persistence 2021-11-09 01:25:29 +08:00 committed by GitHub
parent cfd67b1c8d
commit c97cab0ebb
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
3 changed files with 123 additions and 28 deletions

View file

@ -111,11 +111,63 @@ const defaultState = {
checkingReflecting: false, checkingReflecting: false,
}; };
// ****************************************************************************
// Helpers
// ****************************************************************************
function isObjEmpty(object: any) {
return Object.keys(object).length === 0;
}
function resolveDelta(original: any, delta: any) {
if (isObjEmpty(delta)) {
// Don't invalidate references when there are no changes, so return original.
return original;
} else {
// When there are changes: create a new object, spread existing references,
// and overwrite specific items with new data.
return { ...original, ...delta };
}
}
function claimObjHasNewData(original, fresh) {
// Don't blow away 'is_my_output' just because the next query didn't ask for it.
const ignoreIsMyOutput = original.is_my_output !== undefined && fresh.is_my_output === undefined;
// Something is causing the tags to be re-ordered differently
// (https://github.com/OdyseeTeam/odysee-frontend/issues/116#issuecomment-962747147).
// Just do a length comparison for now, which covers 99% of cases while we
// figure out what's causing the order to change.
const ignoreTags =
original.value &&
fresh.value &&
original.value.tags &&
fresh.value.tags &&
original.value.tags.length !== fresh.value.tags.length;
const excludeKeys = (key, value) => {
if (key === 'confirmations' || (ignoreTags && key === 'tags') || (ignoreIsMyOutput && key === 'is_my_output')) {
return undefined;
}
return value;
};
const originalStringified = JSON.stringify(original, excludeKeys);
const freshStringified = JSON.stringify(fresh, excludeKeys);
return originalStringified !== freshStringified;
}
// ****************************************************************************
// handleClaimAction
// ****************************************************************************
function handleClaimAction(state: State, action: any): State { function handleClaimAction(state: State, action: any): State {
const { resolveInfo }: ClaimActionResolveInfo = action.data; const { resolveInfo }: ClaimActionResolveInfo = action.data;
const byUri = Object.assign({}, state.claimsByUri); const byUri = Object.assign({}, state.claimsByUri);
const byId = Object.assign({}, state.byId); const byIdDelta = {};
const channelClaimCounts = Object.assign({}, state.channelClaimCounts); const channelClaimCounts = Object.assign({}, state.channelClaimCounts);
const pendingById = state.pendingById; const pendingById = state.pendingById;
let newResolvingUrls = new Set(state.resolvingUris); let newResolvingUrls = new Set(state.resolvingUris);
@ -128,10 +180,13 @@ function handleClaimAction(state: State, action: any): State {
if (stream) { if (stream) {
if (pendingById[stream.claim_id]) { if (pendingById[stream.claim_id]) {
byId[stream.claim_id] = mergeClaim(stream, byId[stream.claim_id]); byIdDelta[stream.claim_id] = mergeClaim(stream, state.byId[stream.claim_id]);
} else { } else {
byId[stream.claim_id] = stream; if (!state.byId[stream.claim_id] || claimObjHasNewData(state.byId[stream.claim_id], stream)) {
byIdDelta[stream.claim_id] = stream;
}
} }
byUri[url] = stream.claim_id; byUri[url] = stream.claim_id;
// If url isn't a canonical_url, make sure that is added too // If url isn't a canonical_url, make sure that is added too
@ -158,9 +213,9 @@ function handleClaimAction(state: State, action: any): State {
} }
if (pendingById[channel.claim_id]) { if (pendingById[channel.claim_id]) {
byId[channel.claim_id] = mergeClaim(channel, byId[channel.claim_id]); byIdDelta[channel.claim_id] = mergeClaim(channel, state.byId[channel.claim_id]);
} else { } else {
byId[channel.claim_id] = channel; byIdDelta[channel.claim_id] = channel;
} }
byUri[channel.permanent_url] = channel.claim_id; byUri[channel.permanent_url] = channel.claim_id;
@ -171,10 +226,11 @@ function handleClaimAction(state: State, action: any): State {
if (collection) { if (collection) {
if (pendingById[collection.claim_id]) { if (pendingById[collection.claim_id]) {
byId[collection.claim_id] = mergeClaim(collection, byId[collection.claim_id]); byIdDelta[collection.claim_id] = mergeClaim(collection, state.byId[collection.claim_id]);
} else { } else {
byId[collection.claim_id] = collection; byIdDelta[collection.claim_id] = collection;
} }
byUri[url] = collection.claim_id; byUri[url] = collection.claim_id;
byUri[collection.canonical_url] = collection.claim_id; byUri[collection.canonical_url] = collection.claim_id;
byUri[collection.permanent_url] = collection.claim_id; byUri[collection.permanent_url] = collection.claim_id;
@ -193,7 +249,7 @@ function handleClaimAction(state: State, action: any): State {
}); });
return Object.assign({}, state, { return Object.assign({}, state, {
byId, byId: resolveDelta(state.byId, byIdDelta),
claimsByUri: byUri, claimsByUri: byUri,
channelClaimCounts, channelClaimCounts,
resolvingUris: Array.from(newResolvingUrls), resolvingUris: Array.from(newResolvingUrls),
@ -201,6 +257,10 @@ function handleClaimAction(state: State, action: any): State {
}); });
} }
// ****************************************************************************
// Reducers
// ****************************************************************************
reducers[ACTIONS.RESOLVE_URIS_STARTED] = (state: State, action: any): State => { reducers[ACTIONS.RESOLVE_URIS_STARTED] = (state: State, action: any): State => {
const { uris }: { uris: Array<string> } = action.data; const { uris }: { uris: Array<string> } = action.data;
@ -237,7 +297,8 @@ reducers[ACTIONS.FETCH_CLAIM_LIST_MINE_COMPLETED] = (state: State, action: any):
const byId = Object.assign({}, state.byId); const byId = Object.assign({}, state.byId);
const byUri = Object.assign({}, state.claimsByUri); const byUri = Object.assign({}, state.claimsByUri);
const pendingById = Object.assign({}, state.pendingById); const pendingByIdDelta = {};
let myClaimIds = new Set(state.myClaims); let myClaimIds = new Set(state.myClaims);
let urlsForCurrentPage = []; let urlsForCurrentPage = [];
@ -245,8 +306,10 @@ reducers[ACTIONS.FETCH_CLAIM_LIST_MINE_COMPLETED] = (state: State, action: any):
const { permanent_url: permanentUri, claim_id: claimId, canonical_url: canonicalUri } = claim; const { permanent_url: permanentUri, claim_id: claimId, canonical_url: canonicalUri } = claim;
if (claim.type && claim.type.match(/claim|update/)) { if (claim.type && claim.type.match(/claim|update/)) {
urlsForCurrentPage.push(permanentUri); urlsForCurrentPage.push(permanentUri);
if (claim.confirmations < 1) { if (claim.confirmations < 1) {
pendingById[claimId] = claim; pendingByIdDelta[claimId] = claim;
if (byId[claimId]) { if (byId[claimId]) {
byId[claimId] = mergeClaim(claim, byId[claimId]); byId[claimId] = mergeClaim(claim, byId[claimId]);
} else { } else {
@ -255,6 +318,7 @@ reducers[ACTIONS.FETCH_CLAIM_LIST_MINE_COMPLETED] = (state: State, action: any):
} else { } else {
byId[claimId] = claim; byId[claimId] = claim;
} }
byUri[permanentUri] = claimId; byUri[permanentUri] = claimId;
byUri[canonicalUri] = claimId; byUri[canonicalUri] = claimId;
myClaimIds.add(claimId); myClaimIds.add(claimId);
@ -265,7 +329,7 @@ reducers[ACTIONS.FETCH_CLAIM_LIST_MINE_COMPLETED] = (state: State, action: any):
isFetchingClaimListMine: false, isFetchingClaimListMine: false,
myClaims: Array.from(myClaimIds), myClaims: Array.from(myClaimIds),
byId, byId,
pendingById, pendingById: resolveDelta(state.pendingById, pendingByIdDelta),
claimsByUri: byUri, claimsByUri: byUri,
myClaimsPageResults: urlsForCurrentPage, myClaimsPageResults: urlsForCurrentPage,
myClaimsPageNumber: page, myClaimsPageNumber: page,
@ -279,7 +343,7 @@ reducers[ACTIONS.FETCH_CHANNEL_LIST_STARTED] = (state: State): State =>
reducers[ACTIONS.FETCH_CHANNEL_LIST_COMPLETED] = (state: State, action: any): State => { reducers[ACTIONS.FETCH_CHANNEL_LIST_COMPLETED] = (state: State, action: any): State => {
const { claims }: { claims: Array<ChannelClaim> } = action.data; const { claims }: { claims: Array<ChannelClaim> } = action.data;
let myClaimIds = new Set(state.myClaims); let myClaimIds = new Set(state.myClaims);
const pendingById = Object.assign({}, state.pendingById); const pendingByIdDelta = {};
let myChannelClaims; let myChannelClaims;
const byId = Object.assign({}, state.byId); const byId = Object.assign({}, state.byId);
const byUri = Object.assign({}, state.claimsByUri); const byUri = Object.assign({}, state.claimsByUri);
@ -302,8 +366,10 @@ reducers[ACTIONS.FETCH_CHANNEL_LIST_COMPLETED] = (state: State, action: any): St
// $FlowFixMe // $FlowFixMe
myChannelClaims.add(claimId); myChannelClaims.add(claimId);
if (confirmations < 1) { if (confirmations < 1) {
pendingById[claimId] = claim; pendingByIdDelta[claimId] = claim;
if (byId[claimId]) { if (byId[claimId]) {
byId[claimId] = mergeClaim(claim, byId[claimId]); byId[claimId] = mergeClaim(claim, byId[claimId]);
} else { } else {
@ -312,13 +378,14 @@ reducers[ACTIONS.FETCH_CHANNEL_LIST_COMPLETED] = (state: State, action: any): St
} else { } else {
byId[claimId] = claim; byId[claimId] = claim;
} }
myClaimIds.add(claimId); myClaimIds.add(claimId);
}); });
} }
return Object.assign({}, state, { return Object.assign({}, state, {
byId, byId,
pendingById, pendingById: resolveDelta(state.pendingById, pendingByIdDelta),
claimsByUri: byUri, claimsByUri: byUri,
channelClaimCounts, channelClaimCounts,
fetchingMyChannels: false, fetchingMyChannels: false,
@ -342,7 +409,7 @@ reducers[ACTIONS.FETCH_COLLECTION_LIST_COMPLETED] = (state: State, action: any):
const { claims }: { claims: Array<CollectionClaim> } = action.data; const { claims }: { claims: Array<CollectionClaim> } = action.data;
const myClaims = state.myClaims || []; const myClaims = state.myClaims || [];
let myClaimIds = new Set(myClaims); let myClaimIds = new Set(myClaims);
const pendingById = Object.assign({}, state.pendingById); const pendingByIdDelta = {};
let myCollectionClaimsSet = new Set([]); let myCollectionClaimsSet = new Set([]);
const byId = Object.assign({}, state.byId); const byId = Object.assign({}, state.byId);
const byUri = Object.assign({}, state.claimsByUri); const byUri = Object.assign({}, state.claimsByUri);
@ -357,9 +424,11 @@ reducers[ACTIONS.FETCH_COLLECTION_LIST_COMPLETED] = (state: State, action: any):
// $FlowFixMe // $FlowFixMe
myCollectionClaimsSet.add(claimId); myCollectionClaimsSet.add(claimId);
// we don't want to overwrite a pending result with a resolve // we don't want to overwrite a pending result with a resolve
if (confirmations < 1) { if (confirmations < 1) {
pendingById[claimId] = claim; pendingByIdDelta[claimId] = claim;
if (byId[claimId]) { if (byId[claimId]) {
byId[claimId] = mergeClaim(claim, byId[claimId]); byId[claimId] = mergeClaim(claim, byId[claimId]);
} else { } else {
@ -368,6 +437,7 @@ reducers[ACTIONS.FETCH_COLLECTION_LIST_COMPLETED] = (state: State, action: any):
} else { } else {
byId[claimId] = claim; byId[claimId] = claim;
} }
myClaimIds.add(claimId); myClaimIds.add(claimId);
}); });
} }
@ -375,7 +445,7 @@ reducers[ACTIONS.FETCH_COLLECTION_LIST_COMPLETED] = (state: State, action: any):
return { return {
...state, ...state,
byId, byId,
pendingById, pendingById: resolveDelta(state.pendingById, pendingByIdDelta),
claimsByUri: byUri, claimsByUri: byUri,
fetchingMyCollections: false, fetchingMyCollections: false,
myCollectionClaims: Array.from(myCollectionClaimsSet), myCollectionClaims: Array.from(myCollectionClaimsSet),

View file

@ -1,5 +1,5 @@
import { createSelector } from 'reselect'; import { createSelector } from 'reselect';
import { selectClaimsById, selectMyChannelClaims, makeSelectStakedLevelForChannelUri } from 'redux/selectors/claims'; import { selectClaimWithId, selectMyChannelClaims, makeSelectStakedLevelForChannelUri } from 'redux/selectors/claims';
export const selectState = (state) => state.app || {}; export const selectState = (state) => state.app || {};
@ -70,15 +70,13 @@ export const selectSplashAnimationEnabled = (state) => selectState(state).splash
export const selectActiveChannelId = (state) => selectState(state).activeChannel; export const selectActiveChannelId = (state) => selectState(state).activeChannel;
export const selectActiveChannelClaim = createSelector( export const selectActiveChannelClaim = createSelector(
selectActiveChannelId, (state) => selectClaimWithId(state, selectActiveChannelId(state)), // i.e. 'byId[activeChannelId]' specifically, instead of just 'byId'.
selectClaimsById,
selectMyChannelClaims, selectMyChannelClaims,
(activeChannelClaimId, claimsById, myChannelClaims) => { (activeChannelClaim, myChannelClaims) => {
if (!activeChannelClaimId || !claimsById || !myChannelClaims || !myChannelClaims.length) { if (!activeChannelClaim || !myChannelClaims || !myChannelClaims.length) {
return undefined; return undefined;
} }
const activeChannelClaim = claimsById[activeChannelClaimId];
if (activeChannelClaim) { if (activeChannelClaim) {
return activeChannelClaim; return activeChannelClaim;
} }

View file

@ -43,6 +43,21 @@ export const selectClaimsByUri = createSelector(selectClaimIdsByUri, selectClaim
return claims; return claims;
}); });
/**
* Returns the claim with the specified ID. The claim could be undefined if does
* not exist or have not fetched. Take note of the second parameter, which means
* an inline function or helper would be required when used as an input to
* 'createSelector'.
*
* @param state
* @param claimId
* @returns {*}
*/
export const selectClaimWithId = (state: State, claimId: string) => {
const byId = selectClaimsById(state);
return byId[claimId];
};
export const selectAllClaimsByChannel = createSelector(selectState, (state) => state.paginatedClaimsByChannel || {}); export const selectAllClaimsByChannel = createSelector(selectState, (state) => state.paginatedClaimsByChannel || {});
export const selectPendingIds = createSelector(selectState, (state) => Object.keys(state.pendingById) || []); export const selectPendingIds = createSelector(selectState, (state) => Object.keys(state.pendingById) || []);
@ -404,14 +419,26 @@ export const selectFetchingMyCollections = (state: State) => selectState(state).
export const selectMyChannelClaimIds = (state: State) => selectState(state).myChannelClaims; export const selectMyChannelClaimIds = (state: State) => selectState(state).myChannelClaims;
export const selectMyChannelClaims = createSelector(selectState, selectClaimsById, (state, byId) => { export const selectMyChannelClaims = createSelector(selectMyChannelClaimIds, (myChannelClaimIds) => {
const ids = state.myChannelClaims; if (!myChannelClaimIds) {
if (!ids) { return myChannelClaimIds;
return ids;
} }
if (!window || !window.store) {
return undefined;
}
// Note: Grabbing the store and running the selector this way is anti-pattern,
// but it is _needed_ and works only because we know for sure that 'byId[]'
// will be populated with the same claims as when 'myChannelClaimIds' is populated.
// If we put 'state' or 'byId' as the input selector, it essentially
// recalculates every time. Putting 'state' as input to createSelector() is
// always wrong from a memoization standpoint.
const state = window.store.getState();
const byId = selectClaimsById(state);
const claims = []; const claims = [];
ids.forEach((id) => { myChannelClaimIds.forEach((id) => {
if (byId[id]) { if (byId[id]) {
// I'm not sure why this check is necessary, but it ought to be a quick fix for https://github.com/lbryio/lbry-desktop/issues/544 // I'm not sure why this check is necessary, but it ought to be a quick fix for https://github.com/lbryio/lbry-desktop/issues/544
claims.push(byId[id]); claims.push(byId[id]);