sourcegraph / sourcegraph-public-snapshot

Code AI platform with Code Search & Cody
https://sourcegraph.com
Other
10.11k stars 1.29k forks source link

Chrome extension brings page load to a halt on 400-file PR #4994

Closed byrongrogan closed 5 years ago

byrongrogan commented 5 years ago

Steps to reproduce:

  1. Load a very large PR (issue observed on 400-file PR)
  2. Scroll down to check out the files

Expected behavior:

Files load normally, code intelligence works, etc.

Actual behavior:

Tab slows to a halt after ~60 code views have been loaded. Full page load takes >5 minutes and sometimes crashes.

I have a profile from one case that I'll send along via e-mail (since it contains some PII).

Note: If there are hard limits that make it impossible for this to be a good user experience, it would be better to simply make the extension a no-op on PRs this large. Currently people have to uninstall the chrome extension entirely to be able to work with certain very large PRs.

nicksnyder commented 5 years ago

More context is available in this conversation: https://sourcegraph.slack.com/archives/GDS3PJ8GK/p1564437051003400

@felixfbecker can you triage this and determine if there is an obvious performance issue we need to correct or if we need to limit the size of PRs that we provide code intel for?

christinaforney commented 5 years ago

Chrome uninstall feedback from user on 7/29: https://app.hubspot.com/forms/2762526/a1bc77c9-019c-4f83-b5ba-327949f8e587/submissions/007dff41-6d26-4ec7-9792-ebda4fc74656

christinaforney commented 5 years ago

Chrome uninstall feedback from user at different company on 7/26: https://app.hubspot.com/forms/2762526/a1bc77c9-019c-4f83-b5ba-327949f8e587/submissions/e56ea2eb-060b-4797-8bde-974b17d36579

unknwon commented 5 years ago

Observation

Test PR: https://github.com/sourcegraph/sourcegraph/pull/5157 (be caution with "Files" tab, 999 files)

It seems that the problem is mainly caused by loading too many files into memory with at least two (head and base) requests per file to Sourcegraph GraphQL API.

Code path that potentially causes the problem:

  1. resolveFileInfo -> fetchFileContents: https://github.com/sourcegraph/sourcegraph/blob/436a39cd4fc177cd389c50987fd940e377ba5829/browser/src/libs/code_intelligence/code_intelligence.tsx#L549-L552

  2. fetchFileContents -> fetchBlobContentLines: https://github.com/sourcegraph/sourcegraph/blob/436a39cd4fc177cd389c50987fd940e377ba5829/browser/src/libs/code_intelligence/code_views.ts#L95-L110

  3. fetchBlobContentLines: https://github.com/sourcegraph/sourcegraph/blob/436a39cd4fc177cd389c50987fd940e377ba5829/browser/src/shared/repo/backend.tsx#L116-L133

The loading process significantly slows down when the number before "Code view added" gets large enough (depends on the CPU and RAM?), then page freezes.

Just keep in mind, there is a big overhead based on number of files that for Chrome itself to finish rendering the page.

Experiments

I did a syntactically invalid hack by replacing the request value to empty object in the function fetchBlobContentLines as follows:

            requestGraphQL<GQL.IQuery>({
+               request: {},
-               request: gql`
-                   query BlobContent($repoName: String!, $commitID: String!, $filePath: String!) {
-                       repository(name: $repoName) {
-                           commit(rev: $commitID) {
-                               file(path: $filePath) {
-                                   content
-                               }
-                           }
-                       }
-                   }
-               `,
                variables: ctx,
                mightContainPrivateInfo: true,

And try again, it still slow (I'm not sure if the request still been sent with my changes), but at least slowly towards number of changed files:

image

(this is last screenshot I could get before my Chrome dies)

It will freeze at one point eventually (I can't be sure but my guess is there are just too many memory objects, which makes some function calls way more expansive than we only have to deal with dozens of files)

If I only make 110 changed files with empty request, it can load very quickly: image

Thoughts

What I experimented is definitely not a solution to the problem, but may indicate how we could tackle this issue with this root issue discovery.

unknwon commented 5 years ago

Did a performance profiling with @nicksnyder and we found two suspicious calls very frequently, namely addEditor and addModel.

image

We're not sure if we interpret the profiling graph correctly, it seems these two calls are slowing down the entire process:

image (each takes about 1200ms and adds up to ~2400ms)

We also notice the JS heap goes up fairly quick within 22s: image

In conclusion, the questions are:

  1. Do calls of addEditor and addModel functions are intentional? Or just some side effects we didn't notice.
  2. Is the memory usage alone causing the freeze or is it combined with overheads of addEditor and addModel function calls?

@felixfbecker would you mind give some feedbacks and insights about this? What do you think our best next move is?

nicksnyder commented 5 years ago

Thanks for investigating this @unknwon !

Random thoughts that I had:

nicksnyder commented 5 years ago

@felixfbecker deployed a change to limit the number of files that our browser extension provides code intel for.

You can update your Firefox or Chrome extension (latest version is 19.8.12.2135) and it should no longer freeze on large PRs.

I created a separate issue to track the progress for fixing the root cause of the performance issue so we can remove the 50 file limit: https://github.com/sourcegraph/sourcegraph/issues/5172