From 087c3c6088f86568636183197662dc6fd25d7c49 Mon Sep 17 00:00:00 2001 From: Jonathan Rainville Date: Wed, 28 Aug 2024 11:21:13 -0400 Subject: [PATCH] perf(chat_section): improve onChatsLoaded performance (#16186) Fixes #16181 This commit improves the time taken by onChatsLoaded that is called on login. The culprit is `buildChatSectionUI` in the chat_section module. There is no silver bullet here. It's slow because the community is big and has a lot of channels to load and generate. This commit helps by removing some old code that was inefficient (calculating if a channel has a permission instead of just using the `tokenGated` property for example). It also adds all the section items in one go instead of one another. There are some other small improvements, but again, no way to make it way faster. Thankfully, that time is spent with the loading spinner at the same time. --- .../modules/main/chat_section/controller.nim | 8 +-- .../main/chat_section/io_interface.nim | 3 +- src/app/modules/main/chat_section/item.nim | 2 +- src/app/modules/main/chat_section/model.nim | 4 +- src/app/modules/main/chat_section/module.nim | 67 ++++++++++--------- src/app/modules/main/module.nim | 8 ++- .../modules/shared_models/section_model.nim | 15 ++++- src/app_service/service/community/service.nim | 16 ----- 8 files changed, 60 insertions(+), 63 deletions(-) diff --git a/src/app/modules/main/chat_section/controller.nim b/src/app/modules/main/chat_section/controller.nim index 30b3ad0d70..989fc1f8f2 100644 --- a/src/app/modules/main/chat_section/controller.nim +++ b/src/app/modules/main/chat_section/controller.nim @@ -437,7 +437,7 @@ proc getChatsAndBuildUI*(self: Controller) = community = self.getMyCommunity() let normalChats = self.chatService.getChatsForCommunity(community.id) - # TODO remove this once we do this refactor https://github.com/status-im/status-desktop/issues/14219 + # TODO remove this once we do this refactor https://github.com/status-im/status-desktop/issues/11694 var fullChats: seq[ChatDto] = @[] for communityChat in community.chats: for chat in normalChats: @@ -714,12 +714,6 @@ proc getColorHash*(self: Controller, pubkey: string): ColorHashDto = proc getColorId*(self: Controller, pubkey: string): int = procs_from_visual_identity_service.colorIdOf(pubkey) -proc checkChatHasPermissions*(self: Controller, communityId: string, chatId: string): bool = - return self.communityService.checkChatHasPermissions(communityId, chatId) - -proc checkChatIsLocked*(self: Controller, communityId: string, chatId: string): bool = - return self.communityService.checkChatIsLocked(communityId, chatId) - proc createOrEditCommunityTokenPermission*(self: Controller, communityId: string, tokenPermission: CommunityTokenPermissionDto) = self.communityService.createOrEditCommunityTokenPermission(communityId, tokenPermission) diff --git a/src/app/modules/main/chat_section/io_interface.nim b/src/app/modules/main/chat_section/io_interface.nim index 3bc96483d7..d65af7b15c 100644 --- a/src/app/modules/main/chat_section/io_interface.nim +++ b/src/app/modules/main/chat_section/io_interface.nim @@ -80,7 +80,8 @@ method addOrUpdateChat*(self: AccessInterface, mailserversService: mailservers_service.Service, sharedUrlsService: shared_urls_service.Service, setChatAsActive: bool = true, - insertIntoModel: bool = true + insertIntoModel: bool = true, + isSectionBuild: bool = false, ): Item {.base.} = raise newException(ValueError, "No implementation available") diff --git a/src/app/modules/main/chat_section/item.nim b/src/app/modules/main/chat_section/item.nim index 75b02bc3cf..abe08a7232 100644 --- a/src/app/modules/main/chat_section/item.nim +++ b/src/app/modules/main/chat_section/item.nim @@ -323,7 +323,7 @@ proc `loaderActive=`*(self: var Item, value: bool) = proc isCategory*(self: Item): bool = self.`type` == CATEGORY_TYPE -proc isLocked*(self: Item): bool = +proc locked*(self: Item): bool = self.locked proc `locked=`*(self: Item, value: bool) = diff --git a/src/app/modules/main/chat_section/model.nim b/src/app/modules/main/chat_section/model.nim index 194603615a..01c9e5a44a 100644 --- a/src/app/modules/main/chat_section/model.nim +++ b/src/app/modules/main/chat_section/model.nim @@ -210,7 +210,7 @@ QtObject: of ModelRole.LoaderActive: result = newQVariant(item.loaderActive) of ModelRole.Locked: - result = newQVariant(item.isLocked) + result = newQVariant(item.locked) of ModelRole.RequiresPermissions: result = newQVariant(item.requiresPermissions) of ModelRole.CanPost: @@ -366,7 +366,7 @@ QtObject: if index == -1: return - if (self.items[index].isLocked == locked): + if (self.items[index].locked == locked): return self.items[index].locked = locked diff --git a/src/app/modules/main/chat_section/module.nim b/src/app/modules/main/chat_section/module.nim index 8acdf18657..5207782ce1 100644 --- a/src/app/modules/main/chat_section/module.nim +++ b/src/app/modules/main/chat_section/module.nim @@ -88,7 +88,9 @@ method addOrUpdateChat(self: Module, sharedUrlsService: shared_urls_service.Service, setChatAsActive: bool = true, insertIntoModel: bool = true, + isSectionBuild: bool = false, ): chat_item.Item +proc updateParentBadgeNotifications(self: Module) proc newModule*( delegate: delegate_interface.AccessInterface, @@ -333,9 +335,12 @@ proc buildChatSectionUI( mailserversService, sharedUrlsService, setChatAsActive = false, - insertIntoModel = false + insertIntoModel = false, + isSectionBuild = true, )) + self.updateParentBadgeNotifications() + self.view.chatsModel.setData(items) self.setActiveItem(selectedItemId) @@ -431,6 +436,7 @@ method onChatsLoaded*( sharedUrlsService: shared_urls_service.Service, ) = self.chatsLoaded = true + self.buildChatSectionUI(community, chats, events, settingsService, nodeConfigurationService, contactService, chatService, communityService, messageService, mailserversService, sharedUrlsService) @@ -570,21 +576,17 @@ proc updateParentBadgeNotifications(self: Module) = unviewedMentionsCount ) -proc updateChatLocked(self: Module, chatId: string) = +proc updateChatLocked(self: Module, communityChat: ChatDto) = if not self.controller.isCommunity(): return - let communityId = self.controller.getMySectionId() - let locked = self.controller.checkChatIsLocked(communityId, chatId) - self.view.chatsModel().setItemLocked(chatId, locked) + self.view.chatsModel().setItemLocked(communityChat.id, communityChat.tokenGated and not communityChat.canPost) -proc updatePermissionsRequiredOnChat(self: Module, chatId: string) = +proc updatePermissionsRequiredOnChat(self: Module, communityChat: ChatDto) = if not self.controller.isCommunity(): return - let communityId = self.controller.getMySectionId - let requiresPermissions = self.controller.checkChatHasPermissions(communityId, chatId) - self.view.chatsModel().setItemPermissionsRequired(chatId, requiresPermissions) + self.view.chatsModel().setItemPermissionsRequired(communityChat.id, communityChat.tokenGated) -proc updateBadgeNotifications(self: Module, chat: ChatDto, hasUnreadMessages: bool, unviewedMentionsCount: int) = +proc updateBadgeNotifications(self: Module, chat: ChatDto, hasUnreadMessages: bool, unviewedMentionsCount: int, skipParentBadge: bool = false) = let chatId = chat.id if self.chatsLoaded: @@ -601,7 +603,9 @@ proc updateBadgeNotifications(self: Module, chat: ChatDto, hasUnreadMessages: bo let hasUnreadMessages = self.controller.categoryHasUnreadMessages(communityChat.communityId, communityChat.categoryId) self.view.chatsModel().setCategoryHasUnreadMessages(communityChat.categoryId, hasUnreadMessages) - self.updateParentBadgeNotifications() + # We skip evaluation the parent badge when initiating the section, since it's heavy and can be done at the end + if not skipParentBadge: + self.updateParentBadgeNotifications() method updateLastMessageTimestamp*(self: Module, chatId: string, lastMessageTimestamp: int) = self.view.chatsModel().updateLastMessageTimestampOnItemById(chatId, lastMessageTimestamp) @@ -701,6 +705,7 @@ proc getChatItemFromChatDto( var hideIfPermissionsNotMet = false var viewersCanPostReactions = true var missingEncryptionKey = false + var tokenGated = false if self.controller.isCommunity: # NOTE: workaround for new community chat, which is delivered in chatDto before the community will know about that if community.hasCommunityChat(chatDto.id): @@ -712,6 +717,7 @@ proc getChatItemFromChatDto( hideIfPermissionsNotMet = communityChat.hideIfPermissionsNotMet viewersCanPostReactions = communityChat.viewersCanPostReactions missingEncryptionKey = communityChat.missingEncryptionKey + tokenGated = communityChat.tokenGated else: canPost = chatDto.canPost canView = chatDto.canView @@ -744,14 +750,8 @@ proc getChatItemFromChatDto( categoryOpened, onlineStatus = onlineStatus, loaderActive = setChatAsActive, - locked = if self.controller.isCommunity: - self.controller.checkChatIsLocked(self.controller.getMySectionId(), chatDto.id) - else: - false, - requiresPermissions = if self.controller.isCommunity: - self.controller.checkChatHasPermissions(self.controller.getMySectionId(), chatDto.id) - else: - false, + locked = tokenGated and not canPost, + requiresPermissions = tokenGated, canPost = canPost, canView = canView, canPostReactions = canPostReactions, @@ -957,9 +957,9 @@ method changeMutedOnChat*(self: Module, chatId: string, muted: bool) = proc changeCanPostValues*(self: Module, chatId: string, canPost, canView, canPostReactions, viewersCanPostReactions: bool) = self.view.chatsModel().changeCanPostValues(chatId, canPost, canView, canPostReactions, viewersCanPostReactions) -proc updateChatsRequiredPermissions(self: Module, chats: seq[ChatDto]) = - for chat in chats: - self.updatePermissionsRequiredOnChat(chat.id) +proc updateChatsRequiredPermissions(self: Module, communityChats: seq[ChatDto]) = + for communityChat in communityChats: + self.updatePermissionsRequiredOnChat(communityChat) proc displayTokenPermissionChangeNotification(self: Module, title: string, message: string, community: CommunityDto, tokenPermission: CommunityTokenPermissionDto) = if self.showPermissionUpdateNotification(community, tokenPermission): @@ -969,17 +969,17 @@ method onCommunityTokenPermissionDeleted*(self: Module, communityId: string, tok self.view.tokenPermissionsModel.removeItemWithId(tokenPermission.id) self.reevaluateRequiresTokenPermissionToJoin() let community = self.controller.getMyCommunity() - let chats = community.getCommunityChats(tokenPermission.chatIds) + let communityChats = community.getCommunityChats(tokenPermission.chatIds) - self.updateChatsRequiredPermissions(chats) + self.updateChatsRequiredPermissions(communityChats) self.displayTokenPermissionChangeNotification("Community permission deleted", "A token permission has been removed", community, tokenPermission) method onCommunityTokenPermissionCreated*(self: Module, communityId: string, tokenPermission: CommunityTokenPermissionDto) = let community = self.controller.getMyCommunity() - let chats = community.getCommunityChats(tokenPermission.chatIds) - let tokenPermissionItem = buildTokenPermissionItem(tokenPermission, chats) + let communityChats = community.getCommunityChats(tokenPermission.chatIds) + let tokenPermissionItem = buildTokenPermissionItem(tokenPermission, communityChats) - self.updateChatsRequiredPermissions(chats) + self.updateChatsRequiredPermissions(communityChats) self.view.tokenPermissionsModel.addItem(tokenPermissionItem) self.reevaluateRequiresTokenPermissionToJoin() self.displayTokenPermissionChangeNotification("Community permission created", "A token permission has been added", community, tokenPermission) @@ -1079,8 +1079,9 @@ proc updateChannelPermissionViewData*( let viewOnlyUpdated = self.updateTokenPermissionModel(viewOnlyPermissions.permissions, community) let viewAndPostUpdated = self.updateTokenPermissionModel(viewAndPostPermissions.permissions, community) if viewOnlyUpdated or viewAndPostUpdated: - self.updatePermissionsRequiredOnChat(chatId) - self.updateChatLocked(chatId) + let communityChat = community.getCommunityChat(chatId) + self.updatePermissionsRequiredOnChat(communityChat) + self.updateChatLocked(communityChat) if self.chatContentModules.hasKey(chatId): self.chatContentModules[chatId].setPermissionsCheckOngoing(false) @@ -1452,13 +1453,13 @@ method addOrUpdateChat(self: Module, sharedUrlsService: shared_urls_service.Service, setChatAsActive: bool = true, insertIntoModel: bool = true, + isSectionBuild: bool = false, ): chat_item.Item = let sectionId = self.controller.getMySectionId() if belongsToCommunity and sectionId != chat.communityId or not belongsToCommunity and sectionId != singletonInstance.userProfile.getPubKey(): return - - self.updateBadgeNotifications(chat, chat.unviewedMessagesCount > 0, chat.unviewedMentionsCount) + self.updateBadgeNotifications(chat, chat.unviewedMessagesCount > 0, chat.unviewedMentionsCount, skipParentBadge = isSectionBuild) if not self.chatsLoaded: return @@ -1478,8 +1479,8 @@ method addOrUpdateChat(self: Module, self.changeMutedOnChat(chat.id, chat.muted) self.changeCanPostValues(chat.id, result.canPost, result.canView, result.canPostReactions, result.viewersCanPostReactions) - self.updatePermissionsRequiredOnChat(chat.id) - self.updateChatLocked(chat.id) + self.view.chatsModel().setItemPermissionsRequired(chat.id, result.requiresPermissions) + self.view.chatsModel().setItemLocked(chat.id, result.locked) self.view.chatsModel().updateMissingEncryptionKey(chat.id, result.missingEncryptionKey) if (chat.chatType == ChatType.PrivateGroupChat): self.onGroupChatDetailsUpdated(chat.id, chat.name, chat.color, chat.icon) diff --git a/src/app/modules/main/module.nim b/src/app/modules/main/module.nim index d76bfea647..a05a33bb6c 100644 --- a/src/app/modules/main/module.nim +++ b/src/app/modules/main/module.nim @@ -656,6 +656,8 @@ method onChatsLoaded*[T]( myPubKey, sectionIsMuted = false ) + var items: seq[SectionItem] = @[] + let personalChatSectionItem = initItem( myPubKey, sectionType = SectionType.Chat, @@ -671,7 +673,7 @@ method onChatsLoaded*[T]( isMember = true, muted = false, ) - self.view.model().addItem(personalChatSectionItem) + items.add(personalChatSectionItem) if activeSectionId == personalChatSectionItem.id: activeSection = personalChatSectionItem @@ -699,12 +701,14 @@ method onChatsLoaded*[T]( networkService ) let communitySectionItem = self.createCommunitySectionItem(community) - self.view.model().addItem(communitySectionItem) + items.add(communitySectionItem) if activeSectionId == communitySectionItem.id: activeSection = communitySectionItem self.chatSectionModules[community.id].load() + self.view.model().addItems(items) + # Set active section if it is one of the channel sections if not activeSection.isEmpty(): self.setActiveSection(activeSection) diff --git a/src/app/modules/shared_models/section_model.nim b/src/app/modules/shared_models/section_model.nim index 0a6b7e8f40..355584add0 100644 --- a/src/app/modules/shared_models/section_model.nim +++ b/src/app/modules/shared_models/section_model.nim @@ -224,7 +224,6 @@ QtObject: return true return false - proc addItem*(self: SectionModel, item: SectionItem) = let parentModelIndex = newQModelIndex() defer: parentModelIndex.delete @@ -247,6 +246,20 @@ QtObject: self.countChanged() + proc addItems*(self: SectionModel, items: seq[SectionItem]) = + if items.len == 0: + return + + let parentModelIndex = newQModelIndex() + defer: parentModelIndex.delete + + let first = self.items.len + let last = first + items.len - 1 + self.beginInsertRows(parentModelIndex, first, last) + self.items.add(items) + self.endInsertRows() + self.countChanged() + proc getItemIndex*(self: SectionModel, id: string): int = var i = 0 for item in self.items: diff --git a/src/app_service/service/community/service.nim b/src/app_service/service/community/service.nim index a63c50dc7a..1ce877b947 100644 --- a/src/app_service/service/community/service.nim +++ b/src/app_service/service/community/service.nim @@ -2247,22 +2247,6 @@ QtObject: else: return community.declinedRequestsToJoin[indexDeclined].publicKey - proc checkChatIsLocked*(self: Service, communityId: string, chatId: string): bool = - if not self.communities.hasKey(communityId): - return false - - let community = self.getCommunityById(communityId) - return community.channelPermissions.channels.hasKey(chatId) and not community.channelPermissions.channels[chatId].viewAndPostPermissions.satisfied - - proc checkChatHasPermissions*(self: Service, communityId: string, chatId: string): bool = - let community = self.getCommunityById(communityId) - for id, tokenPermission in community.tokenPermissions: - if TokenPermissionType(tokenPermission.`type`) == TokenPermissionType.View or TokenPermissionType(tokenPermission.`type`) == TokenPermissionType.ViewAndPost: - for id in tokenPermission.chatIds: - if id == chatId: - return true - return false - proc shareCommunityUrlWithChatKey*(self: Service, communityId: string): string = try: let response = status_go.shareCommunityUrlWithChatKey(communityId)