Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Timeline rendering optimizations #6143

Merged
merged 9 commits into from
Jun 9, 2021
Merged
326 changes: 178 additions & 148 deletions src/TextForEvent.js → src/TextForEvent.ts

Large diffs are not rendered by default.

14 changes: 7 additions & 7 deletions src/components/structures/MessagePanel.js
Original file line number Diff line number Diff line change
Expand Up @@ -26,10 +26,11 @@ import * as sdk from '../../index';

import {MatrixClientPeg} from '../../MatrixClientPeg';
import SettingsStore from '../../settings/SettingsStore';
import RoomContext from "../../contexts/RoomContext";
import {Layout, LayoutPropType} from "../../settings/Layout";
import {_t} from "../../languageHandler";
import {haveTileForEvent} from "../views/rooms/EventTile";
import {textForEvent} from "../../TextForEvent";
import {hasText} from "../../TextForEvent";
import IRCTimelineProfileResizer from "../views/elements/IRCTimelineProfileResizer";
import DMRoomMap from "../../utils/DMRoomMap";
import NewRoomIntro from "../views/rooms/NewRoomIntro";
Expand Down Expand Up @@ -152,6 +153,8 @@ export default class MessagePanel extends React.Component {
enableFlair: PropTypes.bool,
};

static contextType = RoomContext;

constructor(props) {
super(props);

Expand Down Expand Up @@ -381,7 +384,7 @@ export default class MessagePanel extends React.Component {
// Always show highlighted event
if (this.props.highlightedEventId === mxEv.getId()) return true;

return !shouldHideEvent(mxEv);
return !shouldHideEvent(mxEv, this.context);
}

_readMarkerForEvent(eventId, isLastEvent) {
Expand Down Expand Up @@ -1177,11 +1180,8 @@ class MemberGrouper {

add(ev) {
if (ev.getType() === 'm.room.member') {
// We'll just double check that it's worth our time to do so, through an
// ugly hack. If textForEvent returns something, we should group it for
// rendering but if it doesn't then we'll exclude it.
const renderText = textForEvent(ev);
if (!renderText || renderText.trim().length === 0) return; // quietly ignore
// We can ignore any events that don't actually have a message to display
if (!hasText(ev)) return;
}
this.readMarker = this.readMarker || this.panel._readMarkerForEvent(
ev.getId(),
Expand Down
71 changes: 58 additions & 13 deletions src/components/structures/RoomView.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -155,7 +155,6 @@ export interface IState {
canPeek: boolean;
showApps: boolean;
isPeeking: boolean;
showReadReceipts: boolean;
showRightPanel: boolean;
// error object, as from the matrix client/server API
// If we failed to load information about the room,
Expand Down Expand Up @@ -183,6 +182,12 @@ export interface IState {
canReact: boolean;
canReply: boolean;
layout: Layout;
lowBandwidth: boolean;
showReadReceipts: boolean;
showRedactions: boolean;
showJoinLeaves: boolean;
showAvatarChanges: boolean;
showDisplaynameChanges: boolean;
matrixClientIsReady: boolean;
showUrlPreview?: boolean;
e2eStatus?: E2EStatus;
Expand All @@ -200,8 +205,7 @@ export default class RoomView extends React.Component<IProps, IState> {
private readonly dispatcherRef: string;
private readonly roomStoreToken: EventSubscription;
private readonly rightPanelStoreToken: EventSubscription;
private readonly showReadReceiptsWatchRef: string;
private readonly layoutWatcherRef: string;
private settingWatchers: string[];

private unmounted = false;
private permalinkCreators: Record<string, RoomPermalinkCreator> = {};
Expand Down Expand Up @@ -232,7 +236,6 @@ export default class RoomView extends React.Component<IProps, IState> {
canPeek: false,
showApps: false,
isPeeking: false,
showReadReceipts: true,
showRightPanel: RightPanelStore.getSharedInstance().isOpenForRoom,
joining: false,
atEndOfLiveTimeline: true,
Expand All @@ -242,6 +245,12 @@ export default class RoomView extends React.Component<IProps, IState> {
canReact: false,
canReply: false,
layout: SettingsStore.getValue("layout"),
lowBandwidth: SettingsStore.getValue("lowBandwidth"),
showReadReceipts: true,
showRedactions: true,
showJoinLeaves: true,
showAvatarChanges: true,
showDisplaynameChanges: true,
matrixClientIsReady: this.context && this.context.isInitialSyncComplete(),
dragCounter: 0,
};
Expand All @@ -268,9 +277,14 @@ export default class RoomView extends React.Component<IProps, IState> {
WidgetEchoStore.on(UPDATE_EVENT, this.onWidgetEchoStoreUpdate);
WidgetStore.instance.on(UPDATE_EVENT, this.onWidgetStoreUpdate);

this.showReadReceiptsWatchRef = SettingsStore.watchSetting("showReadReceipts", null,
this.onReadReceiptsChange);
this.layoutWatcherRef = SettingsStore.watchSetting("layout", null, this.onLayoutChange);
this.settingWatchers = [
SettingsStore.watchSetting("layout", null, () =>
this.setState({ layout: SettingsStore.getValue("layout") }),
),
SettingsStore.watchSetting("lowBandwidth", null, () =>
this.setState({ lowBandwidth: SettingsStore.getValue("lowBandwidth") }),
),
];
}

private onWidgetStoreUpdate = () => {
Expand Down Expand Up @@ -327,9 +341,42 @@ export default class RoomView extends React.Component<IProps, IState> {
// we should only peek once we have a ready client
shouldPeek: this.state.matrixClientIsReady && RoomViewStore.shouldPeek(),
showReadReceipts: SettingsStore.getValue("showReadReceipts", roomId),
showRedactions: SettingsStore.getValue("showRedactions", roomId),
showJoinLeaves: SettingsStore.getValue("showJoinLeaves", roomId),
showAvatarChanges: SettingsStore.getValue("showAvatarChanges", roomId),
showDisplaynameChanges: SettingsStore.getValue("showDisplaynameChanges", roomId),
wasContextSwitch: RoomViewStore.getWasContextSwitch(),
};

// Add watchers for each of the settings we just looked up
this.settingWatchers = this.settingWatchers.concat([
SettingsStore.watchSetting("showReadReceipts", null, () =>
this.setState({
showReadReceipts: SettingsStore.getValue("showReadReceipts", roomId),
}),
),
SettingsStore.watchSetting("showRedactions", null, () =>
this.setState({
showRedactions: SettingsStore.getValue("showRedactions", roomId),
}),
),
SettingsStore.watchSetting("showJoinLeaves", null, () =>
this.setState({
showJoinLeaves: SettingsStore.getValue("showJoinLeaves", roomId),
}),
),
SettingsStore.watchSetting("showAvatarChanges", null, () =>
this.setState({
showAvatarChanges: SettingsStore.getValue("showAvatarChanges", roomId),
}),
),
SettingsStore.watchSetting("showDisplaynameChanges", null, () =>
this.setState({
showDisplaynameChanges: SettingsStore.getValue("showDisplaynameChanges", roomId),
}),
),
]);

if (!initial && this.state.shouldPeek && !newState.shouldPeek) {
// Stop peeking because we have joined this room now
this.context.stopPeeking();
Expand Down Expand Up @@ -638,18 +685,16 @@ export default class RoomView extends React.Component<IProps, IState> {
);
}

if (this.showReadReceiptsWatchRef) {
SettingsStore.unwatchSetting(this.showReadReceiptsWatchRef);
}

// cancel any pending calls to the rate_limited_funcs
this.updateRoomMembers.cancelPendingCall();

// no need to do this as Dir & Settings are now overlays. It just burnt CPU.
// console.log("Tinter.tint from RoomView.unmount");
// Tinter.tint(); // reset colourscheme

SettingsStore.unwatchSetting(this.layoutWatcherRef);
for (const watcher of this.settingWatchers) {
SettingsStore.unwatchSetting(watcher);
}
}

private onUserScroll = () => {
Expand Down Expand Up @@ -819,7 +864,7 @@ export default class RoomView extends React.Component<IProps, IState> {
// update unread count when scrolled up
if (!this.state.searchResults && this.state.atEndOfLiveTimeline) {
// no change
} else if (!shouldHideEvent(ev)) {
} else if (!shouldHideEvent(ev, this.state)) {
this.setState((state, props) => {
return {numUnreadMessages: state.numUnreadMessages + 1};
});
Expand Down
5 changes: 4 additions & 1 deletion src/components/structures/TimelinePanel.js
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ import {EventTimeline} from "matrix-js-sdk/src/models/event-timeline";
import {TimelineWindow} from "matrix-js-sdk/src/timeline-window";
import { _t } from '../../languageHandler';
import {MatrixClientPeg} from "../../MatrixClientPeg";
import RoomContext from "../../contexts/RoomContext";
import UserActivity from "../../UserActivity";
import Modal from "../../Modal";
import dis from "../../dispatcher/dispatcher";
Expand Down Expand Up @@ -122,6 +123,8 @@ class TimelinePanel extends React.Component {
layout: LayoutPropType,
}

static contextType = RoomContext;

// a map from room id to read marker event timestamp
static roomReadMarkerTsMap = {};

Expand Down Expand Up @@ -1285,7 +1288,7 @@ class TimelinePanel extends React.Component {

const shouldIgnore = !!ev.status || // local echo
(ignoreOwn && ev.sender && ev.sender.userId == myUserId); // own message
const isWithoutTile = !haveTileForEvent(ev) || shouldHideEvent(ev);
const isWithoutTile = !haveTileForEvent(ev) || shouldHideEvent(ev, this.context);

if (isWithoutTile || !node) {
// don't start counting if the event should be ignored,
Expand Down
15 changes: 11 additions & 4 deletions src/components/views/avatars/BaseAvatar.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ import classNames from 'classnames';
import * as AvatarLogic from '../../../Avatar';
import SettingsStore from "../../../settings/SettingsStore";
import AccessibleButton from '../elements/AccessibleButton';
import RoomContext from "../../../contexts/RoomContext";
import MatrixClientContext from "../../../contexts/MatrixClientContext";
import {useEventEmitter} from "../../../hooks/useEventEmitter";
import {toPx} from "../../../utils/units";
Expand All @@ -44,12 +45,12 @@ interface IProps {
className?: string;
}

const calculateUrls = (url, urls) => {
const calculateUrls = (url, urls, lowBandwidth) => {
// work out the full set of urls to try to load. This is formed like so:
// imageUrls: [ props.url, ...props.urls ]

let _urls = [];
if (!SettingsStore.getValue("lowBandwidth")) {
if (!lowBandwidth) {
_urls = urls || [];

if (url) {
Expand All @@ -63,15 +64,21 @@ const calculateUrls = (url, urls) => {
};

const useImageUrl = ({url, urls}): [string, () => void] => {
const [imageUrls, setUrls] = useState<string[]>(calculateUrls(url, urls));
// Since this is a hot code path and the settings store can be slow, we
// use the cached lowBandwidth value from the room context if it exists
const roomContext = useContext(RoomContext);
const lowBandwidth = roomContext ?
roomContext.lowBandwidth : SettingsStore.getValue("lowBandwidth");

const [imageUrls, setUrls] = useState<string[]>(calculateUrls(url, urls, lowBandwidth));
const [urlsIndex, setIndex] = useState<number>(0);

const onError = useCallback(() => {
setIndex(i => i + 1); // try the next one
}, []);

useEffect(() => {
setUrls(calculateUrls(url, urls));
setUrls(calculateUrls(url, urls, lowBandwidth));
setIndex(0);
}, [url, JSON.stringify(urls)]); // eslint-disable-line react-hooks/exhaustive-deps

Expand Down
4 changes: 2 additions & 2 deletions src/components/views/rooms/EventTile.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ import { RoomMember } from "matrix-js-sdk/src/models/room-member";

import ReplyThread from "../elements/ReplyThread";
import { _t } from '../../../languageHandler';
import * as TextForEvent from "../../../TextForEvent";
import { hasText } from "../../../TextForEvent";
import * as sdk from "../../../index";
import dis from '../../../dispatcher/dispatcher';
import SettingsStore from "../../../settings/SettingsStore";
Expand Down Expand Up @@ -1200,7 +1200,7 @@ export function haveTileForEvent(e) {
const handler = getHandlerTile(e);
if (handler === undefined) return false;
if (handler === 'messages.TextualEvent') {
return TextForEvent.textForEvent(e) !== '';
return hasText(e);
} else if (handler === 'messages.RoomCreate') {
return Boolean(e.getContent()['predecessor']);
} else {
Expand Down
7 changes: 6 additions & 1 deletion src/contexts/RoomContext.ts
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,6 @@ const RoomContext = createContext<IState>({
canPeek: false,
showApps: false,
isPeeking: false,
showReadReceipts: true,
showRightPanel: true,
joining: false,
atEndOfLiveTimeline: true,
Expand All @@ -41,6 +40,12 @@ const RoomContext = createContext<IState>({
canReact: false,
canReply: false,
layout: Layout.Group,
lowBandwidth: false,
showReadReceipts: true,
showRedactions: true,
showJoinLeaves: true,
showAvatarChanges: true,
showDisplaynameChanges: true,
matrixClientIsReady: false,
dragCounter: 0,
});
Expand Down
2 changes: 1 addition & 1 deletion src/i18n/strings/en_EN.json
Original file line number Diff line number Diff line change
Expand Up @@ -556,8 +556,8 @@
"%(senderName)s made future room history visible to all room members.": "%(senderName)s made future room history visible to all room members.",
"%(senderName)s made future room history visible to anyone.": "%(senderName)s made future room history visible to anyone.",
"%(senderName)s made future room history visible to unknown (%(visibility)s).": "%(senderName)s made future room history visible to unknown (%(visibility)s).",
"%(userId)s from %(fromPowerLevel)s to %(toPowerLevel)s": "%(userId)s from %(fromPowerLevel)s to %(toPowerLevel)s",
"%(senderName)s changed the power level of %(powerLevelDiffText)s.": "%(senderName)s changed the power level of %(powerLevelDiffText)s.",
"%(userId)s from %(fromPowerLevel)s to %(toPowerLevel)s": "%(userId)s from %(fromPowerLevel)s to %(toPowerLevel)s",
"%(senderName)s changed the pinned messages for the room.": "%(senderName)s changed the pinned messages for the room.",
"%(widgetName)s widget modified by %(senderName)s": "%(widgetName)s widget modified by %(senderName)s",
"%(widgetName)s widget added by %(senderName)s": "%(widgetName)s widget added by %(senderName)s",
Expand Down
3 changes: 1 addition & 2 deletions src/settings/WatchManager.ts
Original file line number Diff line number Diff line change
Expand Up @@ -63,8 +63,7 @@ export class WatchManager {

if (!inRoomId) {
// Fire updates to all the individual room watchers too, as they probably care about the change higher up.
const callbacks = Array.from(roomWatchers.values()).flat(1);
callbacks.push(...callbacks);
callbacks.push(...Array.from(roomWatchers.values()).flat(1));
} else if (roomWatchers.has(IRRELEVANT_ROOM)) {
callbacks.push(...roomWatchers.get(IRRELEVANT_ROOM));
}
Expand Down
18 changes: 13 additions & 5 deletions src/shouldHideEvent.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
import {MatrixEvent} from "matrix-js-sdk/src/models/event";

import SettingsStore from "./settings/SettingsStore";
import {IState} from "./components/structures/RoomView";

interface IDiff {
isMemberEvent: boolean;
Expand Down Expand Up @@ -47,11 +48,18 @@ function memberEventDiff(ev: MatrixEvent): IDiff {
return diff;
}

export default function shouldHideEvent(ev: MatrixEvent): boolean {
// Wrap getValue() for readability. Calling the SettingsStore can be
// fairly resource heavy, so the checks below should avoid hitting it
// where possible.
const isEnabled = (name) => SettingsStore.getValue(name, ev.getRoomId());
/**
* Determines whether the given event should be hidden from timelines.
* @param ev The event
* @param ctx An optional RoomContext to pull cached settings values from to avoid
* hitting the settings store
*/
export default function shouldHideEvent(ev: MatrixEvent, ctx?: IState): boolean {
// Accessing the settings store directly can be expensive if done frequently,
// so we should prefer using cached values if a RoomContext is available
const isEnabled = ctx ?
name => ctx[name] :
name => SettingsStore.getValue(name, ev.getRoomId());

// Hide redacted events
if (ev.isRedacted() && !isEnabled('showRedactions')) return true;
Expand Down
14 changes: 13 additions & 1 deletion test/components/structures/MessagePanel-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -51,8 +51,20 @@ class WrappedMessagePanel extends React.Component {
};

render() {
const roomContext = {
room,
roomId: room.roomId,
canReact: true,
canReply: true,
showReadReceipts: true,
showRedactions: false,
showJoinLeaves: false,
showAvatarChanges: false,
showDisplaynameChanges: true,
};

return <MatrixClientContext.Provider value={client}>
<RoomContext.Provider value={{ canReact: true, canReply: true, room, roomId: room.roomId }}>
<RoomContext.Provider value={roomContext}>
<MessagePanel room={room} {...this.props} resizeNotifier={this.state.resizeNotifier} />
</RoomContext.Provider>
</MatrixClientContext.Provider>;
Expand Down