From 196593e2e996aa4a59547629b870701f2b001d9b Mon Sep 17 00:00:00 2001 From: zeripath Date: Sun, 20 Jun 2021 23:39:12 +0100 Subject: [PATCH] More efficiently parse shas for shaPostProcessor (#16101) * More efficiently parse shas for shaPostProcessor The shaPostProcessor currently repeatedly calls git rev-parse --verify on both backends which is fine if there is only one thing that matches a sha - however if there are multiple things then this becomes wildly inefficient. This PR provides functions for both backends which are much faster to use. Fix #16092 * Add ShaExistCache to RenderContext Signed-off-by: Andrew Thornton Co-authored-by: 6543 <6543@obermui.de> --- modules/git/repo_branch_gogit.go | 24 ++++++++++++++++ modules/git/repo_branch_nogogit.go | 18 ++++++++++++ modules/markup/html.go | 28 ++++++++++++++++-- modules/markup/renderer.go | 46 +++++++++++++++++++++++++----- routers/web/org/home.go | 1 + routers/web/repo/issue.go | 5 ++++ routers/web/repo/milestone.go | 2 ++ routers/web/repo/projects.go | 2 ++ routers/web/repo/release.go | 2 ++ routers/web/repo/view.go | 3 ++ routers/web/user/profile.go | 1 + 11 files changed, 122 insertions(+), 10 deletions(-) diff --git a/modules/git/repo_branch_gogit.go b/modules/git/repo_branch_gogit.go index b00253f6ff..e8386b2dbd 100644 --- a/modules/git/repo_branch_gogit.go +++ b/modules/git/repo_branch_gogit.go @@ -13,6 +13,30 @@ import ( "github.com/go-git/go-git/v5/plumbing" ) +// IsObjectExist returns true if given reference exists in the repository. +func (repo *Repository) IsObjectExist(name string) bool { + if name == "" { + return false + } + + _, err := repo.gogitRepo.ResolveRevision(plumbing.Revision(name)) + + return err == nil +} + +// IsReferenceExist returns true if given reference exists in the repository. +func (repo *Repository) IsReferenceExist(name string) bool { + if name == "" { + return false + } + + reference, err := repo.gogitRepo.Reference(plumbing.ReferenceName(name), true) + if err != nil { + return false + } + return reference.Type() != plumbing.InvalidReference +} + // IsBranchExist returns true if given branch exists in current repository. func (repo *Repository) IsBranchExist(name string) bool { if name == "" { diff --git a/modules/git/repo_branch_nogogit.go b/modules/git/repo_branch_nogogit.go index 13ddcf06cf..dd34e48899 100644 --- a/modules/git/repo_branch_nogogit.go +++ b/modules/git/repo_branch_nogogit.go @@ -9,10 +9,28 @@ package git import ( "bufio" + "bytes" "io" "strings" ) +// IsObjectExist returns true if given reference exists in the repository. +func (repo *Repository) IsObjectExist(name string) bool { + if name == "" { + return false + } + + wr, rd, cancel := repo.CatFileBatchCheck() + defer cancel() + _, err := wr.Write([]byte(name + "\n")) + if err != nil { + log("Error writing to CatFileBatchCheck %v", err) + return false + } + sha, _, _, err := ReadBatchLine(rd) + return err == nil && bytes.HasPrefix(sha, []byte(strings.TrimSpace(name))) +} + // IsReferenceExist returns true if given reference exists in the repository. func (repo *Repository) IsReferenceExist(name string) bool { if name == "" { diff --git a/modules/markup/html.go b/modules/markup/html.go index 2a83b8716e..edf860da45 100644 --- a/modules/markup/html.go +++ b/modules/markup/html.go @@ -286,6 +286,7 @@ var tagCleaner = regexp.MustCompile(`<((?:/?\w+/\w+)|(?:/[\w ]+/)|(/?[hH][tT][mM var nulCleaner = strings.NewReplacer("\000", "") func postProcess(ctx *RenderContext, procs []processor, input io.Reader, output io.Writer) error { + defer ctx.Cancel() // FIXME: don't read all content to memory rawHTML, err := ioutil.ReadAll(input) if err != nil { @@ -996,6 +997,9 @@ func sha1CurrentPatternProcessor(ctx *RenderContext, node *html.Node) { start := 0 next := node.NextSibling + if ctx.ShaExistCache == nil { + ctx.ShaExistCache = make(map[string]bool) + } for node != nil && node != next && start < len(node.Data) { m := sha1CurrentPattern.FindStringSubmatchIndex(node.Data[start:]) if m == nil { @@ -1013,10 +1017,28 @@ func sha1CurrentPatternProcessor(ctx *RenderContext, node *html.Node) { // as used by git and github for linking and thus we have to do similar. // Because of this, we check to make sure that a matched hash is actually // a commit in the repository before making it a link. - if _, err := git.NewCommand("rev-parse", "--verify", hash).RunInDirBytes(ctx.Metas["repoPath"]); err != nil { - if !strings.Contains(err.Error(), "fatal: Needed a single revision") { - log.Debug("sha1CurrentPatternProcessor git rev-parse: %v", err) + + // check cache first + exist, inCache := ctx.ShaExistCache[hash] + if !inCache { + if ctx.GitRepo == nil { + var err error + ctx.GitRepo, err = git.OpenRepository(ctx.Metas["repoPath"]) + if err != nil { + log.Error("unable to open repository: %s Error: %v", ctx.Metas["repoPath"], err) + return + } + ctx.AddCancel(func() { + ctx.GitRepo.Close() + ctx.GitRepo = nil + }) } + + exist = ctx.GitRepo.IsObjectExist(hash) + ctx.ShaExistCache[hash] = exist + } + + if !exist { start = m[3] continue } diff --git a/modules/markup/renderer.go b/modules/markup/renderer.go index 5d35bd5a67..d60c8ad710 100644 --- a/modules/markup/renderer.go +++ b/modules/markup/renderer.go @@ -13,6 +13,7 @@ import ( "strings" "sync" + "code.gitea.io/gitea/modules/git" "code.gitea.io/gitea/modules/setting" ) @@ -35,13 +36,44 @@ func Init() { // RenderContext represents a render context type RenderContext struct { - Ctx context.Context - Filename string - Type string - IsWiki bool - URLPrefix string - Metas map[string]string - DefaultLink string + Ctx context.Context + Filename string + Type string + IsWiki bool + URLPrefix string + Metas map[string]string + DefaultLink string + GitRepo *git.Repository + ShaExistCache map[string]bool + cancelFn func() +} + +// Cancel runs any cleanup functions that have been registered for this Ctx +func (ctx *RenderContext) Cancel() { + if ctx == nil { + return + } + ctx.ShaExistCache = map[string]bool{} + if ctx.cancelFn == nil { + return + } + ctx.cancelFn() +} + +// AddCancel adds the provided fn as a Cleanup for this Ctx +func (ctx *RenderContext) AddCancel(fn func()) { + if ctx == nil { + return + } + oldCancelFn := ctx.cancelFn + if oldCancelFn == nil { + ctx.cancelFn = fn + return + } + ctx.cancelFn = func() { + defer oldCancelFn() + fn() + } } // Renderer defines an interface for rendering markup file to HTML diff --git a/routers/web/org/home.go b/routers/web/org/home.go index d84ae870ab..ad14f18454 100644 --- a/routers/web/org/home.go +++ b/routers/web/org/home.go @@ -41,6 +41,7 @@ func Home(ctx *context.Context) { desc, err := markdown.RenderString(&markup.RenderContext{ URLPrefix: ctx.Repo.RepoLink, Metas: map[string]string{"mode": "document"}, + GitRepo: ctx.Repo.GitRepo, }, org.Description) if err != nil { ctx.ServerError("RenderString", err) diff --git a/routers/web/repo/issue.go b/routers/web/repo/issue.go index fd2877e706..a7951b6bce 100644 --- a/routers/web/repo/issue.go +++ b/routers/web/repo/issue.go @@ -1137,6 +1137,7 @@ func ViewIssue(ctx *context.Context) { issue.RenderedContent, err = markdown.RenderString(&markup.RenderContext{ URLPrefix: ctx.Repo.RepoLink, Metas: ctx.Repo.Repository.ComposeMetas(), + GitRepo: ctx.Repo.GitRepo, }, issue.Content) if err != nil { ctx.ServerError("RenderString", err) @@ -1301,6 +1302,7 @@ func ViewIssue(ctx *context.Context) { comment.RenderedContent, err = markdown.RenderString(&markup.RenderContext{ URLPrefix: ctx.Repo.RepoLink, Metas: ctx.Repo.Repository.ComposeMetas(), + GitRepo: ctx.Repo.GitRepo, }, comment.Content) if err != nil { ctx.ServerError("RenderString", err) @@ -1376,6 +1378,7 @@ func ViewIssue(ctx *context.Context) { comment.RenderedContent, err = markdown.RenderString(&markup.RenderContext{ URLPrefix: ctx.Repo.RepoLink, Metas: ctx.Repo.Repository.ComposeMetas(), + GitRepo: ctx.Repo.GitRepo, }, comment.Content) if err != nil { ctx.ServerError("RenderString", err) @@ -1734,6 +1737,7 @@ func UpdateIssueContent(ctx *context.Context) { content, err := markdown.RenderString(&markup.RenderContext{ URLPrefix: ctx.Query("context"), Metas: ctx.Repo.Repository.ComposeMetas(), + GitRepo: ctx.Repo.GitRepo, }, issue.Content) if err != nil { ctx.ServerError("RenderString", err) @@ -2161,6 +2165,7 @@ func UpdateCommentContent(ctx *context.Context) { content, err := markdown.RenderString(&markup.RenderContext{ URLPrefix: ctx.Query("context"), Metas: ctx.Repo.Repository.ComposeMetas(), + GitRepo: ctx.Repo.GitRepo, }, comment.Content) if err != nil { ctx.ServerError("RenderString", err) diff --git a/routers/web/repo/milestone.go b/routers/web/repo/milestone.go index bb6b310cbe..4cdca38dd0 100644 --- a/routers/web/repo/milestone.go +++ b/routers/web/repo/milestone.go @@ -88,6 +88,7 @@ func Milestones(ctx *context.Context) { m.RenderedContent, err = markdown.RenderString(&markup.RenderContext{ URLPrefix: ctx.Repo.RepoLink, Metas: ctx.Repo.Repository.ComposeMetas(), + GitRepo: ctx.Repo.GitRepo, }, m.Content) if err != nil { ctx.ServerError("RenderString", err) @@ -280,6 +281,7 @@ func MilestoneIssuesAndPulls(ctx *context.Context) { milestone.RenderedContent, err = markdown.RenderString(&markup.RenderContext{ URLPrefix: ctx.Repo.RepoLink, Metas: ctx.Repo.Repository.ComposeMetas(), + GitRepo: ctx.Repo.GitRepo, }, milestone.Content) if err != nil { ctx.ServerError("RenderString", err) diff --git a/routers/web/repo/projects.go b/routers/web/repo/projects.go index eb0719995c..c7490893d5 100644 --- a/routers/web/repo/projects.go +++ b/routers/web/repo/projects.go @@ -81,6 +81,7 @@ func Projects(ctx *context.Context) { projects[i].RenderedContent, err = markdown.RenderString(&markup.RenderContext{ URLPrefix: ctx.Repo.RepoLink, Metas: ctx.Repo.Repository.ComposeMetas(), + GitRepo: ctx.Repo.GitRepo, }, projects[i].Description) if err != nil { ctx.ServerError("RenderString", err) @@ -322,6 +323,7 @@ func ViewProject(ctx *context.Context) { project.RenderedContent, err = markdown.RenderString(&markup.RenderContext{ URLPrefix: ctx.Repo.RepoLink, Metas: ctx.Repo.Repository.ComposeMetas(), + GitRepo: ctx.Repo.GitRepo, }, project.Description) if err != nil { ctx.ServerError("RenderString", err) diff --git a/routers/web/repo/release.go b/routers/web/repo/release.go index b7730e4ee2..3b700e8016 100644 --- a/routers/web/repo/release.go +++ b/routers/web/repo/release.go @@ -145,6 +145,7 @@ func releasesOrTags(ctx *context.Context, isTagList bool) { r.Note, err = markdown.RenderString(&markup.RenderContext{ URLPrefix: ctx.Repo.RepoLink, Metas: ctx.Repo.Repository.ComposeMetas(), + GitRepo: ctx.Repo.GitRepo, }, r.Note) if err != nil { ctx.ServerError("RenderString", err) @@ -213,6 +214,7 @@ func SingleRelease(ctx *context.Context) { release.Note, err = markdown.RenderString(&markup.RenderContext{ URLPrefix: ctx.Repo.RepoLink, Metas: ctx.Repo.Repository.ComposeMetas(), + GitRepo: ctx.Repo.GitRepo, }, release.Note) if err != nil { ctx.ServerError("RenderString", err) diff --git a/routers/web/repo/view.go b/routers/web/repo/view.go index cd5b0f43ed..74e2a29597 100644 --- a/routers/web/repo/view.go +++ b/routers/web/repo/view.go @@ -338,6 +338,7 @@ func renderDirectory(ctx *context.Context, treeLink string) { Filename: readmeFile.name, URLPrefix: readmeTreelink, Metas: ctx.Repo.Repository.ComposeDocumentMetas(), + GitRepo: ctx.Repo.GitRepo, }, rd, &result) if err != nil { log.Error("Render failed: %v then fallback", err) @@ -512,6 +513,7 @@ func renderFile(ctx *context.Context, entry *git.TreeEntry, treeLink, rawLink st Filename: blob.Name(), URLPrefix: path.Dir(treeLink), Metas: ctx.Repo.Repository.ComposeDocumentMetas(), + GitRepo: ctx.Repo.GitRepo, }, rd, &result) if err != nil { ctx.ServerError("Render", err) @@ -570,6 +572,7 @@ func renderFile(ctx *context.Context, entry *git.TreeEntry, treeLink, rawLink st Filename: blob.Name(), URLPrefix: path.Dir(treeLink), Metas: ctx.Repo.Repository.ComposeDocumentMetas(), + GitRepo: ctx.Repo.GitRepo, }, rd, &result) if err != nil { ctx.ServerError("Render", err) diff --git a/routers/web/user/profile.go b/routers/web/user/profile.go index e66820e131..72d0066645 100644 --- a/routers/web/user/profile.go +++ b/routers/web/user/profile.go @@ -117,6 +117,7 @@ func Profile(ctx *context.Context) { content, err := markdown.RenderString(&markup.RenderContext{ URLPrefix: ctx.Repo.RepoLink, Metas: map[string]string{"mode": "document"}, + GitRepo: ctx.Repo.GitRepo, }, ctxUser.Description) if err != nil { ctx.ServerError("RenderString", err)