Skip to content
Snippets Groups Projects

graphqlbackend: More lazy loading of git commits

Merged Administrator requested to merge cloud/more-lazy-commit-loading into main

Created by: tsenart

This changeset builds on top of #15752 to extend lazy loading of git commits to these code paths:

  • graphqlbackend.RepositoryResolver.{Commit, CommitFromID}
  • graphqlbackend.NewRepositoryComparison

As per what we observed in getCommits data set in Honeycomb, most of the GraphQL queries using the gitCommitResolver, either directly or indirectly, don't ask for information beyond the commit ID or URL, which can be constructed statically without calling out to git-server to run an expensive git log operation.

This change also makes it so the above code paths don't trigger any lazy adding of repos that are being seen for the first time, which on sourcegraph.com wasn't happening anyways, and on customer instances isn't needed, since we know the repos we have at hand already do exist in the repo table.

This doesn't change the behaviour of ensuring a revision exists when running a git command (like git log).

These are client calls that should see speed improvements from having at least -1 RPCs to gitserver:

  1. https://github.com/sourcegraph/code-intel-extensions/blob/6ac2e6dca697f6077b8021c81197427d2a9d93dc/shared/lsif/definition-hover.ts#L26
  2. https://github.com/sourcegraph/code-intel-extensions/blob/6ac2e6dca697f6077b8021c81197427d2a9d93dc/shared/lsif/ranges.ts#L246
  3. https://github.com/sourcegraph/code-intel-extensions/blob/6ac2e6dca697f6077b8021c81197427d2a9d93dc/shared/lsif/references.ts#L24
  4. https://github.com/sourcegraph/code-intel-extensions/blob/6ac2e6dca697f6077b8021c81197427d2a9d93dc/shared/util/api.ts#L129
  5. https://github.com/sourcegraph/code-intel-extensions/blob/6ac2e6dca697f6077b8021c81197427d2a9d93dc/shared/util/api.ts#L307
  6. https://github.com/sourcegraph/codenav-bash/blob/2ab71ff256c7c1d62a98befb88ca74fb8965c936/src/api.ts#L122
  7. https://github.com/sourcegraph/codenav-bash/blob/2ab71ff256c7c1d62a98befb88ca74fb8965c936/src/api.ts#L319
  8. https://github.com/sourcegraph/sourcegraph-eslint/blob/9e474333c7db0cdc60e94407ed75b3ad6a9c1b61/src/eslint.ts#L22
  9. https://github.com/sourcegraph/sourcegraph-test-leaderboard/blob/b9ccbc12052220c38fec14efea4677d3b2a4127e/src/sourcegraph-test-leaderboard.ts#L102
  10. https://github.com/sourcegraph/sourcegraph-test-leaderboard/blob/b9ccbc12052220c38fec14efea4677d3b2a4127e/src/sourcegraph-test-leaderboard.ts#L143
  11. https://github.com/sourcegraph/sourcegraph/blob/2d837e4fff632b95dabb58014af3e1b8844b28b2/client/browser/src/shared/repo/backend.tsx#L64
  12. https://github.com/sourcegraph/sourcegraph/blob/421ebb264118cc0b64206e8f769d1b2d7db2056f/client/browser/src/shared/repo/backend.tsx#L129
  13. https://github.com/sourcegraph/sourcegraph/blob/7d11c94f4fb982daf625dad34a713743440ce5a9/client/browser/src/shared/backend/diffs.tsx#L21
  14. https://github.com/sourcegraph/sourcegraph/blob/7d11c94f4fb982daf625dad34a713743440ce5a9/client/web/src/repo/backend.ts#L168
  15. https://github.com/sourcegraph/sourcegraph/blob/7d11c94f4fb982daf625dad34a713743440ce5a9/client/web/src/repo/backend.ts#L229
  16. https://github.com/sourcegraph/sourcegraph/blob/7d11c94f4fb982daf625dad34a713743440ce5a9/client/web/src/repo/backend.ts#L259
  17. https://github.com/sourcegraph/sourcegraph/blob/7d11c94f4fb982daf625dad34a713743440ce5a9/client/web/src/repo/compare/RepositoryCompareDiffPage.tsx#L31
  18. https://github.com/sourcegraph/sourcegraph/blob/7d11c94f4fb982daf625dad34a713743440ce5a9/client/web/src/repo/tree/TreePage.tsx#L60

These code paths are so pervasive, that this should result in a slight to significant (depending on the specific client call) performance improvement across the whole product. We'll be able to monitor the performance of these GraphQL queries with the internal dashboard checked-in with this PR (cc @bobheadxi @pecigonzalo).

Screenshot 2020-11-19 at 14 21 23

This also fixes #15392 for good (for now).

Merge request reports

Merged by avatar (Jul 5, 2025 9:06am UTC)

Loading

Activity

Filter activity
  • Approvals
  • Assignees & reviewers
  • Comments (from bots)
  • Comments (from users)
  • Commits & branches
  • Edits
  • Labels
  • Lock status
  • Mentions
  • Merge request status
  • Tracking
  • Created by: codecov[bot]

    Codecov Report

    Merging #15971 (833c4df) into main (27704f2) will increase coverage by 0.00%. The diff coverage is 71.42%.

    @@           Coverage Diff           @@
    ##             main   #15971   +/-   ##
    =======================================
      Coverage   52.89%   52.90%           
    =======================================
      Files        1638     1638           
      Lines       82459    82434   -25     
      Branches     7351     7351           
    =======================================
    - Hits        43620    43611    -9     
    + Misses      34955    34940   -15     
    + Partials     3884     3883    -1     
    Flag Coverage Δ *Carryforward flag
    go 52.77% <71.42%> (+<0.01%) :arrow_up:
    integration 28.83% <ø> (ø) Carriedforward from 27704f2
    storybook 27.78% <ø> (ø) Carriedforward from 27704f2
    typescript 53.22% <ø> (ø) Carriedforward from 27704f2
    unit 35.44% <ø> (ø) Carriedforward from 27704f2

    *This pull request uses carry forward flags. Click here to find out more.

    Impacted Files Coverage Δ
    cmd/frontend/graphqlbackend/git_commit.go 33.09% <60.00%> (+0.19%) :arrow_up:
    cmd/frontend/graphqlbackend/repository.go 18.72% <100.00%> (-0.19%) :arrow_down:
    ...d/frontend/graphqlbackend/repository_comparison.go 73.45% <100.00%> (+1.21%) :arrow_up:
    .../internal/codeintel/resolvers/graphql/locations.go 83.50% <0.00%> (-2.07%) :arrow_down:
  • Created by: sourcegraph-bot

    Notifying subscribers in CODENOTIFY files for diff 82546c6c6cce1a43dc1b420f2cceff28b3f0dfaf...d49e00ddb0e62d158ea2c9e9063b8df436bba6dd.

    Notify File(s)
    @LawnGnome enterprise/internal/campaigns/resolvers/main_test.go
    @bobheadxi docker-images/grafana/config/provisioning/dashboards/sourcegraph_internal/graphql-latencies.json
    @efritz enterprise/cmd/frontend/internal/codeintel/resolvers/graphql/locations_test.go
    @eseliger enterprise/internal/campaigns/resolvers/main_test.go
Please register or sign in to reply
Loading