perf: dedupe PR thread fetches across tree + notification poll#67
Open
CompN3rd wants to merge 2 commits into
Open
perf: dedupe PR thread fetches across tree + notification poll#67CompN3rd wants to merge 2 commits into
CompN3rd wants to merge 2 commits into
Conversation
Introduce PrThreadCache: a small TTL (30s) + in-flight-promise dedup map keyed on org/project/repo/prId. Wire the same instance into both PullRequestProvider and PrCommentHandler. Before: each notification poll tick fetched threads for every active PR, and any subsequent tree expand within seconds re-fetched the same data. Multiple concurrent expands of the same PR (refresh-while-expanding) also issued duplicate requests. After: poll populates the cache; tree expand within 30s is served from memory; concurrent fetches share one in-flight promise. Tree refresh (full or per-bucket) invalidates the cache so user-triggered refreshes still see fresh data. Cache is optional in both constructors to keep callers without DI happy.
There was a problem hiding this comment.
Pull request overview
This PR introduces a shared, short-lived cache for PR comment threads to reduce redundant Azure DevOps API calls across the Pull Requests tree view and the PR comment notification poller, including deduplication of concurrent fetches via shared in-flight promises.
Changes:
- Added
PrThreadCache(30s TTL + in-flight promise dedup) keyed byorg/project/repo/prId. - Wired a single
PrThreadCacheinstance into bothPullRequestProviderandPrCommentHandler. - Cleared the cache on tree refresh / bucket refresh to keep user-triggered refresh behavior consistent.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| src/providers/pullRequestProvider.ts | Uses PrThreadCache.getOrFetch() when available and clears cache on refresh actions. |
| src/providers/prThreadCache.ts | New TTL + in-flight dedup cache implementation for PR comment threads. |
| src/notifications/handlers/prCommentHandler.ts | Routes thread polling through PrThreadCache when provided. |
| src/extension.ts | Creates one shared PrThreadCache instance and passes it to both consumers. |
Comment on lines
+35
to
+38
| * Consumers (tree provider, notification poll, details panel, diff controller) | ||
| * all hit the same instance via `getOrFetch`. Within the TTL window, repeat | ||
| * calls return cached data; concurrent calls for the same PR share a single | ||
| * in-flight promise so we never issue duplicate ADO requests. |
Comment on lines
+52
to
+64
| get(k: PrThreadKey): GitPullRequestCommentThread[] | undefined { | ||
| const entry = this.entries.get(keyOf(k)); | ||
| if (!entry) { return undefined; } | ||
| if (entry.expires <= Date.now()) { | ||
| this.entries.delete(keyOf(k)); | ||
| return undefined; | ||
| } | ||
| return entry.threads; | ||
| } | ||
|
|
||
| set(k: PrThreadKey, threads: GitPullRequestCommentThread[]): void { | ||
| this.entries.set(keyOf(k), { threads, expires: Date.now() + this.ttlMs }); | ||
| } |
| } | ||
|
|
||
| clear(): void { | ||
| this.entries.clear(); |
- clear() now also drops in-flight promises so an active fetch can't repopulate the cache with stale data after a refresh - set() opportunistically sweeps expired entries to bound memory growth - docstring corrected to list only the wired consumers (PullRequestProvider, PrCommentHandler); details panel and diff controller were aspirational
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Introduces
PrThreadCache\u2014 a small TTL (30s) + in-flight-promise dedup map keyed onorg/project/repo/prId. Wires the same instance into bothPullRequestProviderandPrCommentHandler.Problem
Before this change, comment threads were re-fetched aggressively:
After
getOrFetchfor the same PR share one in-flight promise \u2014 second caller waits for the first request.Design notes
PullRequestProvider,PrCommentHandler). Callers that don't pass one fall back to the existing direct call path; useful for tests and keeps the diff small.Files
src/providers/prThreadCache.tssrc/providers/pullRequestProvider.ts\u2014 optionalthreadCachector param;loadThreadsroutes through it;refresh/refreshBucketclear itsrc/notifications/handlers/prCommentHandler.ts\u2014 optional_threadCachector param;pollPullRequestsroutes through itsrc/extension.ts\u2014 constructs singlePrThreadCacheand passes to both consumersVerification
npm run compilecleannpm run lintclean