From 685b311401f471edda136cfc8f8873b3133ca7d8 Mon Sep 17 00:00:00 2001 From: Agniva De Sarker Date: Thu, 28 Oct 2021 09:57:54 +0530 Subject: [PATCH] ThreadStore: Remove quadratic complexity for participants population (#18755) * ThreadStore: Remove quadratic complexity for participants population While populating the participants, we would traverse the global list twice to get the users for that thread. For threads with large number of users, this can take considerable time. We replace that with a map to change it to O(n). While here, we improve memory usage of this code by pre-allocating slices and maps. And also add some comments to help readers. https://community-daily.mattermost.com/boards/workspace/zyoahc9uapdn3xdptac6jb69ic/285b80a3-257d-41f6-8cf4-ed80ca9d92e5/495cdb4d-c13a-4992-8eb9-80cfee2819a4?c=c9aa17f3-b918-4a78-86aa-01562e418634 ```release-note NONE ``` * Apply in post store as well ```release-note NONE ``` Co-authored-by: Mattermod --- store/sqlstore/post_store.go | 26 ++++++++++++-------------- store/sqlstore/thread_store.go | 29 ++++++++++++++++------------- 2 files changed, 28 insertions(+), 27 deletions(-) diff --git a/store/sqlstore/post_store.go b/store/sqlstore/post_store.go index cafca3d666..0364ca3802 100644 --- a/store/sqlstore/post_store.go +++ b/store/sqlstore/post_store.go @@ -819,33 +819,31 @@ func (s *SqlPostStore) prepareThreadedResponse(posts []*postWithExtra, extended, } } } - var users []*model.User + // usersMap is the global profile map of all participants from all threads. + usersMap := make(map[string]*model.User, len(userIds)) if extended { - var err error - users, err = s.User().GetProfileByIds(context.Background(), userIds, &store.UserGetByIdsOpts{}, true) + users, err := s.User().GetProfileByIds(context.Background(), userIds, &store.UserGetByIdsOpts{}, true) if err != nil { return nil, err } + for _, user := range users { + usersMap[user.Id] = user + } } else { for _, userId := range userIds { - users = append(users, &model.User{Id: userId}) + usersMap[userId] = &model.User{Id: userId} } } + processPost := func(p *postWithExtra) error { p.Post.ReplyCount = p.ThreadReplyCount if p.IsFollowing != nil { p.Post.IsFollowing = model.NewBool(*p.IsFollowing) } - for _, th := range p.ThreadParticipants { - var participant *model.User - for _, u := range users { - if u.Id == th { - participant = u - break - } - } - if participant == nil { - return errors.New("cannot find thread participant with id=" + th) + for _, userID := range p.ThreadParticipants { + participant, ok := usersMap[userID] + if !ok { + return errors.New("cannot find thread participant with id=" + userID) } p.Post.Participants = append(p.Post.Participants, participant) } diff --git a/store/sqlstore/thread_store.go b/store/sqlstore/thread_store.go index 8a1cce8f20..9592a1eee9 100644 --- a/store/sqlstore/thread_store.go +++ b/store/sqlstore/thread_store.go @@ -291,7 +291,10 @@ func (s *SqlThreadStore) GetThreadsForUser(userId, teamId string, opts model.Get } totalUnreadThreads := totalUnreadThreadsResult.Data.(int64) + // userIds is the de-duped list of participant ids from all threads. var userIds []string + // userIdMap is the map of participant ids from all threads. + // Used to generate userIds userIdMap := map[string]bool{} result := &model.Threads{ @@ -315,30 +318,30 @@ func (s *SqlThreadStore) GetThreadsForUser(userId, teamId string, opts model.Get } } } - var users []*model.User + // usersMap is the global profile map of all participants from all threads. + usersMap := make(map[string]*model.User, len(userIds)) if opts.Extended { - var err error - users, err = s.User().GetProfileByIds(context.Background(), userIds, &store.UserGetByIdsOpts{}, true) + users, err := s.User().GetProfileByIds(context.Background(), userIds, &store.UserGetByIdsOpts{}, true) if err != nil { return nil, errors.Wrapf(err, "failed to get threads for user id=%s", userId) } + for _, user := range users { + usersMap[user.Id] = user + } } else { for _, userId := range userIds { - users = append(users, &model.User{Id: userId}) + usersMap[userId] = &model.User{Id: userId} } } + result.Threads = make([]*model.ThreadResponse, 0, len(threads)) for _, thread := range threads { - var participants []*model.User + participants := make([]*model.User, 0, len(thread.Participants)) + // We get the user profiles for only a single thread filtered from the + // global users map. for _, participantId := range thread.Participants { - var participant *model.User - for _, u := range users { - if u.Id == participantId { - participant = u - break - } - } - if participant == nil { + participant, ok := usersMap[participantId] + if !ok { return nil, errors.New("cannot find thread participant with id=" + participantId) } participants = append(participants, participant)