Skip to content

Commit 6a9d5cc

Browse files
alexr00alexr00
authored
Improve PR list view performance (#7287)
Part of #7141
1 parent 1a97fe5 commit 6a9d5cc

16 files changed

+143
-76
lines changed

‎src/github/folderRepositoryManager.ts

Lines changed: 7 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -197,8 +197,8 @@ export class FolderRepositoryManager extends Disposable {
197197
private _onDidMergePullRequest = this._register(new vscode.EventEmitter<void>());
198198
readonly onDidMergePullRequest = this._onDidMergePullRequest.event;
199199

200-
private _onDidChangeActivePullRequest = this._register(new vscode.EventEmitter<{ new: number | undefined, old: number | undefined }>());
201-
readonly onDidChangeActivePullRequest: vscode.Event<{ new: number | undefined, old: number | undefined }> = this._onDidChangeActivePullRequest.event;
200+
private _onDidChangeActivePullRequest = this._register(new vscode.EventEmitter<{ new: PullRequestModel | undefined, old: PullRequestModel | undefined }>());
201+
readonly onDidChangeActivePullRequest: vscode.Event<{ new: PullRequestModel | undefined, old: PullRequestModel | undefined }> = this._onDidChangeActivePullRequest.event;
202202
private _onDidChangeActiveIssue = this._register(new vscode.EventEmitter<void>());
203203
readonly onDidChangeActiveIssue: vscode.Event<void> = this._onDidChangeActiveIssue.event;
204204

@@ -215,8 +215,8 @@ export class FolderRepositoryManager extends Disposable {
215215
readonly onDidChangeGithubRepositories: vscode.Event<GitHubRepository[]> = this._onDidChangeGithubRepositories.event;
216216

217217
private _onDidChangePullRequestsEvents: vscode.Disposable[] = [];
218-
private readonly _onDidChangeAnyPullRequests = this._register(new vscode.EventEmitter<void>());
219-
readonly onDidChangeAnyPullRequests: vscode.Event<void> = this._onDidChangeAnyPullRequests.event;
218+
private readonly _onDidChangeAnyPullRequests = this._register(new vscode.EventEmitter<IssueModel[]>());
219+
readonly onDidChangeAnyPullRequests: vscode.Event<IssueModel[]> = this._onDidChangeAnyPullRequests.event;
220220

221221
private _onDidDispose = this._register(new vscode.EventEmitter<void>());
222222
readonly onDidDispose: vscode.Event<void> = this._onDidDispose.event;
@@ -361,7 +361,7 @@ export class FolderRepositoryManager extends Disposable {
361361
if (pullRequest === this._activePullRequest) {
362362
return;
363363
}
364-
const oldNumber = this._activePullRequest?.number;
364+
const oldPR = this._activePullRequest;
365365
if (this._activePullRequest) {
366366
this._activePullRequest.isActive = false;
367367
}
@@ -370,10 +370,9 @@ export class FolderRepositoryManager extends Disposable {
370370
pullRequest.isActive = true;
371371
pullRequest.githubRepository.commentsHandler?.unregisterCommentController(pullRequest.number);
372372
}
373-
const newNumber = pullRequest?.number;
374373

375374
this._activePullRequest = pullRequest;
376-
this._onDidChangeActivePullRequest.fire({ old: oldNumber, new: newNumber });
375+
this._onDidChangeActivePullRequest.fire({ old: oldPR, new: pullRequest });
377376
}
378377

379378
get repository(): Repository {
@@ -534,7 +533,7 @@ export class FolderRepositoryManager extends Disposable {
534533
disposeAll(this._onDidChangePullRequestsEvents);
535534
this._githubRepositories = repositories;
536535
for (const repo of this._githubRepositories) {
537-
this._onDidChangePullRequestsEvents.push(repo.onDidChangePullRequests(() => this._onDidChangeAnyPullRequests.fire()));
536+
this._onDidChangePullRequestsEvents.push(repo.onDidChangePullRequests(e => this._onDidChangeAnyPullRequests.fire(e)));
538537
}
539538
oldRepositories.filter(old => this._githubRepositories.indexOf(old) < 0).forEach(repo => repo.dispose());
540539

‎src/github/githubRepository.ts

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -168,8 +168,8 @@ export class GitHubRepository extends Disposable {
168168

169169
private _onDidAddPullRequest: vscode.EventEmitter<PullRequestModel> = this._register(new vscode.EventEmitter());
170170
public readonly onDidAddPullRequest: vscode.Event<PullRequestModel> = this._onDidAddPullRequest.event;
171-
private _onDidChangePullRequests: vscode.EventEmitter<void> = this._register(new vscode.EventEmitter());
172-
public readonly onDidChangePullRequests: vscode.Event<void> = this._onDidChangePullRequests.event;
171+
private _onDidChangePullRequests: vscode.EventEmitter<IssueModel[]> = this._register(new vscode.EventEmitter());
172+
public readonly onDidChangePullRequests: vscode.Event<IssueModel[]> = this._onDidChangePullRequests.event;
173173

174174
public get hub(): GitHub {
175175
if (!this._hub) {
@@ -1511,8 +1511,8 @@ export class GitHubRepository extends Disposable {
15111511
}
15121512
const oldTimeline = issueModel.timelineEvents;
15131513
issueModel.timelineEvents = allEvents;
1514-
if (!oldLastEvent || (allEvents.length !== oldTimeline.length)) {
1515-
this._onDidChangePullRequests.fire();
1514+
if (oldLastEvent && (allEvents.length !== oldTimeline.length)) {
1515+
this._onDidChangePullRequests.fire([issueModel]);
15161516
}
15171517
}
15181518
return timelineEvents;
@@ -1562,9 +1562,9 @@ export class GitHubRepository extends Disposable {
15621562
const oldEvents = issueModel.timelineEvents;
15631563
issueModel.timelineEvents = events;
15641564
if (crossRefs.size > 0) {
1565-
this._onDidChangePullRequests.fire();
1565+
this._onDidChangePullRequests.fire([issueModel]);
15661566
} else if (oldEvents.length !== events.length) {
1567-
this._onDidChangePullRequests.fire();
1567+
this._onDidChangePullRequests.fire([issueModel]);
15681568
}
15691569
return events;
15701570
} catch (e) {
@@ -1633,7 +1633,7 @@ export class GitHubRepository extends Disposable {
16331633
const oldEvents = pullRequestModel.timelineEvents;
16341634
pullRequestModel.timelineEvents = events;
16351635
if (oldEvents.length !== events.length) {
1636-
this._onDidChangePullRequests.fire();
1636+
this._onDidChangePullRequests.fire([pullRequestModel]);
16371637
}
16381638
return events;
16391639
}

‎src/github/notifications.ts

Lines changed: 14 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -153,21 +153,31 @@ export class NotificationProvider extends Disposable {
153153
} : undefined;
154154
}
155155

156-
private adaptPRNotifications(node: TreeNode | void) {
156+
private adaptPRNotifications(node: TreeNode | TreeNode[] | void) {
157157
if (this._pollingHandler === undefined) {
158158
this.startPolling();
159159
}
160160

161-
if (node instanceof PRNode) {
162-
const prNotifications = this._notifications.get(this.getPrIdentifier(node.pullRequestModel));
161+
const updateWithPRModel = (prModel: PullRequestModel) => {
162+
const prNotifications = this._notifications.get(this.getPrIdentifier(prModel));
163163
if (prNotifications) {
164164
for (const prNotification of prNotifications) {
165165
if (prNotification) {
166-
prNotification.pullRequestModel = node.pullRequestModel;
166+
prNotification.pullRequestModel = prModel;
167167
return;
168168
}
169169
}
170170
}
171+
};
172+
173+
if (node instanceof PRNode) {
174+
updateWithPRModel(node.pullRequestModel);
175+
} else if (Array.isArray(node)) {
176+
for (const n of node) {
177+
if (n instanceof PRNode) {
178+
updateWithPRModel(n.pullRequestModel);
179+
}
180+
}
171181
}
172182

173183
this._gitHubPrsTree.cachedChildren().then(async (catNodes: CategoryTreeNode[]) => {

‎src/github/repositoriesManager.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -45,7 +45,7 @@ export class RepositoriesManager extends Disposable {
4545
private _onDidLoadAnyRepositories = new vscode.EventEmitter<void>();
4646
readonly onDidLoadAnyRepositories = this._onDidLoadAnyRepositories.event;
4747

48-
private _onDidChangeAnyPullRequests = new vscode.EventEmitter<void>();
48+
private _onDidChangeAnyPullRequests = new vscode.EventEmitter<IssueModel[]>();
4949
readonly onDidChangeAnyPullRequests = this._onDidChangeAnyPullRequests.event;
5050

5151
private _state: ReposManagerState = ReposManagerState.Initializing;
@@ -81,7 +81,7 @@ export class RepositoriesManager extends Disposable {
8181
}),
8282
folderManager.onDidChangeActivePullRequest(() => this.updateActiveReviewCount()),
8383
folderManager.onDidDispose(() => this.removeRepo(folderManager.repository)),
84-
folderManager.onDidChangeAnyPullRequests(() => this._onDidChangeAnyPullRequests.fire()),
84+
folderManager.onDidChangeAnyPullRequests(e => this._onDidChangeAnyPullRequests.fire(e)),
8585
];
8686
this._subs.set(folderManager, disposables);
8787
}

‎src/view/prsTreeDataProvider.ts

Lines changed: 44 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@ import { CopilotRemoteAgentManager } from '../github/copilotRemoteAgent';
1616
import { CredentialStore } from '../github/credentials';
1717
import { FolderRepositoryManager, ReposManagerState } from '../github/folderRepositoryManager';
1818
import { PRType } from '../github/interface';
19+
import { IssueModel } from '../github/issueModel';
1920
import { issueMarkdown } from '../github/markdownUtils';
2021
import { NotificationProvider } from '../github/notifications';
2122
import { PullRequestModel } from '../github/pullRequestModel';
@@ -33,7 +34,7 @@ import { TreeUtils } from './treeNodes/treeUtils';
3334
import { WorkspaceFolderNode } from './treeNodes/workspaceFolderNode';
3435

3536
export class PullRequestsTreeDataProvider extends Disposable implements vscode.TreeDataProvider<TreeNode>, BaseTreeNode {
36-
private _onDidChangeTreeData = new vscode.EventEmitter<TreeNode | void>();
37+
private _onDidChangeTreeData = new vscode.EventEmitter<TreeNode[] | TreeNode | void>();
3738
readonly onDidChangeTreeData = this._onDidChangeTreeData.event;
3839
private _onDidChange = new vscode.EventEmitter<vscode.Uri>();
3940
get onDidChange(): vscode.Event<vscode.Uri> {
@@ -56,7 +57,15 @@ export class PullRequestsTreeDataProvider extends Disposable implements vscode.T
5657
constructor(private readonly _telemetry: ITelemetry, private readonly _context: vscode.ExtensionContext, private readonly _reposManager: RepositoriesManager, private readonly _copilotManager: CopilotRemoteAgentManager) {
5758
super();
5859
this.prsTreeModel = this._register(new PrsTreeModel(this._telemetry, this._reposManager, _context));
59-
this._register(this.prsTreeModel.onDidChangeData(folderManager => folderManager ? this.refreshRepo(folderManager) : this.refresh()));
60+
this._register(this.prsTreeModel.onDidChangeData(e => {
61+
if (e instanceof FolderRepositoryManager) {
62+
this.refreshRepo(e);
63+
} else if (Array.isArray(e) && e[0] instanceof IssueModel) {
64+
this.refreshPullRequests(e);
65+
} else {
66+
this.refresh();
67+
}
68+
}));
6069
this._register(new PRStatusDecorationProvider(this.prsTreeModel, this._copilotManager));
6170
this._register(vscode.commands.registerCommand('pr.refreshList', _ => {
6271
this.refresh(undefined, true);
@@ -98,7 +107,6 @@ export class PullRequestsTreeDataProvider extends Disposable implements vscode.T
98107
tooltip: this._copilotManager.notificationsCount === 1 ? vscode.l10n.t('Coding agent has 1 pull request to view') : vscode.l10n.t('Coding agent has {0} pull requests to view', this._copilotManager.notificationsCount),
99108
value: this._copilotManager.notificationsCount
100109
};
101-
this.refresh();
102110
} else {
103111
this._view.badge = undefined;
104112
}
@@ -311,6 +319,39 @@ export class PullRequestsTreeDataProvider extends Disposable implements vscode.T
311319
}
312320
}
313321

322+
private refreshPullRequests(pullRequests: IssueModel[]): void {
323+
if (!this._children?.length || !pullRequests?.length) {
324+
return;
325+
}
326+
const toRefresh: TreeNode[] = [];
327+
// Yes, multiple PRs can exist in different repos with the same number, but at worst we'll refresh all the duplicate numbers, which shouldn't be many.
328+
const prNumbers = new Set(pullRequests.map(pr => pr.number));
329+
const collectPRNodes = (node: TreeNode) => {
330+
const prNodes = node.children ?? [];
331+
for (const prNode of prNodes) {
332+
if (prNode instanceof PRNode && prNumbers.has(prNode.pullRequestModel.number)) {
333+
toRefresh.push(prNode);
334+
}
335+
}
336+
};
337+
338+
for (const child of this._children) {
339+
if (child instanceof WorkspaceFolderNode) {
340+
const categories = child.children ?? [];
341+
for (const category of categories) {
342+
if (category instanceof CategoryTreeNode) {
343+
collectPRNodes(category);
344+
}
345+
}
346+
} else if (child instanceof CategoryTreeNode) {
347+
collectPRNodes(child);
348+
}
349+
}
350+
if (toRefresh.length) {
351+
this._onDidChangeTreeData.fire(toRefresh);
352+
}
353+
}
354+
314355
getTreeItem(element: TreeNode): vscode.TreeItem | Promise<vscode.TreeItem> {
315356
return element.getTreeItem();
316357
}

‎src/view/prsTreeModel.ts

Lines changed: 17 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ import { ITelemetry } from '../common/telemetry';
1010
import { createPRNodeIdentifier } from '../common/uri';
1111
import { FolderRepositoryManager, ItemsResponseResult } from '../github/folderRepositoryManager';
1212
import { CheckState, PRType, PullRequestChecks, PullRequestReviewRequirement } from '../github/interface';
13+
import { IssueModel } from '../github/issueModel';
1314
import { PullRequestModel } from '../github/pullRequestModel';
1415
import { RepositoriesManager } from '../github/repositoriesManager';
1516
import { UnsatisfiedChecks } from '../github/utils';
@@ -27,7 +28,7 @@ export class PrsTreeModel extends Disposable {
2728
private _activePRDisposables: Map<FolderRepositoryManager, vscode.Disposable[]> = new Map();
2829
private readonly _onDidChangePrStatus: vscode.EventEmitter<string[]> = this._register(new vscode.EventEmitter<string[]>());
2930
public readonly onDidChangePrStatus = this._onDidChangePrStatus.event;
30-
private readonly _onDidChangeData: vscode.EventEmitter<FolderRepositoryManager | void> = this._register(new vscode.EventEmitter<FolderRepositoryManager | void>());
31+
private readonly _onDidChangeData: vscode.EventEmitter<IssueModel[] | FolderRepositoryManager | void> = this._register(new vscode.EventEmitter<IssueModel[] | FolderRepositoryManager | void>());
3132
public readonly onDidChangeData = this._onDidChangeData.event;
3233
private _expandedQueries: Set<string> | undefined;
3334
private _hasLoaded: boolean = false;
@@ -49,21 +50,31 @@ export class PrsTreeModel extends Disposable {
4950
this._repoEvents.set(manager, []);
5051
}
5152

52-
this._repoEvents.get(manager)!.push(manager.onDidChangeActivePullRequest(() => {
53-
this.clearRepo(manager);
53+
this._repoEvents.get(manager)!.push(manager.onDidChangeActivePullRequest(e => {
54+
const prs: IssueModel[] = [];
55+
if (e.old) {
56+
prs.push(e.old);
57+
}
58+
if (e.new) {
59+
prs.push(e.new);
60+
}
61+
this._onDidChangeData.fire(prs);
62+
5463
if (this._activePRDisposables.has(manager)) {
5564
disposeAll(this._activePRDisposables.get(manager)!);
5665
this._activePRDisposables.delete(manager);
5766
}
5867
if (manager.activePullRequest) {
5968
this._activePRDisposables.set(manager, [
6069
manager.activePullRequest.onDidChangeComments(() => {
61-
this.clearRepo(manager);
70+
if (manager.activePullRequest) {
71+
this._onDidChangeData.fire([manager.activePullRequest]);
72+
}
6273
})]);
6374
}
6475
}));
65-
this._repoEvents.get(manager)!.push(manager.onDidChangeAnyPullRequests(() => {
66-
this._onDidChangeData.fire(manager);
76+
this._repoEvents.get(manager)!.push(manager.onDidChangeAnyPullRequests(e => {
77+
this._onDidChangeData.fire(e);
6778
}));
6879
};
6980
this._register({ dispose: () => this._repoEvents.forEach((disposables) => disposeAll(disposables)) });

‎src/view/pullRequestCommentControllerRegistry.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -82,7 +82,7 @@ export class PRCommentControllerRegistry extends Disposable implements vscode.Co
8282
if (!this._activeChangeListeners.has(folderRepositoryManager)) {
8383
this._activeChangeListeners.set(folderRepositoryManager, folderRepositoryManager.onDidChangeActivePullRequest(e => {
8484
if (e.old) {
85-
this._prCommentHandlers[e.old]?.dispose();
85+
this._prCommentHandlers[e.old.number]?.dispose();
8686
}
8787
}));
8888
}

‎src/view/treeNodes/categoryNode.ts

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -198,12 +198,12 @@ export class CategoryTreeNode extends TreeNode implements vscode.TreeItem {
198198
}
199199

200200
public async expandPullRequest(pullRequest: PullRequestModel, retry: boolean = true): Promise<boolean> {
201-
if (!this.children && retry) {
201+
if (!this._children && retry) {
202202
await this.getChildren();
203203
retry = false;
204204
}
205-
if (this.children) {
206-
for (const child of this.children) {
205+
if (this._children) {
206+
for (const child of this._children) {
207207
if (child instanceof PRNode) {
208208
if (child.pullRequestModel.equals(pullRequest)) {
209209
this.reveal(child, { expand: true, select: true });
@@ -222,8 +222,8 @@ export class CategoryTreeNode extends TreeNode implements vscode.TreeItem {
222222

223223
override async getChildren(shouldDispose: boolean = true): Promise<TreeNode[]> {
224224
await super.getChildren(shouldDispose);
225-
if (!shouldDispose && this.children) {
226-
return this.children;
225+
if (!shouldDispose && this._children) {
226+
return this._children;
227227
}
228228
const isFirstLoad = !this._firstLoad;
229229
if (isFirstLoad) {
@@ -293,13 +293,13 @@ export class CategoryTreeNode extends TreeNode implements vscode.TreeItem {
293293
nodes.push(new PRCategoryActionNode(this, PRCategoryActionType.TryOtherRemotes, this));
294294
}
295295

296-
this.children = nodes;
296+
this._children = nodes;
297297
return nodes;
298298
} else {
299299
const category = needLogin ? PRCategoryActionType.Login : PRCategoryActionType.Empty;
300300
const result = [new PRCategoryActionNode(this, category)];
301301

302-
this.children = result;
302+
this._children = result;
303303
return result;
304304
}
305305
}

‎src/view/treeNodes/commitNode.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -107,15 +107,15 @@ export class CommitNode extends TreeNode implements vscode.TreeItem {
107107
dirNode.finalize();
108108
if (dirNode.label === '') {
109109
// nothing on the root changed, pull children to parent
110-
result.push(...dirNode.children);
110+
result.push(...dirNode._children);
111111
} else {
112112
result.push(dirNode);
113113
}
114114
} else {
115115
// flat view
116116
result = fileChangeNodes;
117117
}
118-
this.children = result;
118+
this._children = result;
119119
return result;
120120
}
121121
}

‎src/view/treeNodes/commitsCategoryNode.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -46,11 +46,11 @@ export class CommitsNode extends TreeNode implements vscode.TreeItem {
4646
try {
4747
Logger.appendLine(`Getting children for Commits node`, PR_TREE);
4848
const commits = await this._pr.getCommits();
49-
this.children = commits.map(
49+
this._children = commits.map(
5050
(commit, index) => new CommitNode(this, this._folderRepoManager, this._pr, commit, (index === commits.length - 1) && (this._folderRepoManager.repository.state.HEAD?.commit === commit.sha)),
5151
);
5252
Logger.appendLine(`Got all children for Commits node`, PR_TREE);
53-
return this.children;
53+
return this._children;
5454
} catch (e) {
5555
return [];
5656
}

0 commit comments

Comments
 (0)