Skip muted list update if no change #7228

Closed

infinite-persistence wants to merge 1 commit from ip/muted.uris into master

infinite-persistence commented

2021-10-05 16:15:59 +02:00

(Migrated from github.com)

Issue

Components render unnecessarily due to reference invalidation from selectMutedChannels.

selectMutedChannels run and return a new reference each time the app gains focus. createSelector will not help in this case, because we are indeed invalidating the data in the store in USER_STATE_POPULATE.

Changes

Don't update the state if the array is identical in content.
Fixed selectMutedChannels to return the reference from the store, so createSelector is not needed.
- Also, the filtering is not needed because we've already done it in the reducer.

## Issue Components render unnecessarily due to reference invalidation from `selectMutedChannels`. `selectMutedChannels` run and return a new reference each time the app gains focus. `createSelector` will not help in this case, because we are indeed invalidating the data in the store in `USER_STATE_POPULATE`. ## Changes - Don't update the state if the array is identical in content. - Fixed `selectMutedChannels` to return the reference from the store, so `createSelector` is not needed. - Also, the filtering is not needed because we've already done it in the reducer. <img src="https://user-images.githubusercontent.com/64950861/136040723-65b51335-a3a3-4c71-84c9-bcdcc78199c9.png" width="500">

jessopb (Migrated from github.com) approved these changes 2021-10-05 16:38:41 +02:00

jessopb (Migrated from github.com) left a comment

I think it's ok, given the way sync populate works
How big of a hit is filtering the blocklist every doPopulate?

I think it's ok, given the way sync populate works How big of a hit is filtering the blocklist every doPopulate?

infinite-persistence commented

2021-10-06 02:27:13 +02:00

(Migrated from github.com)

How big of a hit is filtering the blocklist every doPopulate?

Yeah, was (still) concerned about that too for large blocklists. But the change pretty much just moved the work in selectMutedChannels here, so we're still roughly doing the same amount of work with existing code at this point -- the saving comes from not making all listeners of this selector to re-render.

Update:
I've done some profiling on large blocklists. The time needed for the array comparison is still an order magnitude lower than the time needed to render all the Components that got incorrectly marked by this. The ideal solution is for the sync code to return a hash or timestamp of the array, so that we can compare that instead of the array.

Status:

Will move this into another PR, possible after the lbry-redux and inc consolidation.

> _How big of a hit is filtering the blocklist every doPopulate?_ Yeah, was (still) concerned about that too for large blocklists. But the change pretty much just moved the work in `selectMutedChannels` here, so we're still roughly doing the same amount of work with existing code at this point -- the saving comes from not making all listeners of this selector to re-render. _Update:_ I've done some profiling on large blocklists. The time needed for the array comparison is still an order magnitude lower than the time needed to render all the Components that got incorrectly marked by this. The ideal solution is for the sync code to return a hash or timestamp of the array, so that we can compare that instead of the array. ## Status: Will move this into another PR, possible after the lbry-redux and inc consolidation.

infinite-persistence commented

2021-10-11 04:35:58 +02:00

(Migrated from github.com)

Will combine this in the Ody-repo PR instead. Desktop can use it if interested.