Performance regression when creating pull requests on a repo with lots of branches

[janakjjp]

Repo:

• In VSCode, execute a git push to a GitHub enterprise repo with thousands of branches (we have >6,000, someone should clean this up, but that's orthogonal).
• Press the button to create a Pull Request.
• VSCode saturates CPU and takes ~60-90 seconds for the Create Pull Request dialog to appear. At that point, it still takes another 30-60 seconds for the "merge into" branch to update to master.

Expected behavior:

• This should only take a few seconds (which was the case until a few weeks ago, as far as I can tell).
• Executing this on a repo with fewer branches (e.g., with 300 branches) is nearly instantaneous.

Other notes:

• Running latest VSCode and extension (1.78.1, plugin version v0.64.0) on an Intel Mac with 32GB RAM.
• The merge into doesn't show all of these 6,000 branches, just a subset.
• I had previously filed a bug a year ago that master would not appear on a repo with lots of branches; some special logic was added so that it does appear (New pull requests on branches off master try to merge into a wrong branch #3303).
• While master does eventually appear, it feels like it's actually fetching the entire list before dropping most of them.
• The plugin otherwise performs fine.

Let me know if I can collect any more data — this is driving me nuts, so help is appreciated 😄

[alexr00]

• VSCode saturates CPU and takes ~60-90 seconds for the Create Pull Request dialog to appear. At that point, it still takes another 30-60 seconds for the "merge into" branch to update to master.

We should be immediately populating the "merge into" with master and then only adding the other branches after we've finished fetching them. What branch is selected in "merge into" before master?

[janakjjp]

Good question, I'll let you know the next time I create a PR.

[janakjjp]

OK, I ran into this again; the answer is "alphanumerically sorted, the first branch in the repo."

In other words, I see, in order, the following:

• If the PR dialog was up before in this VSCode session, it's made visible, with a previous PR in question (could be in a different repo)
  • If the PR dialog was not up before in this VSCode session, nothing is visible
• A huge pause while CPU spikes (~1 minute)
• The PR dialog loads and shows the repo I'm merging into, with the branch field incorrectly populated as the first alphanumeric branch
  • Interestingly this is not the first branch returned by the GH API; so, for example, if I list all branches in the Source Control view, it shows a seemingly-random ordering (I assume most recent is first)
  • This seems to imply that the PR extension is in fact fetching and sorting before it's supposed to?
• Another huge pause while CPU spikes (1-2 minutes)
• Merge into branch finally updates to master, CPU recovers, and the tool is useable

[alexr00]

Thanks @janakjjp for the additional details. I found a case where we're listing branches way too many times. I'm making a fix for it and I'll let you know when it's available in the pre-release build of the extension.

[alexr00]

@janakjjp, the new pre-release build is available and can be used with VS Code insiders. I fixed an issue with listing branches in the create pr view, but since you're the only one who reported this particular issue it would be very helpful if you could try out the new pre-release build and comment about whether this issue is fixed!

[janakjjp]

I tested this, and unfortunately I am still seeing a CPU spike in loading the PR create dialog. Looks like there's probably another callsite?

Now that I have the insiders build, let me know if I can do any other debugging for you to help with this.

[janakjjp]

A little more debugging detail:

• Loading the dialog took ~45 seconds.
• Once it loaded, the "INTO branch" immediately populated with master.
• Then the system froze up again while the CPU spiked for another ~45-60 seconds.

So, if I had to guess, it seems there are three expensive calls, and one of them was fixed!

[alexr00]

@janakjjp thanks for the details! I'll have another look since that first 45 seconds shouldn't be happening.

For the last expensive call: We're going to have to fetch branches at some point to fully populate the dropdown. I'll make sure we don't have extra calls in there, but at best I can do the following:

• We have to fetch the from branches every time.
• For the into branches, if we notice there are a lot we could cache them and then offer a way to refresh on demand.

[janakjjp]

Hmmm, I'm torn about having to fully populate the dropdown, as opposed to trimming it after X branches. Part of the challenge is that GH (I think) uses a type-to-complete mechanism and so can lazily fetch.

I don't want to make your life too difficult here, I know we're an edge case, so any optimizations are welcome but I understand you can't always solve for the extreme case.

[alexr00]

I found some more places where we're making extra GitHub API calls. The change will be in the pre-release build on Monday. @janakjjp I'd be very interested to hear if you find that the build on Monday has better performance! I have not yet added any caching, but I can consider it if you're still seeing serious slowness.

[janakjjp]

Just updated my VSCode Insider's with the latest GH extension and it appears that the performance is very similar. Both the nightly and the stable release took 2m30s for the dialog to load.

(It seems the dialog was more immediately responsive on the new plugin, but I didn't test that part extensively.)

[alexr00]

Thanks @janakjjp for trying it out and reporting back!

At this point, given that I can't reproduce the slowness and that I've exhausted the perf gains to be had by removing unneeded calls, I think we have to look at caching next.

We're currently reworking the Create PR view. Maybe you'll see some gains as we transition to the new view. I'll also be hyper aware of perf in the new view.

Reopening to consider adding caching.

[alexr00]

@janakjjp if you try out the latest pre-release build you'll get the new Create PR view. It should be much faster. Please let me know if you try it out and see any noticeable perf change in either direction!

[alexr00]

Closing, as we don't fetch all branches before creating the view now.

[janakjjp]

Sorry I vanished on this; I got busy with vacation and other things and wasn't working on our monorepo for a few weeks.

I am running v0.72, which is from 9/8, and unfortunately the performance issue is still there; it takes ~2-3 minutes to show the list of branches. The UI is noticeably more responsive in the new view, at least, i.e. even when VSCode is churning (and pegging the CPU) there's a working indicator and the UI remains responsive to the mouse.

Let me know if you still want to pursue this and I can try and spend some more time digging in.

[alexr00]

it takes ~2-3 minutes to show the list of branches

Since the UI shows before we have all the branches now, this is no longer blocking us from showing the Create view.

@janakjjp, do you find that you need to change the branch often? If so, could you walk me through why? Here's my thinking:

• We try to do a better job guessing the best base branch now, so I wouldn't expect you to need to change branches often.
• This means that we expect changing branches to be an infrequent occurrence.
• If changing branches is infrequent, then any branch cache we maintain will probably be stale.
• If the cache is often stale, then we should just fetch all the branches every time.
• Fetching branches every time is the slow part.

Given the new Create view, I would rather see if we can improve our default branch selection than cache branches.

[janakjjp]

I agree, caching is a pain and would not be ideal. I almost never change the branch, but it wasn't clear on the UI that I could just use the defaults. 🤔 I'll give it a try and report back.

(The other issue is the CPU peg is just annoying, as everything slows down. I'd be a huge fan of defaults, and then populate everything else if you drop down the branch list. But I'll give more feedback after my next PR.)