Skip to content
This repository was archived by the owner on Sep 30, 2024. It is now read-only.

Commit c595d4d

Browse files
camdencheekvovakulikov
authored andcommitted
Perf: optimize fetching last file commit (#58055)
When fetching the information about the commit that modified a file, we use an ancestors query that is filtered to the file name. This is a paginated interface, so we always request n+1 items so we know whether there is a next page. However, the extra requested commit can actually be quite expensive to fetch, particularly in the relatively common case that a file has only been modified in one commit: the one that created it. In that case, we have to iterate over the entire history of the repository to look for a commit that doesn't exist to produce information about a next page that will never be fetched. The ideal solution here would be to only fetch n+1 if the hasNextPage field of the resolver is requested. Unfortunately, our GraphQL package still doesn't support querying for which fields are requested, which is a huge bummer. Instead, this takes a slightly hackier approach and assumes that if the number requested is 1, it's better to serve that quickly and claim we have a next page without knowing whether that page will contain anything.
1 parent bbbb431 commit c595d4d

File tree

1 file changed

+25
-8
lines changed

1 file changed

+25
-8
lines changed

cmd/frontend/graphqlbackend/git_commits.go

Lines changed: 25 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ import (
1010
"github.com/sourcegraph/sourcegraph/internal/gitserver"
1111
"github.com/sourcegraph/sourcegraph/internal/gitserver/gitdomain"
1212
"github.com/sourcegraph/sourcegraph/lib/errors"
13+
"github.com/sourcegraph/sourcegraph/lib/pointers"
1314
)
1415

1516
type gitCommitConnectionResolver struct {
@@ -60,13 +61,21 @@ func (r *gitCommitConnectionResolver) afterCursorAsInt() (int, error) {
6061

6162
func (r *gitCommitConnectionResolver) compute(ctx context.Context) ([]*gitdomain.Commit, error) {
6263
do := func() ([]*gitdomain.Commit, error) {
63-
var n int32
64-
// IMPORTANT: We cannot use toValue here because we toValue will return 0 if r.first is nil.
65-
// And n will be incorrectly set to 1. A nil value for r.first implies no limits, so skip
66-
// setting a value for n completely.
67-
if r.first != nil {
68-
n = *r.first
69-
n++ // fetch +1 additional result so we can determine if a next page exists
64+
n := pointers.Deref(r.first, 0)
65+
66+
// PERF: only request extra if we request more than one result. A
67+
// common scenario is requesting the latest commit that modified a
68+
// file, but many files have only been modified by the commit that
69+
// created them. If we request more than one commit, we have to
70+
// traverse the entire git history to find a second commit that doesn't
71+
// exist, which is useless information in the case that we only want
72+
// the latest commit anyways.
73+
//
74+
// NOTE: in a world where we can view which fields were requested (our
75+
// GraphQL library doesn't currently support this), we could make this
76+
// conditional on whether `hasNextPage` was part of the request.
77+
if n > 1 {
78+
n += 1
7079
}
7180

7281
// If no value for afterCursor is set, then skip is 0. And this is fine as --skip=0 is the
@@ -142,9 +151,17 @@ func (r *gitCommitConnectionResolver) PageInfo(ctx context.Context) (*graphqluti
142151

143152
limit := int(*r.first)
144153

154+
// In the special case that only one commit was requested, we want
155+
// to always say there is a next page because we didn't request an
156+
// extra to know whether there were more.
157+
//
158+
// NOTE: this means that `hasNextPage` can possibly incorrectly
159+
// return true when only one result is requested.
160+
gotSingleRequestedCommit := limit == 1 && totalCommits == limit
161+
145162
// If a limit is set, we attempt to fetch N+1 commits to know if there is a next page or not. If
146163
// we have more than N commits then we have a next page.
147-
if totalCommits > limit {
164+
if totalCommits > limit || gotSingleRequestedCommit {
148165
// Pagination logic below.
149166
//
150167
// Example:

0 commit comments

Comments
 (0)