Only render sublists when they change significantly

We can ignore off-screen updates, so do that. See diff for more details on what we're doing.
pull/21833/head
Travis Ralston 2020-07-23 22:13:32 -06:00
parent ad92e6ba00
commit 9969b01c5f
2 changed files with 85 additions and 1 deletions

View File

@ -48,6 +48,7 @@ import { polyfillTouchEvent } from "../../../@types/polyfill";
import { RoomNotificationStateStore } from "../../../stores/notifications/RoomNotificationStateStore"; import { RoomNotificationStateStore } from "../../../stores/notifications/RoomNotificationStateStore";
import RoomListLayoutStore from "../../../stores/room-list/RoomListLayoutStore"; import RoomListLayoutStore from "../../../stores/room-list/RoomListLayoutStore";
import { arrayHasDiff, arrayHasOrderChange } from "../../../utils/arrays"; import { arrayHasDiff, arrayHasOrderChange } from "../../../utils/arrays";
import { objectExcluding, objectHasValueChange } from "../../../utils/objects";
const SHOW_N_BUTTON_HEIGHT = 28; // As defined by CSS const SHOW_N_BUTTON_HEIGHT = 28; // As defined by CSS
const RESIZE_HANDLE_HEIGHT = 4; // As defined by CSS const RESIZE_HANDLE_HEIGHT = 4; // As defined by CSS
@ -174,6 +175,55 @@ export default class RoomSublist extends React.Component<IProps, IState> {
} }
} }
public shouldComponentUpdate(nextProps: Readonly<IProps>, nextState: Readonly<IState>): boolean {
if (objectHasValueChange(this.props, nextProps)) {
// Something we don't care to optimize has updated, so update.
return true;
}
// Do the same check used on props for state, without the rooms we're going to no-op
const prevStateNoRooms = objectExcluding(this.state, ['rooms']);
const nextStateNoRooms = objectExcluding(nextState, ['rooms']);
if (objectHasValueChange(prevStateNoRooms, nextStateNoRooms)) {
return true;
}
// If we're supposed to handle extra tiles, take the performance hit and re-render all the
// time so we don't have to consider them as part of the visible room optimization.
const prevExtraTiles = this.props.extraBadTilesThatShouldntExist || [];
const nextExtraTiles = nextProps.extraBadTilesThatShouldntExist || [];
if (prevExtraTiles.length > 0 || nextExtraTiles.length > 0) {
return true;
}
// If we're about to update the height of the list, we don't really care about which rooms
// are visible or not for no-op purposes, so ensure that the height calculation runs through.
if (RoomSublist.calcNumTiles(nextState.rooms, nextProps.extraBadTilesThatShouldntExist) !== this.numTiles) {
return true;
}
// Finally, determine if the room update (as presumably that's all that's left) is within
// our visible range. If it is, then do a render. If the update is outside our visible range
// then we can skip the update.
//
// We also optimize for order changing here: if the update did happen in our visible range
// but doesn't result in the list re-sorting itself then there's no reason for us to update
// on our own.
const prevSlicedRooms = this.state.rooms.slice(0, this.numVisibleTiles);
const nextSlicedRooms = nextState.rooms.slice(0, this.numVisibleTiles);
if (arrayHasOrderChange(prevSlicedRooms, nextSlicedRooms)) {
return true;
}
// Quickly double check we're not about to break something due to the number of rooms changing.
if (arrayHasDiff(this.state.rooms, nextState.rooms)) {
return true;
}
// Finally, nothing happened so no-op the update
return false;
}
public componentWillUnmount() { public componentWillUnmount() {
this.state.notificationState.destroy(); this.state.notificationState.destroy();
defaultDispatcher.unregister(this.dispatcherRef); defaultDispatcher.unregister(this.dispatcherRef);

View File

@ -14,7 +14,41 @@ See the License for the specific language governing permissions and
limitations under the License. limitations under the License.
*/ */
import { arrayDiff, arrayMerge, arrayUnion } from "./arrays"; import { arrayDiff, arrayHasDiff, arrayMerge, arrayUnion } from "./arrays";
/**
* Gets a new object which represents the provided object, excluding some properties.
* @param a The object to strip properties of. Must be defined.
* @param props The property names to remove.
* @returns The new object without the provided properties.
*/
export function objectExcluding(a: any, props: string[]): any {
// We use a Map to avoid hammering the `delete` keyword, which is slow and painful.
const tempMap = new Map<string, any>(Object.entries(a));
for (const prop of props) {
tempMap.delete(prop);
}
// Convert the map to an object again
return Array.from(tempMap.entries()).reduce((c, [k, v]) => {
c[k] = v;
return c;
}, {});
}
/**
* Determines if the two objects, which are assumed to be of the same
* key shape, have a difference in their values. If a difference is
* determined, true is returned.
* @param a The first object. Must be defined.
* @param b The second object. Must be defined.
* @returns True if there's a perceptual difference in the object's values.
*/
export function objectHasValueChange(a: any, b: any): boolean {
const aValues = Object.values(a);
const bValues = Object.values(b);
return arrayHasDiff(aValues, bValues);
}
/** /**
* Determines the keys added, changed, and removed between two objects. * Determines the keys added, changed, and removed between two objects.