-
Notifications
You must be signed in to change notification settings - Fork 3.7k
fix: autocomplete performance issues #8364
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
ff36b8f
14caa79
d39e7af
8ff2b85
3dcf050
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -128,6 +128,9 @@ const getSnippetsFromRecentlyOpenedFiles = async ( | |
| // Create a promise that resolves to a snippet or null | ||
| const readPromise = new Promise<AutocompleteCodeSnippet | null>( | ||
| (resolve) => { | ||
| console.log( | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This debug console.log runs on every file read from the recently opened cache, so autocomplete will emit a log per file on each invocation; this adds synchronous logging overhead and floods the console, undermining the performance improvements you're targeting. Please drop or gate the log before merging. Prompt for AI agentsThere was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This new console.log will fire for every recently opened file read, spamming the console and adding overhead in production usages. Please remove it or gate it behind the existing logging/instrumentation mechanism. Prompt for AI agents |
||
| `read file - getAllSnippets getSnippetsFromRecentlyOpenedFiles - ${fileUri}`, | ||
| ); | ||
| ide | ||
| .readFile(fileUri) | ||
| .then((fileContent) => { | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,65 @@ | ||
| import { LRUCache } from "lru-cache"; | ||
| import * as URI from "uri-js"; | ||
|
|
||
| const MAX_READ_CACHE_SIZE = 100; | ||
| type ReadFn = (uri: string) => Promise<string>; | ||
| export class AutocompleteReadCache { | ||
| private static _instance: AutocompleteReadCache | null = null; | ||
| private cache: LRUCache<string, string>; | ||
|
|
||
| constructor(private readonly readFile: ReadFn) { | ||
| this.cache = new LRUCache({ | ||
| max: MAX_READ_CACHE_SIZE, | ||
| ttl: 5 * 60 * 1000, // 5 minutes in milliseconds | ||
| updateAgeOnGet: true, | ||
| }); | ||
| } | ||
|
|
||
| static getInstance(readFile: ReadFn): AutocompleteReadCache { | ||
| if (!AutocompleteReadCache._instance) { | ||
| AutocompleteReadCache._instance = new AutocompleteReadCache(readFile); | ||
| } | ||
| return AutocompleteReadCache._instance; | ||
| } | ||
|
|
||
| get(uri: string): string | undefined { | ||
| return this.cache.get(URI.normalize(uri)); | ||
| } | ||
|
|
||
| set(uri: string, contents: string): void { | ||
| this.cache.set(URI.normalize(uri), contents); | ||
| } | ||
|
|
||
| delete(uri: string): boolean { | ||
| return this.cache.delete(URI.normalize(uri)); | ||
| } | ||
|
|
||
| has(uri: string): boolean { | ||
| return this.cache.has(URI.normalize(uri)); | ||
| } | ||
|
|
||
| clear(): void { | ||
| this.cache.clear(); | ||
| } | ||
|
|
||
| size(): number { | ||
| return this.cache.size; | ||
| } | ||
|
|
||
| async getOrRead(uri: string): Promise<string> { | ||
| const normalizedUri = URI.normalize(uri); | ||
| const cached = this.cache.get(normalizedUri); | ||
| if (cached !== undefined) { | ||
| return cached; | ||
| } | ||
| console.log(`read file - Autocomplete read cache - ${normalizedUri}`); | ||
|
|
||
| const contents = await this.readFile(normalizedUri); | ||
| this.cache.set(normalizedUri, contents); | ||
| return contents; | ||
| } | ||
|
|
||
| invalidate(uri: string): void { | ||
| this.cache.delete(URI.normalize(uri)); | ||
| } | ||
| } |
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Commenting out the onDidChangeActiveTextEditor handler stops warming this cache when the user switches files, so import definition lookups now return undefined instead of the cached definitions.
Prompt for AI agents