Improve PR list view performance (#7287)

Part of #7141

Author: alexr00

src/github/folderRepositoryManager.ts

@@ -197,8 +197,8 @@ export class FolderRepositoryManager extends Disposable {
 	private _onDidMergePullRequest = this._register(new vscode.EventEmitter<void>());
 	readonly onDidMergePullRequest = this._onDidMergePullRequest.event;
 
-	private _onDidChangeActivePullRequest = this._register(new vscode.EventEmitter<{ new: number | undefined, old: number | undefined }>());
-	readonly onDidChangeActivePullRequest: vscode.Event<{ new: number | undefined, old: number | undefined }> = this._onDidChangeActivePullRequest.event;
+	private _onDidChangeActivePullRequest = this._register(new vscode.EventEmitter<{ new: PullRequestModel | undefined, old: PullRequestModel | undefined }>());
+	readonly onDidChangeActivePullRequest: vscode.Event<{ new: PullRequestModel | undefined, old: PullRequestModel | undefined }> = this._onDidChangeActivePullRequest.event;
 	private _onDidChangeActiveIssue = this._register(new vscode.EventEmitter<void>());
 	readonly onDidChangeActiveIssue: vscode.Event<void> = this._onDidChangeActiveIssue.event;
 
@@ -215,8 +215,8 @@ export class FolderRepositoryManager extends Disposable {
 	readonly onDidChangeGithubRepositories: vscode.Event<GitHubRepository[]> = this._onDidChangeGithubRepositories.event;
 
 	private _onDidChangePullRequestsEvents: vscode.Disposable[] = [];
-	private readonly _onDidChangeAnyPullRequests = this._register(new vscode.EventEmitter<void>());
-	readonly onDidChangeAnyPullRequests: vscode.Event<void> = this._onDidChangeAnyPullRequests.event;
+	private readonly _onDidChangeAnyPullRequests = this._register(new vscode.EventEmitter<IssueModel[]>());
+	readonly onDidChangeAnyPullRequests: vscode.Event<IssueModel[]> = this._onDidChangeAnyPullRequests.event;
 
 	private _onDidDispose = this._register(new vscode.EventEmitter<void>());
 	readonly onDidDispose: vscode.Event<void> = this._onDidDispose.event;
@@ -361,7 +361,7 @@ export class FolderRepositoryManager extends Disposable {
 		if (pullRequest === this._activePullRequest) {
 			return;
 		}
-		const oldNumber = this._activePullRequest?.number;
+		const oldPR = this._activePullRequest;
 		if (this._activePullRequest) {
 			this._activePullRequest.isActive = false;
 		}
@@ -370,10 +370,9 @@ export class FolderRepositoryManager extends Disposable {
 			pullRequest.isActive = true;
 			pullRequest.githubRepository.commentsHandler?.unregisterCommentController(pullRequest.number);
 		}
-		const newNumber = pullRequest?.number;
 
 		this._activePullRequest = pullRequest;
-		this._onDidChangeActivePullRequest.fire({ old: oldNumber, new: newNumber });
+		this._onDidChangeActivePullRequest.fire({ old: oldPR, new: pullRequest });
 	}
 
 	get repository(): Repository {
@@ -534,7 +533,7 @@ export class FolderRepositoryManager extends Disposable {
 			disposeAll(this._onDidChangePullRequestsEvents);
 			this._githubRepositories = repositories;
 			for (const repo of this._githubRepositories) {
-				this._onDidChangePullRequestsEvents.push(repo.onDidChangePullRequests(() => this._onDidChangeAnyPullRequests.fire()));
+				this._onDidChangePullRequestsEvents.push(repo.onDidChangePullRequests(e => this._onDidChangeAnyPullRequests.fire(e)));
 			}
 			oldRepositories.filter(old => this._githubRepositories.indexOf(old) < 0).forEach(repo => repo.dispose());
 

src/github/githubRepository.ts

@@ -168,8 +168,8 @@ export class GitHubRepository extends Disposable {
 
 	private _onDidAddPullRequest: vscode.EventEmitter<PullRequestModel> = this._register(new vscode.EventEmitter());
 	public readonly onDidAddPullRequest: vscode.Event<PullRequestModel> = this._onDidAddPullRequest.event;
-	private _onDidChangePullRequests: vscode.EventEmitter<void> = this._register(new vscode.EventEmitter());
-	public readonly onDidChangePullRequests: vscode.Event<void> = this._onDidChangePullRequests.event;
+	private _onDidChangePullRequests: vscode.EventEmitter<IssueModel[]> = this._register(new vscode.EventEmitter());
+	public readonly onDidChangePullRequests: vscode.Event<IssueModel[]> = this._onDidChangePullRequests.event;
 
 	public get hub(): GitHub {
 		if (!this._hub) {
@@ -1511,8 +1511,8 @@ export class GitHubRepository extends Disposable {
 				}
 				const oldTimeline = issueModel.timelineEvents;
 				issueModel.timelineEvents = allEvents;
-				if (!oldLastEvent || (allEvents.length !== oldTimeline.length)) {
-					this._onDidChangePullRequests.fire();
+				if (oldLastEvent && (allEvents.length !== oldTimeline.length)) {
+					this._onDidChangePullRequests.fire([issueModel]);
 				}
 			}
 			return timelineEvents;
@@ -1562,9 +1562,9 @@ export class GitHubRepository extends Disposable {
 			const oldEvents = issueModel.timelineEvents;
 			issueModel.timelineEvents = events;
 			if (crossRefs.size > 0) {
-				this._onDidChangePullRequests.fire();
+				this._onDidChangePullRequests.fire([issueModel]);
 			} else if (oldEvents.length !== events.length) {
-				this._onDidChangePullRequests.fire();
+				this._onDidChangePullRequests.fire([issueModel]);
 			}
 			return events;
 		} catch (e) {
@@ -1633,7 +1633,7 @@ export class GitHubRepository extends Disposable {
 		const oldEvents = pullRequestModel.timelineEvents;
 		pullRequestModel.timelineEvents = events;
 		if (oldEvents.length !== events.length) {
-			this._onDidChangePullRequests.fire();
+			this._onDidChangePullRequests.fire([pullRequestModel]);
 		}
 		return events;
 	}

src/github/notifications.ts

@@ -153,21 +153,31 @@ export class NotificationProvider extends Disposable {
 		} : undefined;
 	}
 
-	private adaptPRNotifications(node: TreeNode | void) {
+	private adaptPRNotifications(node: TreeNode | TreeNode[] | void) {
 		if (this._pollingHandler === undefined) {
 			this.startPolling();
 		}
 
-		if (node instanceof PRNode) {
-			const prNotifications = this._notifications.get(this.getPrIdentifier(node.pullRequestModel));
+		const updateWithPRModel = (prModel: PullRequestModel) => {
+			const prNotifications = this._notifications.get(this.getPrIdentifier(prModel));
 			if (prNotifications) {
 				for (const prNotification of prNotifications) {
 					if (prNotification) {
-						prNotification.pullRequestModel = node.pullRequestModel;
+						prNotification.pullRequestModel = prModel;
 						return;
 					}
 				}
 			}
+		};
+
+		if (node instanceof PRNode) {
+			updateWithPRModel(node.pullRequestModel);
+		} else if (Array.isArray(node)) {
+			for (const n of node) {
+				if (n instanceof PRNode) {
+					updateWithPRModel(n.pullRequestModel);
+				}
+			}
 		}
 
 		this._gitHubPrsTree.cachedChildren().then(async (catNodes: CategoryTreeNode[]) => {

src/github/repositoriesManager.ts

@@ -45,7 +45,7 @@ export class RepositoriesManager extends Disposable {
 	private _onDidLoadAnyRepositories = new vscode.EventEmitter<void>();
 	readonly onDidLoadAnyRepositories = this._onDidLoadAnyRepositories.event;
 
-	private _onDidChangeAnyPullRequests = new vscode.EventEmitter<void>();
+	private _onDidChangeAnyPullRequests = new vscode.EventEmitter<IssueModel[]>();
 	readonly onDidChangeAnyPullRequests = this._onDidChangeAnyPullRequests.event;
 
 	private _state: ReposManagerState = ReposManagerState.Initializing;
@@ -81,7 +81,7 @@ export class RepositoriesManager extends Disposable {
 			}),
 			folderManager.onDidChangeActivePullRequest(() => this.updateActiveReviewCount()),
 			folderManager.onDidDispose(() => this.removeRepo(folderManager.repository)),
-			folderManager.onDidChangeAnyPullRequests(() => this._onDidChangeAnyPullRequests.fire()),
+			folderManager.onDidChangeAnyPullRequests(e => this._onDidChangeAnyPullRequests.fire(e)),
 		];
 		this._subs.set(folderManager, disposables);
 	}

src/view/prsTreeDataProvider.ts

@@ -16,6 +16,7 @@ import { CopilotRemoteAgentManager } from '../github/copilotRemoteAgent';
 import { CredentialStore } from '../github/credentials';
 import { FolderRepositoryManager, ReposManagerState } from '../github/folderRepositoryManager';
 import { PRType } from '../github/interface';
+import { IssueModel } from '../github/issueModel';
 import { issueMarkdown } from '../github/markdownUtils';
 import { NotificationProvider } from '../github/notifications';
 import { PullRequestModel } from '../github/pullRequestModel';
@@ -33,7 +34,7 @@ import { TreeUtils } from './treeNodes/treeUtils';
 import { WorkspaceFolderNode } from './treeNodes/workspaceFolderNode';
 
 export class PullRequestsTreeDataProvider extends Disposable implements vscode.TreeDataProvider<TreeNode>, BaseTreeNode {
-	private _onDidChangeTreeData = new vscode.EventEmitter<TreeNode | void>();
+	private _onDidChangeTreeData = new vscode.EventEmitter<TreeNode[] | TreeNode | void>();
 	readonly onDidChangeTreeData = this._onDidChangeTreeData.event;
 	private _onDidChange = new vscode.EventEmitter<vscode.Uri>();
 	get onDidChange(): vscode.Event<vscode.Uri> {
@@ -56,7 +57,15 @@ export class PullRequestsTreeDataProvider extends Disposable implements vscode.T
 	constructor(private readonly _telemetry: ITelemetry, private readonly _context: vscode.ExtensionContext, private readonly _reposManager: RepositoriesManager, private readonly _copilotManager: CopilotRemoteAgentManager) {
 		super();
 		this.prsTreeModel = this._register(new PrsTreeModel(this._telemetry, this._reposManager, _context));
-		this._register(this.prsTreeModel.onDidChangeData(folderManager => folderManager ? this.refreshRepo(folderManager) : this.refresh()));
+		this._register(this.prsTreeModel.onDidChangeData(e => {
+			if (e instanceof FolderRepositoryManager) {
+				this.refreshRepo(e);
+			} else if (Array.isArray(e) && e[0] instanceof IssueModel) {
+				this.refreshPullRequests(e);
+			} else {
+				this.refresh();
+			}
+		}));
 		this._register(new PRStatusDecorationProvider(this.prsTreeModel, this._copilotManager));
 		this._register(vscode.commands.registerCommand('pr.refreshList', _ => {
 			this.refresh(undefined, true);
@@ -98,7 +107,6 @@ export class PullRequestsTreeDataProvider extends Disposable implements vscode.T
 					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),
 					value: this._copilotManager.notificationsCount
 				};
-				this.refresh();
 			} else {
 				this._view.badge = undefined;
 			}
@@ -311,6 +319,39 @@ export class PullRequestsTreeDataProvider extends Disposable implements vscode.T
 		}
 	}
 
+	private refreshPullRequests(pullRequests: IssueModel[]): void {
+		if (!this._children?.length || !pullRequests?.length) {
+			return;
+		}
+		const toRefresh: TreeNode[] = [];
+		// 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.
+		const prNumbers = new Set(pullRequests.map(pr => pr.number));
+		const collectPRNodes = (node: TreeNode) => {
+			const prNodes = node.children ?? [];
+			for (const prNode of prNodes) {
+				if (prNode instanceof PRNode && prNumbers.has(prNode.pullRequestModel.number)) {
+					toRefresh.push(prNode);
+				}
+			}
+		};
+
+		for (const child of this._children) {
+			if (child instanceof WorkspaceFolderNode) {
+				const categories = child.children ?? [];
+				for (const category of categories) {
+					if (category instanceof CategoryTreeNode) {
+						collectPRNodes(category);
+					}
+				}
+			} else if (child instanceof CategoryTreeNode) {
+				collectPRNodes(child);
+			}
+		}
+		if (toRefresh.length) {
+			this._onDidChangeTreeData.fire(toRefresh);
+		}
+	}
+
 	getTreeItem(element: TreeNode): vscode.TreeItem | Promise<vscode.TreeItem> {
 		return element.getTreeItem();
 	}

src/view/prsTreeModel.ts

@@ -10,6 +10,7 @@ import { ITelemetry } from '../common/telemetry';
 import { createPRNodeIdentifier } from '../common/uri';
 import { FolderRepositoryManager, ItemsResponseResult } from '../github/folderRepositoryManager';
 import { CheckState, PRType, PullRequestChecks, PullRequestReviewRequirement } from '../github/interface';
+import { IssueModel } from '../github/issueModel';
 import { PullRequestModel } from '../github/pullRequestModel';
 import { RepositoriesManager } from '../github/repositoriesManager';
 import { UnsatisfiedChecks } from '../github/utils';
@@ -27,7 +28,7 @@ export class PrsTreeModel extends Disposable {
 	private _activePRDisposables: Map<FolderRepositoryManager, vscode.Disposable[]> = new Map();
 	private readonly _onDidChangePrStatus: vscode.EventEmitter<string[]> = this._register(new vscode.EventEmitter<string[]>());
 	public readonly onDidChangePrStatus = this._onDidChangePrStatus.event;
-	private readonly _onDidChangeData: vscode.EventEmitter<FolderRepositoryManager | void> = this._register(new vscode.EventEmitter<FolderRepositoryManager | void>());
+	private readonly _onDidChangeData: vscode.EventEmitter<IssueModel[] | FolderRepositoryManager | void> = this._register(new vscode.EventEmitter<IssueModel[] | FolderRepositoryManager | void>());
 	public readonly onDidChangeData = this._onDidChangeData.event;
 	private _expandedQueries: Set<string> | undefined;
 	private _hasLoaded: boolean = false;
@@ -49,21 +50,31 @@ export class PrsTreeModel extends Disposable {
 				this._repoEvents.set(manager, []);
 			}
 
-			this._repoEvents.get(manager)!.push(manager.onDidChangeActivePullRequest(() => {
-				this.clearRepo(manager);
+			this._repoEvents.get(manager)!.push(manager.onDidChangeActivePullRequest(e => {
+				const prs: IssueModel[] = [];
+				if (e.old) {
+					prs.push(e.old);
+				}
+				if (e.new) {
+					prs.push(e.new);
+				}
+				this._onDidChangeData.fire(prs);
+
 				if (this._activePRDisposables.has(manager)) {
 					disposeAll(this._activePRDisposables.get(manager)!);
 					this._activePRDisposables.delete(manager);
 				}
 				if (manager.activePullRequest) {
 					this._activePRDisposables.set(manager, [
 						manager.activePullRequest.onDidChangeComments(() => {
-							this.clearRepo(manager);
+							if (manager.activePullRequest) {
+								this._onDidChangeData.fire([manager.activePullRequest]);
+							}
 						})]);
 				}
 			}));
-			this._repoEvents.get(manager)!.push(manager.onDidChangeAnyPullRequests(() => {
-				this._onDidChangeData.fire(manager);
+			this._repoEvents.get(manager)!.push(manager.onDidChangeAnyPullRequests(e => {
+				this._onDidChangeData.fire(e);
 			}));
 		};
 		this._register({ dispose: () => this._repoEvents.forEach((disposables) => disposeAll(disposables)) });

src/view/pullRequestCommentControllerRegistry.ts

@@ -82,7 +82,7 @@ export class PRCommentControllerRegistry extends Disposable implements vscode.Co
 		if (!this._activeChangeListeners.has(folderRepositoryManager)) {
 			this._activeChangeListeners.set(folderRepositoryManager, folderRepositoryManager.onDidChangeActivePullRequest(e => {
 				if (e.old) {
-					this._prCommentHandlers[e.old]?.dispose();
+					this._prCommentHandlers[e.old.number]?.dispose();
 				}
 			}));
 		}

src/view/treeNodes/categoryNode.ts

@@ -198,12 +198,12 @@ export class CategoryTreeNode extends TreeNode implements vscode.TreeItem {
 	}
 
 	public async expandPullRequest(pullRequest: PullRequestModel, retry: boolean = true): Promise<boolean> {
-		if (!this.children && retry) {
+		if (!this._children && retry) {
 			await this.getChildren();
 			retry = false;
 		}
-		if (this.children) {
-			for (const child of this.children) {
+		if (this._children) {
+			for (const child of this._children) {
 				if (child instanceof PRNode) {
 					if (child.pullRequestModel.equals(pullRequest)) {
 						this.reveal(child, { expand: true, select: true });
@@ -222,8 +222,8 @@ export class CategoryTreeNode extends TreeNode implements vscode.TreeItem {
 
 	override async getChildren(shouldDispose: boolean = true): Promise<TreeNode[]> {
 		await super.getChildren(shouldDispose);
-		if (!shouldDispose && this.children) {
-			return this.children;
+		if (!shouldDispose && this._children) {
+			return this._children;
 		}
 		const isFirstLoad = !this._firstLoad;
 		if (isFirstLoad) {
@@ -293,13 +293,13 @@ export class CategoryTreeNode extends TreeNode implements vscode.TreeItem {
 				nodes.push(new PRCategoryActionNode(this, PRCategoryActionType.TryOtherRemotes, this));
 			}
 
-			this.children = nodes;
+			this._children = nodes;
 			return nodes;
 		} else {
 			const category = needLogin ? PRCategoryActionType.Login : PRCategoryActionType.Empty;
 			const result = [new PRCategoryActionNode(this, category)];
 
-			this.children = result;
+			this._children = result;
 			return result;
 		}
 	}

src/view/treeNodes/commitNode.ts

@@ -107,15 +107,15 @@ export class CommitNode extends TreeNode implements vscode.TreeItem {
 			dirNode.finalize();
 			if (dirNode.label === '') {
 				// nothing on the root changed, pull children to parent
-				result.push(...dirNode.children);
+				result.push(...dirNode._children);
 			} else {
 				result.push(dirNode);
 			}
 		} else {
 			// flat view
 			result = fileChangeNodes;
 		}
-		this.children = result;
+		this._children = result;
 		return result;
 	}
 }

src/view/treeNodes/commitsCategoryNode.ts

@@ -46,11 +46,11 @@ export class CommitsNode extends TreeNode implements vscode.TreeItem {
 		try {
 			Logger.appendLine(`Getting children for Commits node`, PR_TREE);
 			const commits = await this._pr.getCommits();
-			this.children = commits.map(
+			this._children = commits.map(
 				(commit, index) => new CommitNode(this, this._folderRepoManager, this._pr, commit, (index === commits.length - 1) && (this._folderRepoManager.repository.state.HEAD?.commit === commit.sha)),
 			);
 			Logger.appendLine(`Got all children for Commits node`, PR_TREE);
-			return this.children;
+			return this._children;
 		} catch (e) {
 			return [];
 		}