From 451aef7a1c177b58c4519239a65dbfbab32b4888 Mon Sep 17 00:00:00 2001 From: Unknwon Date: Fri, 10 Mar 2017 14:12:46 -0500 Subject: [PATCH] release: improve page load performance Previously, we load all releases of a repository which could hurt performance when the repository has a lot of releases. Now we're able to only load releases in current page view we need to show by matching with 'tag_name'. --- gogs.go | 2 +- models/release.go | 99 +++++++++++++------ routers/repo/release.go | 82 ++++++++------- templates/.VERSION | 2 +- vendor/github.com/gogits/git-module/git.go | 2 +- .../github.com/gogits/git-module/repo_tag.go | 2 +- vendor/vendor.json | 6 +- 7 files changed, 120 insertions(+), 75 deletions(-) diff --git a/gogs.go b/gogs.go index 6c40e13db..50cccff57 100644 --- a/gogs.go +++ b/gogs.go @@ -16,7 +16,7 @@ import ( "github.com/gogits/gogs/modules/setting" ) -const APP_VER = "0.10.12.0309" +const APP_VER = "0.10.12.0310" func init() { setting.AppVer = APP_VER diff --git a/models/release.go b/models/release.go index f470f54d9..53b2e631d 100644 --- a/models/release.go +++ b/models/release.go @@ -51,6 +51,26 @@ func (r *Release) AfterSet(colName string, _ xorm.Cell) { } } +func (r *Release) loadAttributes(e Engine) (err error) { + if r.Publisher == nil { + r.Publisher, err = getUserByID(e, r.PublisherID) + if err != nil { + if IsErrUserNotExist(err) { + r.PublisherID = -1 + r.Publisher = NewGhostUser() + } else { + return fmt.Errorf("getUserByID.(Publisher) [%d]: %v", r.PublisherID, err) + } + } + } + + return nil +} + +func (r *Release) LoadAttributes() error { + return r.loadAttributes(x) +} + // IsReleaseExist returns true if release with given tag name already exists. func IsReleaseExist(repoID int64, tagName string) (bool, error) { if len(tagName) == 0 { @@ -60,31 +80,31 @@ func IsReleaseExist(repoID int64, tagName string) (bool, error) { return x.Get(&Release{RepoID: repoID, LowerTagName: strings.ToLower(tagName)}) } -func createTag(gitRepo *git.Repository, rel *Release) error { +func createTag(gitRepo *git.Repository, r *Release) error { // Only actual create when publish. - if !rel.IsDraft { - if !gitRepo.IsTagExist(rel.TagName) { - commit, err := gitRepo.GetBranchCommit(rel.Target) + if !r.IsDraft { + if !gitRepo.IsTagExist(r.TagName) { + commit, err := gitRepo.GetBranchCommit(r.Target) if err != nil { return fmt.Errorf("GetBranchCommit: %v", err) } // Trim '--' prefix to prevent command line argument vulnerability. - rel.TagName = strings.TrimPrefix(rel.TagName, "--") - if err = gitRepo.CreateTag(rel.TagName, commit.ID.String()); err != nil { + r.TagName = strings.TrimPrefix(r.TagName, "--") + if err = gitRepo.CreateTag(r.TagName, commit.ID.String()); err != nil { if strings.Contains(err.Error(), "is not a valid tag name") { - return ErrInvalidTagName{rel.TagName} + return ErrInvalidTagName{r.TagName} } return err } } else { - commit, err := gitRepo.GetTagCommit(rel.TagName) + commit, err := gitRepo.GetTagCommit(r.TagName) if err != nil { return fmt.Errorf("GetTagCommit: %v", err) } - rel.Sha1 = commit.ID.String() - rel.NumCommits, err = commit.CommitsCount() + r.Sha1 = commit.ID.String() + r.NumCommits, err = commit.CommitsCount() if err != nil { return fmt.Errorf("CommitsCount: %v", err) } @@ -94,19 +114,19 @@ func createTag(gitRepo *git.Repository, rel *Release) error { } // CreateRelease creates a new release of repository. -func CreateRelease(gitRepo *git.Repository, rel *Release) error { - isExist, err := IsReleaseExist(rel.RepoID, rel.TagName) +func CreateRelease(gitRepo *git.Repository, r *Release) error { + isExist, err := IsReleaseExist(r.RepoID, r.TagName) if err != nil { return err } else if isExist { - return ErrReleaseAlreadyExist{rel.TagName} + return ErrReleaseAlreadyExist{r.TagName} } - if err = createTag(gitRepo, rel); err != nil { + if err = createTag(gitRepo, r); err != nil { return err } - rel.LowerTagName = strings.ToLower(rel.TagName) - _, err = x.InsertOne(rel) + r.LowerTagName = strings.ToLower(r.TagName) + _, err = x.InsertOne(r) return err } @@ -119,53 +139,68 @@ func GetRelease(repoID int64, tagName string) (*Release, error) { return nil, ErrReleaseNotExist{0, tagName} } - rel := &Release{RepoID: repoID, LowerTagName: strings.ToLower(tagName)} - _, err = x.Get(rel) - return rel, err + r := &Release{RepoID: repoID, LowerTagName: strings.ToLower(tagName)} + if _, err = x.Get(r); err != nil { + return nil, fmt.Errorf("Get: %v", err) + } + + return r, r.LoadAttributes() } // GetReleaseByID returns release with given ID. func GetReleaseByID(id int64) (*Release, error) { - rel := new(Release) - has, err := x.Id(id).Get(rel) + r := new(Release) + has, err := x.Id(id).Get(r) if err != nil { return nil, err } else if !has { return nil, ErrReleaseNotExist{id, ""} } - return rel, nil + return r, r.LoadAttributes() } -// GetReleasesByRepoID returns a list of releases of repository. -func GetReleasesByRepoID(repoID int64) (rels []*Release, err error) { - err = x.Desc("created_unix").Find(&rels, Release{RepoID: repoID}) - return rels, err +// GetPublishedReleasesByRepoID returns a list of published releases of repository. +// If matches is not empty, only published releases in matches will be returned. +// In any case, drafts won't be returned by this function. +func GetPublishedReleasesByRepoID(repoID int64, matches ...string) ([]*Release, error) { + sess := x.Where("repo_id = ?", repoID).And("is_draft = ?", false).Desc("created_unix") + if len(matches) > 0 { + sess.In("tag_name", matches) + } + releases := make([]*Release, 0, 5) + return releases, sess.Find(&releases, new(Release)) +} + +// GetDraftReleasesByRepoID returns all draft releases of repository. +func GetDraftReleasesByRepoID(repoID int64) ([]*Release, error) { + releases := make([]*Release, 0) + return releases, x.Where("repo_id = ?", repoID).And("is_draft = ?", true).Find(&releases) } type ReleaseSorter struct { - rels []*Release + releases []*Release } func (rs *ReleaseSorter) Len() int { - return len(rs.rels) + return len(rs.releases) } func (rs *ReleaseSorter) Less(i, j int) bool { - diffNum := rs.rels[i].NumCommits - rs.rels[j].NumCommits + diffNum := rs.releases[i].NumCommits - rs.releases[j].NumCommits if diffNum != 0 { return diffNum > 0 } - return rs.rels[i].Created.After(rs.rels[j].Created) + return rs.releases[i].Created.After(rs.releases[j].Created) } func (rs *ReleaseSorter) Swap(i, j int) { - rs.rels[i], rs.rels[j] = rs.rels[j], rs.rels[i] + rs.releases[i], rs.releases[j] = rs.releases[j], rs.releases[i] } // SortReleases sorts releases by number of commits and created time. func SortReleases(rels []*Release) { - sorter := &ReleaseSorter{rels: rels} + sorter := &ReleaseSorter{releases: rels} sort.Sort(sorter) } diff --git a/routers/repo/release.go b/routers/repo/release.go index 7d35b3180..094732422 100644 --- a/routers/repo/release.go +++ b/routers/repo/release.go @@ -59,35 +59,26 @@ func Releases(ctx *context.Context) { return } - // FIXME: should only get releases match tags result and drafts. - releases, err := models.GetReleasesByRepoID(ctx.Repo.Repository.ID) + releases, err := models.GetPublishedReleasesByRepoID(ctx.Repo.Repository.ID, tagsResult.Tags...) if err != nil { - ctx.Handle(500, "GetReleasesByRepoID", err) + ctx.Handle(500, "GetPublishedReleasesByRepoID", err) return } // Temproray cache commits count of used branches to speed up. countCache := make(map[string]int64) - drafts := make([]*models.Release, 0, 1) - tags := make([]*models.Release, len(tagsResult.Tags)) + results := make([]*models.Release, len(tagsResult.Tags)) for i, rawTag := range tagsResult.Tags { for j, r := range releases { - if r == nil || - (r.IsDraft && !ctx.Repo.IsOwner()) || - (!r.IsDraft && r.TagName != rawTag) { + if r == nil || r.TagName != rawTag { continue } releases[j] = nil // Mark as used. - r.Publisher, err = models.GetUserByID(r.PublisherID) - if err != nil { - if models.IsErrUserNotExist(err) { - r.Publisher = models.NewGhostUser() - } else { - ctx.Handle(500, "GetUserByID", err) - return - } + if err = r.LoadAttributes(); err != nil { + ctx.Handle(500, "LoadAttributes", err) + return } if err := calReleaseNumCommitsBehind(ctx.Repo, r, countCache); err != nil { @@ -96,49 +87,68 @@ func Releases(ctx *context.Context) { } r.Note = markdown.RenderString(r.Note, ctx.Repo.RepoLink, ctx.Repo.Repository.ComposeMetas()) - if r.TagName == rawTag { - tags[i] = r - break - } - - if r.IsDraft { - drafts = append(drafts, r) - } + results[i] = r + break } - if tags[i] == nil { + // No published release matches this tag + if results[i] == nil { commit, err := ctx.Repo.GitRepo.GetTagCommit(rawTag) if err != nil { ctx.Handle(500, "GetTagCommit", err) return } - tags[i] = &models.Release{ + results[i] = &models.Release{ Title: rawTag, TagName: rawTag, Sha1: commit.ID.String(), } - tags[i].NumCommits, err = commit.CommitsCount() + results[i].NumCommits, err = commit.CommitsCount() if err != nil { ctx.Handle(500, "CommitsCount", err) return } - tags[i].NumCommitsBehind = ctx.Repo.CommitsCount - tags[i].NumCommits + results[i].NumCommitsBehind = ctx.Repo.CommitsCount - results[i].NumCommits + } + } + models.SortReleases(results) + + // Only show drafts if user is viewing the latest page + var drafts []*models.Release + if tagsResult.HasLatest { + drafts, err = models.GetDraftReleasesByRepoID(ctx.Repo.Repository.ID) + if err != nil { + ctx.Handle(500, "GetDraftReleasesByRepoID", err) + return + } + + for _, r := range drafts { + if err = r.LoadAttributes(); err != nil { + ctx.Handle(500, "LoadAttributes", err) + return + } + + if err := calReleaseNumCommitsBehind(ctx.Repo, r, countCache); err != nil { + ctx.Handle(500, "calReleaseNumCommitsBehind", err) + return + } + + r.Note = markdown.RenderString(r.Note, ctx.Repo.RepoLink, ctx.Repo.Repository.ComposeMetas()) + } + + if len(drafts) > 0 { + results = append(drafts, results...) } } - models.SortReleases(tags) - if len(drafts) > 0 && tagsResult.HasLatest { - tags = append(drafts, tags...) - } - - ctx.Data["Releases"] = tags + ctx.Data["Releases"] = results ctx.Data["HasPrevious"] = !tagsResult.HasLatest ctx.Data["ReachEnd"] = tagsResult.ReachEnd ctx.Data["PreviousAfter"] = tagsResult.PreviousAfter - if len(tags) > 0 { - ctx.Data["NextAfter"] = tags[len(tags)-1].TagName + if len(results) > 0 { + ctx.Data["NextAfter"] = results[len(results)-1].TagName } ctx.HTML(200, RELEASES) } diff --git a/templates/.VERSION b/templates/.VERSION index beff72a83..2ed58e6a1 100644 --- a/templates/.VERSION +++ b/templates/.VERSION @@ -1 +1 @@ -0.10.12.0309 \ No newline at end of file +0.10.12.0310 \ No newline at end of file diff --git a/vendor/github.com/gogits/git-module/git.go b/vendor/github.com/gogits/git-module/git.go index fc8341968..3eef5db5f 100644 --- a/vendor/github.com/gogits/git-module/git.go +++ b/vendor/github.com/gogits/git-module/git.go @@ -10,7 +10,7 @@ import ( "time" ) -const _VERSION = "0.4.12" +const _VERSION = "0.4.13" func Version() string { return _VERSION diff --git a/vendor/github.com/gogits/git-module/repo_tag.go b/vendor/github.com/gogits/git-module/repo_tag.go index b7e96bdb8..4cef496b8 100644 --- a/vendor/github.com/gogits/git-module/repo_tag.go +++ b/vendor/github.com/gogits/git-module/repo_tag.go @@ -171,7 +171,7 @@ func (repo *Repository) GetTagsAfter(after string, limit int) (*TagsResult, erro } if allTags[i] == after { hasMatch = true - if limit > 0 && i-limit > 0 { + if limit > 0 && i-limit >= 0 { previousAfter = allTags[i-limit] } continue diff --git a/vendor/vendor.json b/vendor/vendor.json index 7bccaadbd..4f45a4918 100644 --- a/vendor/vendor.json +++ b/vendor/vendor.json @@ -159,10 +159,10 @@ "revisionTime": "2016-08-10T03:50:02Z" }, { - "checksumSHA1": "RnWB5UDcTpSGepvWnwic1SyRZrc=", + "checksumSHA1": "sygMoTVpNmOTbsnZbgLR904p5GE=", "path": "github.com/gogits/git-module", - "revision": "1b9552bab7499e5cbd96f6b550aaa0038faf6b67", - "revisionTime": "2017-02-19T18:16:29Z" + "revision": "2c083b82191752340c76271876671e18130ad662", + "revisionTime": "2017-03-10T19:06:55Z" }, { "checksumSHA1": "Rvj0LCHGhFQyIM7MzBPt1iRP89c=",