Skip to content

Commit adee249

Browse files
committed
fix: significantly improve completions performance in heavy applications that use MUI (unoptimized Material-UI) by using cached diagnostics for not declared const variable names suggestions instead
1 parent fe70eb3 commit adee249

File tree

5 files changed

+84
-47
lines changed

5 files changed

+84
-47
lines changed

.eslintrc.json

+2-1
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,8 @@
3434
"@typescript-eslint/consistent-type-imports": "off",
3535
"@typescript-eslint/ban-types": "off",
3636
"sonarjs/prefer-single-boolean-return": "off",
37-
"unicorn/no-typeof-undefined": "off" // todo disable globally
37+
"unicorn/no-typeof-undefined": "off", // todo disable globally
38+
"@typescript-eslint/consistent-type-definitions": "off"
3839
},
3940
"overrides": [
4041
{

typescript/src/completions/boostNameSuggestions.ts

+2-1
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
import { cachedResponse } from '../decorateProxy'
12
import { boostExistingSuggestions, boostOrAddSuggestions, findChildContainingPosition } from '../utils'
23
import { getCannotFindCodes } from '../utils/cannotFindCodes'
34

@@ -46,7 +47,7 @@ export default (
4647
}
4748

4849
if (filterBlock === undefined) return entries
49-
const semanticDiagnostics = languageService.getSemanticDiagnostics(sourceFile.fileName)
50+
const semanticDiagnostics = cachedResponse.getSemanticDiagnostics?.[sourceFile.fileName] ?? []
5051

5152
const notFoundIdentifiers = semanticDiagnostics
5253
.filter(({ code }) => cannotFindCodes.includes(code))

typescript/src/decorateProxy.ts

+12
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,10 @@ export const getInitialProxy = (languageService: ts.LanguageService, proxy = Obj
3333
return proxy
3434
}
3535

36+
export const cachedResponse = {
37+
getSemanticDiagnostics: {} as Record<string, ts.Diagnostic[]>,
38+
}
39+
3640
export const decorateLanguageService = (
3741
{ languageService, languageServiceHost }: PluginCreateArg,
3842
existingProxy: ts.LanguageService | undefined,
@@ -169,11 +173,19 @@ export const decorateLanguageService = (
169173
.filter(([, v]) => v === false)
170174
.map(([k]) => k)
171175
if (toDisable.includes(feature)) return undefined
176+
172177
// eslint-disable-next-line @typescript-eslint/no-require-imports
173178
const performance = globalThis.performance ?? require('perf_hooks').performance
174179
const start = performance.now()
180+
175181
//@ts-expect-error
176182
const result = orig(...args)
183+
184+
if (feature in cachedResponse) {
185+
// todo use weakmap with sourcefiles to ensure it doesn't grow up
186+
cachedResponse[feature][args[0]] = result
187+
}
188+
177189
const time = performance.now() - start
178190
if (time > 100) console.log(`[typescript-vscode-plugin perf warning] ${feature} took ${time}ms: ${args[0]} ${args[1]}`)
179191
return result

typescript/test/completions.spec.ts

+13
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,19 @@ test('Banned positions', () => {
4040
expect(getCompletionsAtPosition(cursorPositions[2]!)?.entries).toHaveLength(1)
4141
})
4242

43+
test.todo('Const name suggestions (boostNameSuggestions)', () => {
44+
const tester = fourslashLikeTester(/* ts */ `
45+
const /*0*/ = 5
46+
testVariable
47+
`)
48+
languageService.getSemanticDiagnostics(entrypoint)
49+
tester.completion(0, {
50+
includes: {
51+
names: ['testVariable'],
52+
},
53+
})
54+
})
55+
4356
test('Banned positions for all method snippets', () => {
4457
const cursorPositions = newFileContents(/* tsx */ `
4558
import {/*|*/} from 'test'

typescript/test/testing.ts

+55-45
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,7 @@ interface CodeActionMatcher {
2727

2828
const { languageService, languageServiceHost, updateProject, getCurrentFile } = sharedLanguageService
2929

30-
const fakeProxy = {} as Pick<typeof languageService, 'getApplicableRefactors' | 'getEditsForRefactor'>
30+
export const fakeProxy = {} as Pick<typeof languageService, 'getApplicableRefactors' | 'getEditsForRefactor'>
3131

3232
codeActionsDecorateProxy(fakeProxy as typeof languageService, languageService, languageServiceHost, defaultConfigFunc)
3333

@@ -82,7 +82,7 @@ export const fourslashLikeTester = (contents: string, fileName = entrypoint, { d
8282

8383
const ranges = positive.reduce<number[][]>(
8484
(prevRanges, pos) => {
85-
const lastPrev = prevRanges[prevRanges.length - 1]!
85+
const lastPrev = prevRanges.at(-1)!
8686
if (lastPrev.length < 2) {
8787
lastPrev.push(pos)
8888
return prevRanges
@@ -92,58 +92,68 @@ export const fourslashLikeTester = (contents: string, fileName = entrypoint, { d
9292
[[]],
9393
)
9494
return {
95-
completion: (marker: number | number[], matcher: CompletionMatcher, meta?) => {
96-
for (const mark of Array.isArray(marker) ? marker : [marker]) {
97-
if (numberedPositions[mark] === undefined) throw new Error(`No marker ${mark} found`)
98-
const result = getCompletionsAtPosition(numberedPositions[mark]!, { shouldHave: true })!
99-
const message = ` at marker ${mark}`
100-
const { exact, includes, excludes } = matcher
101-
if (exact) {
102-
const { names, all, insertTexts } = exact
103-
if (names) {
104-
expect(result?.entryNames, message).toEqual(names)
105-
}
106-
if (insertTexts) {
107-
expect(
108-
result.entries.map(entry => entry.insertText),
109-
message,
110-
).toEqual(insertTexts)
111-
}
112-
if (all) {
113-
for (const entry of result.entries) {
114-
expect(entry, entry.name + message).toContain(all)
115-
}
116-
}
117-
}
118-
if (includes) {
119-
const { names, all, insertTexts } = includes
120-
if (names) {
121-
for (const name of names) {
122-
expect(result?.entryNames, message).toContain(name)
95+
completion(marker: number | number[], matcher: CompletionMatcher, meta?) {
96+
const oldGetSemanticDiagnostics = languageService.getSemanticDiagnostics
97+
languageService.getSemanticDiagnostics = () => {
98+
throw new Error('getSemanticDiagnostics should not be called because of performance reasons')
99+
// return []
100+
}
101+
102+
try {
103+
for (const mark of Array.isArray(marker) ? marker : [marker]) {
104+
if (numberedPositions[mark] === undefined) throw new Error(`No marker ${mark} found`)
105+
const result = getCompletionsAtPosition(numberedPositions[mark]!, { shouldHave: true })!
106+
const message = ` at marker ${mark}`
107+
const { exact, includes, excludes } = matcher
108+
if (exact) {
109+
const { names, all, insertTexts } = exact
110+
if (names) {
111+
expect(result?.entryNames, message).toEqual(names)
123112
}
124-
}
125-
if (insertTexts) {
126-
for (const insertText of insertTexts) {
113+
if (insertTexts) {
127114
expect(
128115
result.entries.map(entry => entry.insertText),
129116
message,
130-
).toContain(insertText)
117+
).toEqual(insertTexts)
118+
}
119+
if (all) {
120+
for (const entry of result.entries) {
121+
expect(entry, entry.name + message).toContain(all)
122+
}
131123
}
132124
}
133-
if (all) {
134-
for (const entry of result.entries.filter(e => names?.includes(e.name))) {
135-
expect(entry, entry.name + message).toContain(all)
125+
if (includes) {
126+
const { names, all, insertTexts } = includes
127+
if (names) {
128+
for (const name of names) {
129+
expect(result?.entryNames, message).toContain(name)
130+
}
131+
}
132+
if (insertTexts) {
133+
for (const insertText of insertTexts) {
134+
expect(
135+
result.entries.map(entry => entry.insertText),
136+
message,
137+
).toContain(insertText)
138+
}
139+
}
140+
if (all) {
141+
for (const entry of result.entries.filter(e => names?.includes(e.name))) {
142+
expect(entry, entry.name + message).toContain(all)
143+
}
136144
}
137145
}
138-
}
139-
if (excludes) {
140-
for (const exclude of excludes) {
141-
expect(result?.entryNames, message).not.toContain(exclude)
146+
if (excludes) {
147+
for (const exclude of excludes) {
148+
expect(result?.entryNames, message).not.toContain(exclude)
149+
}
142150
}
143151
}
152+
} finally {
153+
languageService.getSemanticDiagnostics = oldGetSemanticDiagnostics
144154
}
145155
},
146-
codeAction: (marker: number | number[], matcher: CodeActionMatcher, meta?, { compareContent = false } = {}) => {
156+
codeAction(marker: number | number[], matcher: CodeActionMatcher, meta?, { compareContent = false } = {}) {
147157
for (const mark of Array.isArray(marker) ? marker : [marker]) {
148158
if (!ranges[mark]) throw new Error(`No range with index ${mark} found, highest index is ${ranges.length - 1}`)
149159
const start = ranges[mark]![0]!
@@ -192,10 +202,10 @@ export const fileContentsSpecialPositions = (contents: string, fileName = entryp
192202
let mainMatch = currentMatch[1]!
193203
if (addOnly) mainMatch = mainMatch.slice(0, -1)
194204
const possiblyNum = +mainMatch
195-
if (!Number.isNaN(possiblyNum)) {
196-
addArr[2][possiblyNum] = offset
197-
} else {
205+
if (Number.isNaN(possiblyNum)) {
198206
addArr[mainMatch === 't' ? '0' : '1'].push(offset)
207+
} else {
208+
addArr[2][possiblyNum] = offset
199209
}
200210
replacement.lastIndex -= matchLength
201211
}

0 commit comments

Comments
 (0)