From 8bf979099bba39080ae2e75cb4e68a163023edf7 Mon Sep 17 00:00:00 2001 From: bircni Date: Mon, 12 Oct 2026 18:44:29 +0200 Subject: [PATCH] perf: drop wasted requests and reuse markdown renderers pr edit with reviewers and other fields reads the pull request once, org delete counts only repositories, org view counts concurrently, team list skips the repository count of all-repository teams, pr diff --web builds the URL locally, pr list re-reads pull requests concurrently for review fields, and glamour renderers are built once per width and color. Signed-off-by: bircni --- internal/app/organizations.go | 4 +-- internal/app/pull_mutations.go | 6 +--- internal/app/pulls.go | 6 ++-- internal/services/organizations.go | 43 +++++++++++++++++++--------- internal/services/pulls.go | 23 +++++++++++---- internal/services/review_requests.go | 26 +++++++++++------ internal/services/teams.go | 3 +- internal/terminal/markdown.go | 40 +++++++++++++++++++++----- 8 files changed, 104 insertions(+), 47 deletions(-) diff --git a/internal/app/organizations.go b/internal/app/organizations.go index 397b5713..d55e0175 100644 --- a/internal/app/organizations.go +++ b/internal/app/organizations.go @@ -292,11 +292,11 @@ $ tea org delete acme-old --yes`, if err != nil { return err } - size, err := service.Size(cmd.Context(), org.Name) + repos, err := service.RepositoryCount(cmd.Context(), org.Name) if err != nil { return err } - if size.Repositories != nil && *size.Repositories > 0 { + if repos != nil && *repos > 0 { return &services.ConflictError{Reason: services.ReasonOwnsRepos, Kind: "organization", Key: org.Name, Hint: "transfer or delete them first (see tea repo list " + org.Name + ")"} } if err := a.confirmAction(cmd, terminal.Confirmation{ diff --git a/internal/app/pull_mutations.go b/internal/app/pull_mutations.go index 9a4b8312..2bd33547 100644 --- a/internal/app/pull_mutations.go +++ b/internal/app/pull_mutations.go @@ -176,11 +176,7 @@ tea pr diff 13 | git apply works.`, Annotations: pullArgumentsAnnotation(textOut } service := services.Pulls{Client: t.client, Repository: t.Repo} if web { - item, err := service.View(cmd.Context(), n) - if err != nil { - return err - } - return a.openBrowser(cmd, item.URL+"/files") + return a.openBrowser(cmd, fmt.Sprintf("%s/pulls/%d/files", repositoryURL(t), n)) } if nameOnly { files, err := service.Files(cmd.Context(), n) diff --git a/internal/app/pulls.go b/internal/app/pulls.go index 115c047e..0a0990c3 100644 --- a/internal/app/pulls.go +++ b/internal/app/pulls.go @@ -78,10 +78,8 @@ each pull request, which tea only makes when --json names them.`, Args: argsExac } items := window.Items if a.jsonSelects("reviewers", "reviewer_teams", "merged_by") { - for i, item := range items { - if items[i], err = service.View(cmd.Context(), item.Number); err != nil { - return err - } + if items, err = service.ViewAll(cmd.Context(), items); err != nil { + return err } } noun := map[string]string{"open": "open pull requests", "closed": "closed pull requests", "merged": "merged pull requests", "all": "pull requests"}[state] diff --git a/internal/services/organizations.go b/internal/services/organizations.go index 36caecde..794e498b 100644 --- a/internal/services/organizations.go +++ b/internal/services/organizations.go @@ -94,7 +94,8 @@ type OrganizationSize struct { Members, Teams, Repositories *int } -// Size counts the members, teams and repositories of an organization. +// Size counts the members, teams and repositories of an organization; a count the account cannot +// read is nil. func (s Organizations) Size(ctx context.Context, name string) (OrganizationSize, error) { first := listOptions(1, 1) counts := []func(context.Context, *gitea.Client) (*gitea.Response, error){ @@ -106,24 +107,40 @@ func (s Organizations) Size(ctx context.Context, name string) (OrganizationSize, _, resp, err := c.Organizations.ListOrgTeams(ctx, name, gitea.ListTeamsOptions{ListOptions: first}) return resp, err }, - func(ctx context.Context, c *gitea.Client) (*gitea.Response, error) { - _, resp, err := c.Repositories.ListOrgRepos(ctx, name, gitea.ListOrgReposOptions{ListOptions: first}) - return resp, err - }, + orgRepos(name), } totals := make([]*int, len(counts)) - for i, count := range counts { - total, err := listTotal(ctx, s.Client, count) - if ctx.Err() != nil { - return OrganizationSize{}, ctx.Err() - } - if err == nil { - totals[i] = total - } + _ = concurrently(len(counts), func(i int) error { + totals[i], _ = listTotal(ctx, s.Client, counts[i]) + return nil + }) + if ctx.Err() != nil { + return OrganizationSize{}, ctx.Err() } return OrganizationSize{Members: totals[0], Teams: totals[1], Repositories: totals[2]}, nil } +// RepositoryCount counts the repositories of an organization, or returns nil when the account +// cannot read the count. +func (s Organizations) RepositoryCount(ctx context.Context, name string) (*int, error) { + total, err := listTotal(ctx, s.Client, orgRepos(name)) + if ctx.Err() != nil { + return nil, ctx.Err() + } + if err != nil { + return nil, nil + } + return total, nil +} + +// orgRepos requests the first repository of an organization, to count them. +func orgRepos(name string) func(context.Context, *gitea.Client) (*gitea.Response, error) { + return func(ctx context.Context, c *gitea.Client) (*gitea.Response, error) { + _, resp, err := c.Repositories.ListOrgRepos(ctx, name, gitea.ListOrgReposOptions{ListOptions: listOptions(1, 1)}) + return resp, err + } +} + // listTotal returns the collection size a list request reports, or nil when the server does not count it. func listTotal(ctx context.Context, client *transport.Client, list func(context.Context, *gitea.Client) (*gitea.Response, error)) (*int, error) { _, resp, err := call(ctx, client, func(ctx context.Context, c *gitea.Client) (struct{}, *gitea.Response, error) { diff --git a/internal/services/pulls.go b/internal/services/pulls.go index 00a7cfe1..3469caab 100644 --- a/internal/services/pulls.go +++ b/internal/services/pulls.go @@ -236,6 +236,17 @@ func (s Pulls) View(ctx context.Context, n int64) (Pull, error) { return pullFrom(pull), nil } +// ViewAll retrieves each of items again, a few at a time, for the fields only a single pull +// request carries, such as its reviewers. +func (s Pulls) ViewAll(ctx context.Context, items []Pull) ([]Pull, error) { + viewed := make([]Pull, len(items)) + err := concurrently(len(items), func(i int) (err error) { + viewed[i], err = s.View(ctx, items[i].Number) + return err + }) + return viewed, err +} + func (s Pulls) get(ctx context.Context, n int64) (*gitea.PullRequest, error) { item, err := repoCall(ctx, s.Client, s.Repository, func(ctx context.Context, c *gitea.Client, owner, name string) (*gitea.PullRequest, *gitea.Response, error) { return c.PullRequests.GetPullRequest(ctx, owner, name, n) @@ -264,15 +275,15 @@ func (s Pulls) Create(ctx context.Context, opts PullOptions) (Pull, error) { // Edit updates only explicitly supplied resource fields. func (s Pulls) Edit(ctx context.Context, n int64, opts PullEdit) (Pull, error) { if opts.ReviewerChanges != nil { - item, err := (Reviews{Client: s.Client, Repository: s.Repository}).ChangeRequests(ctx, n, *opts.ReviewerChanges) - if err != nil { - return Pull{}, err - } + reviews, changes := Reviews{Client: s.Client, Repository: s.Repository}, opts.ReviewerChanges opts.ReviewerChanges = nil if opts == (PullEdit{}) { - return item, nil + return reviews.ChangeRequests(ctx, n, *changes) } - item, err = s.Edit(ctx, n, opts) + if err := reviews.changeRequests(ctx, n, *changes); err != nil { + return Pull{}, err + } + item, err := s.Edit(ctx, n, opts) if err != nil { return Pull{}, fmt.Errorf("review requests were updated, but editing other pull request fields failed: %w", err) } diff --git a/internal/services/review_requests.go b/internal/services/review_requests.go index b530549c..dd654bcb 100644 --- a/internal/services/review_requests.go +++ b/internal/services/review_requests.go @@ -51,12 +51,24 @@ func (c ReviewRequestChanges) Validate() error { // ChangeRequests returns the PR after changes and reports partial mutation failures. func (s Reviews) ChangeRequests(ctx context.Context, n int64, changes ReviewRequestChanges) (Pull, error) { - if err := changes.Validate(); err != nil { + if err := s.changeRequests(ctx, n, changes); err != nil { return Pull{}, err } + item, err := (Pulls{Client: s.Client, Repository: s.Repository}).View(ctx, n) + if err != nil { + return Pull{}, fmt.Errorf("review requests were updated, but reading the pull request failed: %w", err) + } + return item, nil +} + +// changeRequests applies the review request changes to pull request n. +func (s Reviews) changeRequests(ctx context.Context, n int64, changes ReviewRequestChanges) error { + if err := changes.Validate(); err != nil { + return err + } if len(changes.Add.Teams)+len(changes.Remove.Teams) > 0 { if err := s.CheckRequestTeams(ctx, append(append([]string{}, changes.Add.Teams...), changes.Remove.Teams...)); err != nil { - return Pull{}, err + return err } } mutated := false @@ -76,17 +88,13 @@ func (s Reviews) ChangeRequests(ctx context.Context, n int64, changes ReviewRequ }) if err != nil { if mutated { - return Pull{}, fmt.Errorf("reviewer removals succeeded, but adding reviewers failed: %w", err) + return fmt.Errorf("reviewer removals succeeded, but adding reviewers failed: %w", err) } - return Pull{}, fmt.Errorf("review request update failed; Gitea may have applied some requests: %w", err) + return fmt.Errorf("review request update failed; Gitea may have applied some requests: %w", err) } mutated = true } - item, err := (Pulls{Client: s.Client, Repository: s.Repository}).View(ctx, n) - if err != nil { - return Pull{}, fmt.Errorf("review requests were updated, but reading the pull request failed: %w", err) - } - return item, nil + return nil } // CheckRequestTeams verifies that every named team exists in the repository's organization. diff --git a/internal/services/teams.go b/internal/services/teams.go index a4ab9656..63500be6 100644 --- a/internal/services/teams.go +++ b/internal/services/teams.go @@ -203,7 +203,8 @@ func (s Teams) Sizes(ctx context.Context, teams []Team) ([]TeamSize, error) { _, resp, err := c.Organizations.ListTeamMembers(ctx, id, gitea.ListTeamMembersOptions{ListOptions: first}) return resp, err }) - if err != nil { + if err != nil || teams[i].AllRepositories { + sizes[i] = TeamSize{Members: members} return err } repositories, err := listTotal(ctx, s.Client, func(ctx context.Context, c *gitea.Client) (*gitea.Response, error) { diff --git a/internal/terminal/markdown.go b/internal/terminal/markdown.go index 0ed8024f..a062a87f 100644 --- a/internal/terminal/markdown.go +++ b/internal/terminal/markdown.go @@ -6,6 +6,7 @@ package terminal import ( "regexp" "strings" + "sync" "charm.land/glamour/v2" "charm.land/glamour/v2/styles" @@ -19,6 +20,34 @@ func MarkdownEnabled(preference string, outTTY bool) bool { // glamourLinks matches the OSC 8 hyperlinks that glamour writes for markdown links. var glamourLinks = regexp.MustCompile("\x1b\\]8;[^\x07\x1b]*(?:\x07|\x1b\\\\)") +// renderers caches a glamour renderer per width and color, as building one parses its style each +// time; the lock also serializes rendering. +var renderers = struct { + sync.Mutex + byKey map[[2]int]*glamour.TermRenderer +}{byKey: map[[2]int]*glamour.TermRenderer{}} + +func markdownRenderer(width int, color bool) (*glamour.TermRenderer, error) { + key := [2]int{width, 0} + if color { + key[1] = 1 + } + if renderer := renderers.byKey[key]; renderer != nil { + return renderer, nil + } + style := styles.NoTTYStyleConfig + if color { + style = styles.DarkStyleConfig + // The terminal's own text color keeps body text readable on light backgrounds too. + style.Document.Color = nil + } + renderer, err := glamour.NewTermRenderer(glamour.WithStyles(style), glamour.WithWordWrap(width), glamour.WithPreservedNewLines()) + if err == nil { + renderers.byKey[key] = renderer + } + return renderer, err +} + // Markdown renders sanitized markdown for d: wrapped to its width (80 when unknown), colored or plain, // with links only when d allows them. It keeps line breaks like Gitea does, and returns the sanitized // text unrendered if rendering fails. @@ -28,17 +57,14 @@ func Markdown(text string, d Display) string { if width <= 0 { width = 80 } - style := styles.NoTTYStyleConfig - if d.Color { - style = styles.DarkStyleConfig - // The terminal's own text color keeps body text readable on light backgrounds too. - style.Document.Color = nil - } - renderer, err := glamour.NewTermRenderer(glamour.WithStyles(style), glamour.WithWordWrap(width), glamour.WithPreservedNewLines()) + renderers.Lock() + renderer, err := markdownRenderer(width, d.Color) if err != nil { + renderers.Unlock() return text } rendered, err := renderer.Render(text) + renderers.Unlock() if err != nil { return text }