From f2468f562d381310585fd7edbca24946c20b5c94 Mon Sep 17 00:00:00 2001 From: Travis Ralston Date: Fri, 7 Dec 2018 20:15:21 -0700 Subject: [PATCH 1/2] Speed up room unread checks by not hitting the SettingsStore so often This was noticed as a problem after `Unread.doesRoomHaveUnreadMessages` started being called a lot more frequently. Down the call stack, `shouldHideEvent` is called which used to call into the `SettingsStore` frequently, causing performance issues in many cases. The `SettingsStore` tries to be as fast as possible, however there's still code paths that make it less than desirable to use as the first condition in an AND condition. By not hitting the `SettingsStore` so often, we can shorten those code paths. As for how much this improves things, I ran some profiling before and after this change. This was done on my massive 1200+ room account. Before it was possible to see nearly 2 seconds spent generating room lists where 20-130ms per room was spent figuring out if the room has unread messages. Afterwards, the room list was generating within ~330ms and each unread check taking 0-2ms. There's still room for improvement on generating the room list, however the significant gains here seem worth it. --- src/shouldHideEvent.js | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/shouldHideEvent.js b/src/shouldHideEvent.js index 3aad05a9760..609f48b1281 100644 --- a/src/shouldHideEvent.js +++ b/src/shouldHideEvent.js @@ -42,14 +42,14 @@ export default function shouldHideEvent(ev) { const isEnabled = (name) => SettingsStore.getValue(name, ev.getRoomId()); // Hide redacted events - if (isEnabled('hideRedactions') && ev.isRedacted()) return true; + if (ev.isRedacted() && isEnabled('hideRedactions')) return true; const eventDiff = memberEventDiff(ev); if (eventDiff.isMemberEvent) { - if (isEnabled('hideJoinLeaves') && (eventDiff.isJoin || eventDiff.isPart)) return true; - if (isEnabled('hideAvatarChanges') && eventDiff.isAvatarChange) return true; - if (isEnabled('hideDisplaynameChanges') && eventDiff.isDisplaynameChange) return true; + if ((eventDiff.isJoin || eventDiff.isPart) && isEnabled('hideJoinLeaves')) return true; + if (eventDiff.isAvatarChange && isEnabled('hideAvatarChanges')) return true; + if (eventDiff.isDisplaynameChange && isEnabled('hideDisplaynameChanges')) return true; } return false; From ebdba32393d7b1b3f7f73aab793e39dc2fedb87d Mon Sep 17 00:00:00 2001 From: Travis Ralston Date: Sat, 8 Dec 2018 12:06:37 -0700 Subject: [PATCH 2/2] Add a comment about the SettingsStore being slow --- src/shouldHideEvent.js | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/shouldHideEvent.js b/src/shouldHideEvent.js index 609f48b1281..adc89a126ab 100644 --- a/src/shouldHideEvent.js +++ b/src/shouldHideEvent.js @@ -38,7 +38,9 @@ function memberEventDiff(ev) { } export default function shouldHideEvent(ev) { - // Wrap getValue() for readability + // 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()); // Hide redacted events