From 10ff864e0c588313ec0cdfa91c4305d410299b4f Mon Sep 17 00:00:00 2001 From: kolaente Date: Sat, 2 Mar 2024 14:27:11 +0100 Subject: [PATCH] fix(projects): load projects only one when fetching subscriptions for a bunch of projects at once This change ensures already loaded projects are passed down when fetching their subscription instead of re-loading each project with a single sql statement. When loading all projects, this meant all projects were loaded twice, which was highly inefficient. This roughly added 25ms to each request, assuming the per page limit was maxed out at 50 projects. Empirical testing shows this change reduces load times by ~20ms. Because the request is already pretty fast, this is ~30% of the overall request time, making the loading of projects now even faster --- pkg/models/project.go | 31 ++++++++-- pkg/models/subscription.go | 87 +++++++++++++++++++---------- pkg/models/task_overdue_reminder.go | 2 +- pkg/models/task_reminder.go | 2 +- pkg/models/tasks.go | 7 ++- 5 files changed, 91 insertions(+), 38 deletions(-) diff --git a/pkg/models/project.go b/pkg/models/project.go index 7bd98dfea..0e5992156 100644 --- a/pkg/models/project.go +++ b/pkg/models/project.go @@ -316,8 +316,8 @@ func GetProjectSimplByTaskID(s *xorm.Session, taskID int64) (l *Project, err err return &project, nil } -// GetProjectsSimplByTaskIDs gets a list of projects by a task ids -func GetProjectsSimplByTaskIDs(s *xorm.Session, taskIDs []int64) (ps map[int64]*Project, err error) { +// GetProjectsMapSimplByTaskIDs gets a list of projects by a task ids +func GetProjectsMapSimplByTaskIDs(s *xorm.Session, taskIDs []int64) (ps map[int64]*Project, err error) { ps = make(map[int64]*Project) err = s. Select("projects.*"). @@ -328,8 +328,18 @@ func GetProjectsSimplByTaskIDs(s *xorm.Session, taskIDs []int64) (ps map[int64]* return } -// GetProjectsByIDs returns a map of projects from a slice with project ids -func GetProjectsByIDs(s *xorm.Session, projectIDs []int64) (projects map[int64]*Project, err error) { +func GetProjectsSimplByTaskIDs(s *xorm.Session, taskIDs []int64) (ps []*Project, err error) { + err = s. + Select("projects.*"). + Table(Project{}). + Join("INNER", "tasks", "projects.id = tasks.project_id"). + In("tasks.id", taskIDs). + Find(&ps) + return +} + +// GetProjectsMapByIDs returns a map of projects from a slice with project ids +func GetProjectsMapByIDs(s *xorm.Session, projectIDs []int64) (projects map[int64]*Project, err error) { projects = make(map[int64]*Project, len(projectIDs)) if len(projectIDs) == 0 { @@ -340,6 +350,17 @@ func GetProjectsByIDs(s *xorm.Session, projectIDs []int64) (projects map[int64]* return } +func GetProjectsByIDs(s *xorm.Session, projectIDs []int64) (projects []*Project, err error) { + projects = make([]*Project, 0, len(projectIDs)) + + if len(projectIDs) == 0 { + return + } + + err = s.In("id", projectIDs).Find(&projects) + return +} + type projectOptions struct { search string user *user.User @@ -559,7 +580,7 @@ func addProjectDetails(s *xorm.Session, projects []*Project, a web.Auth) (err er return err } - subscriptions, err := GetSubscriptions(s, SubscriptionEntityProject, projectIDs, a) + subscriptions, err := GetSubscriptionsForProjects(s, projects, a) if err != nil { log.Errorf("An error occurred while getting project subscriptions for a project: %s", err.Error()) subscriptions = make(map[int64][]*Subscription) diff --git a/pkg/models/subscription.go b/pkg/models/subscription.go index dc5371e33..020978b90 100644 --- a/pkg/models/subscription.go +++ b/pkg/models/subscription.go @@ -223,7 +223,11 @@ func GetSubscriptions(s *xorm.Session, entityType SubscriptionEntityType, entity switch entityType { case SubscriptionEntityProject: - return getSubscriptionsForProjects(s, entityIDs, u) + projects, err := GetProjectsByIDs(s, entityIDs) + if err != nil { + return nil, err + } + return GetSubscriptionsForProjects(s, projects, u) case SubscriptionEntityTask: subs, err := getSubscriptionsForTasks(s, entityIDs, u) if err != nil { @@ -232,22 +236,34 @@ func GetSubscriptions(s *xorm.Session, entityType SubscriptionEntityType, entity // If the task does not have a subscription directly or from its project, get the one // from the parent and return it instead. + var taskIDsWithoutSubscription []int64 for _, eID := range entityIDs { if _, has := subs[eID]; has { continue } - task, err := GetTaskByIDSimple(s, eID) - if err != nil { - return nil, err - } - projectSubscriptions, err := getSubscriptionsForProjects(s, []int64{task.ProjectID}, u) - if err != nil { - return nil, err - } - for _, subscription := range projectSubscriptions { - subs[eID] = subscription // The first project subscription is the subscription we're looking for - break + taskIDsWithoutSubscription = append(taskIDsWithoutSubscription, eID) + } + + projects, err := GetProjectsSimplByTaskIDs(s, taskIDsWithoutSubscription) + if err != nil { + return nil, err + } + + tasks, err := GetTasksSimpleByIDs(s, taskIDsWithoutSubscription) + if err != nil { + return nil, err + } + + projectSubscriptions, err := GetSubscriptionsForProjects(s, projects, u) + if err != nil { + return nil, err + } + + for _, task := range tasks { + sub, has := projectSubscriptions[task.ProjectID] + if has { + subs[task.ID] = sub } } @@ -257,48 +273,59 @@ func GetSubscriptions(s *xorm.Session, entityType SubscriptionEntityType, entity return } -func getSubscriptionsForProjects(s *xorm.Session, projectIDs []int64, u *user.User) (projectsToSubscriptions map[int64][]*Subscription, err error) { - origEntityIDs := projectIDs - var ps = make(map[int64]*Project) +func GetSubscriptionsForProjects(s *xorm.Session, projects []*Project, a web.Auth) (projectsToSubscriptions map[int64][]*Subscription, err error) { + u, is := a.(*user.User) + if u != nil && !is { + return + } - for _, eID := range projectIDs { - if eID < 1 { + var ps = make(map[int64]*Project) + origProjectIDs := make([]int64, 0, len(projects)) + allProjectIDs := make([]int64, 0, len(projects)) + + for _, p := range projects { + ps[p.ID] = p + origProjectIDs = append(origProjectIDs, p.ID) + allProjectIDs = append(allProjectIDs, p.ID) + } + + // We can't just use the projects we have, we need to fetch the parents + // because they may not be loaded in the same object + + for _, p := range projects { + if p.ParentProjectID == 0 { continue } - ps[eID], err = GetProjectSimpleByID(s, eID) - if err != nil && IsErrProjectDoesNotExist(err) { - // If the project does not exist, it might got deleted. There could still be subscribers though. - delete(ps, eID) + + if _, has := ps[p.ParentProjectID]; has { continue } - if err != nil { - return nil, err - } - err = ps[eID].GetAllParentProjects(s) + + err = ps[p.ID].GetAllParentProjects(s) if err != nil { return nil, err } parentIDs := []int64{} - var parent = ps[eID].ParentProject + var parent = ps[p.ID].ParentProject for parent != nil { parentIDs = append(parentIDs, parent.ID) parent = parent.ParentProject } // Now we have all parent ids - projectIDs = append(projectIDs, parentIDs...) // the child project id is already in there + allProjectIDs = append(allProjectIDs, parentIDs...) // the child project id is already in there } var subscriptions []*Subscription if u != nil { err = s. Where("user_id = ?", u.ID). - And(getSubscriberCondForEntities(SubscriptionEntityProject, projectIDs)). + And(getSubscriberCondForEntities(SubscriptionEntityProject, allProjectIDs)). Find(&subscriptions) } else { err = s. - And(getSubscriberCondForEntities(SubscriptionEntityProject, projectIDs)). + And(getSubscriberCondForEntities(SubscriptionEntityProject, allProjectIDs)). Find(&subscriptions) } if err != nil { @@ -313,7 +340,7 @@ func getSubscriptionsForProjects(s *xorm.Session, projectIDs []int64, u *user.Us // Rearrange so that subscriptions trickle down - for _, eID := range origEntityIDs { + for _, eID := range origProjectIDs { // If the current project does not have a subscription, climb up the tree until a project has one, // then use that subscription for all child projects _, has := projectsToSubscriptions[eID] diff --git a/pkg/models/task_overdue_reminder.go b/pkg/models/task_overdue_reminder.go index d23a4ec45..337c02de0 100644 --- a/pkg/models/task_overdue_reminder.go +++ b/pkg/models/task_overdue_reminder.go @@ -139,7 +139,7 @@ func RegisterOverdueReminderCron() { } } - projects, err := GetProjectsSimplByTaskIDs(s, taskIDs) + projects, err := GetProjectsMapSimplByTaskIDs(s, taskIDs) if err != nil { log.Errorf("[Undone Overdue Tasks Reminder] Could not get projects for tasks: %s", err) return diff --git a/pkg/models/task_reminder.go b/pkg/models/task_reminder.go index 46225bb43..bc1fd0208 100644 --- a/pkg/models/task_reminder.go +++ b/pkg/models/task_reminder.go @@ -173,7 +173,7 @@ func getTasksWithRemindersDueAndTheirUsers(s *xorm.Session, now time.Time) (remi seen := make(map[int64]map[int64]bool) - projects, err := GetProjectsSimplByTaskIDs(s, taskIDs) + projects, err := GetProjectsMapSimplByTaskIDs(s, taskIDs) if err != nil { return } diff --git a/pkg/models/tasks.go b/pkg/models/tasks.go index 28c3bf309..49716f9ae 100644 --- a/pkg/models/tasks.go +++ b/pkg/models/tasks.go @@ -356,6 +356,11 @@ func GetTaskSimple(s *xorm.Session, t *Task) (task Task, err error) { return } +func GetTasksSimpleByIDs(s *xorm.Session, ids []int64) (tasks []*Task, err error) { + err = s.In("id", ids).Find(&tasks) + return +} + // GetTasksByIDs returns all tasks for a project of ids func (bt *BulkTask) GetTasksByIDs(s *xorm.Session) (err error) { for _, id := range bt.IDs { @@ -586,7 +591,7 @@ func addMoreInfoToTasks(s *xorm.Session, taskMap map[int64]*Task, a web.Auth) (e } // Get all identifiers - projects, err := GetProjectsByIDs(s, projectIDs) + projects, err := GetProjectsMapByIDs(s, projectIDs) if err != nil { return err }