Skip to content

Commit

Permalink
fix: significantly improve completions performance in heavy applicati…
Browse files Browse the repository at this point in the history
…ons that use MUI (unoptimized Material-UI) by using cached diagnostics for not declared const variable names suggestions instead
  • Loading branch information
zardoy committed Jan 31, 2024
1 parent fe70eb3 commit adee249
Show file tree
Hide file tree
Showing 5 changed files with 84 additions and 47 deletions.
3 changes: 2 additions & 1 deletion .eslintrc.json
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,8 @@
"@typescript-eslint/consistent-type-imports": "off",
"@typescript-eslint/ban-types": "off",
"sonarjs/prefer-single-boolean-return": "off",
"unicorn/no-typeof-undefined": "off" // todo disable globally
"unicorn/no-typeof-undefined": "off", // todo disable globally
"@typescript-eslint/consistent-type-definitions": "off"
},
"overrides": [
{
Expand Down
3 changes: 2 additions & 1 deletion typescript/src/completions/boostNameSuggestions.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import { cachedResponse } from '../decorateProxy'
import { boostExistingSuggestions, boostOrAddSuggestions, findChildContainingPosition } from '../utils'
import { getCannotFindCodes } from '../utils/cannotFindCodes'

Expand Down Expand Up @@ -46,7 +47,7 @@ export default (
}

if (filterBlock === undefined) return entries
const semanticDiagnostics = languageService.getSemanticDiagnostics(sourceFile.fileName)
const semanticDiagnostics = cachedResponse.getSemanticDiagnostics?.[sourceFile.fileName] ?? []

const notFoundIdentifiers = semanticDiagnostics
.filter(({ code }) => cannotFindCodes.includes(code))
Expand Down
12 changes: 12 additions & 0 deletions typescript/src/decorateProxy.ts
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,10 @@ export const getInitialProxy = (languageService: ts.LanguageService, proxy = Obj
return proxy
}

export const cachedResponse = {
getSemanticDiagnostics: {} as Record<string, ts.Diagnostic[]>,
}

export const decorateLanguageService = (
{ languageService, languageServiceHost }: PluginCreateArg,
existingProxy: ts.LanguageService | undefined,
Expand Down Expand Up @@ -169,11 +173,19 @@ export const decorateLanguageService = (
.filter(([, v]) => v === false)
.map(([k]) => k)
if (toDisable.includes(feature)) return undefined

// eslint-disable-next-line @typescript-eslint/no-require-imports
const performance = globalThis.performance ?? require('perf_hooks').performance
const start = performance.now()

//@ts-expect-error
const result = orig(...args)

if (feature in cachedResponse) {
// todo use weakmap with sourcefiles to ensure it doesn't grow up
cachedResponse[feature][args[0]] = result
}

const time = performance.now() - start
if (time > 100) console.log(`[typescript-vscode-plugin perf warning] ${feature} took ${time}ms: ${args[0]} ${args[1]}`)
return result
Expand Down
13 changes: 13 additions & 0 deletions typescript/test/completions.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,19 @@ test('Banned positions', () => {
expect(getCompletionsAtPosition(cursorPositions[2]!)?.entries).toHaveLength(1)
})

test.todo('Const name suggestions (boostNameSuggestions)', () => {
const tester = fourslashLikeTester(/* ts */ `
const /*0*/ = 5
testVariable
`)
languageService.getSemanticDiagnostics(entrypoint)
tester.completion(0, {
includes: {
names: ['testVariable'],
},
})
})

test('Banned positions for all method snippets', () => {
const cursorPositions = newFileContents(/* tsx */ `
import {/*|*/} from 'test'
Expand Down
100 changes: 55 additions & 45 deletions typescript/test/testing.ts
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ interface CodeActionMatcher {

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

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

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

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

const ranges = positive.reduce<number[][]>(
(prevRanges, pos) => {
const lastPrev = prevRanges[prevRanges.length - 1]!
const lastPrev = prevRanges.at(-1)!
if (lastPrev.length < 2) {
lastPrev.push(pos)
return prevRanges
Expand All @@ -92,58 +92,68 @@ export const fourslashLikeTester = (contents: string, fileName = entrypoint, { d
[[]],
)
return {
completion: (marker: number | number[], matcher: CompletionMatcher, meta?) => {
for (const mark of Array.isArray(marker) ? marker : [marker]) {
if (numberedPositions[mark] === undefined) throw new Error(`No marker ${mark} found`)
const result = getCompletionsAtPosition(numberedPositions[mark]!, { shouldHave: true })!
const message = ` at marker ${mark}`
const { exact, includes, excludes } = matcher
if (exact) {
const { names, all, insertTexts } = exact
if (names) {
expect(result?.entryNames, message).toEqual(names)
}
if (insertTexts) {
expect(
result.entries.map(entry => entry.insertText),
message,
).toEqual(insertTexts)
}
if (all) {
for (const entry of result.entries) {
expect(entry, entry.name + message).toContain(all)
}
}
}
if (includes) {
const { names, all, insertTexts } = includes
if (names) {
for (const name of names) {
expect(result?.entryNames, message).toContain(name)
completion(marker: number | number[], matcher: CompletionMatcher, meta?) {
const oldGetSemanticDiagnostics = languageService.getSemanticDiagnostics
languageService.getSemanticDiagnostics = () => {
throw new Error('getSemanticDiagnostics should not be called because of performance reasons')
// return []
}

try {
for (const mark of Array.isArray(marker) ? marker : [marker]) {
if (numberedPositions[mark] === undefined) throw new Error(`No marker ${mark} found`)
const result = getCompletionsAtPosition(numberedPositions[mark]!, { shouldHave: true })!
const message = ` at marker ${mark}`
const { exact, includes, excludes } = matcher
if (exact) {
const { names, all, insertTexts } = exact
if (names) {
expect(result?.entryNames, message).toEqual(names)
}
}
if (insertTexts) {
for (const insertText of insertTexts) {
if (insertTexts) {
expect(
result.entries.map(entry => entry.insertText),
message,
).toContain(insertText)
).toEqual(insertTexts)
}
if (all) {
for (const entry of result.entries) {
expect(entry, entry.name + message).toContain(all)
}
}
}
if (all) {
for (const entry of result.entries.filter(e => names?.includes(e.name))) {
expect(entry, entry.name + message).toContain(all)
if (includes) {
const { names, all, insertTexts } = includes
if (names) {
for (const name of names) {
expect(result?.entryNames, message).toContain(name)
}
}
if (insertTexts) {
for (const insertText of insertTexts) {
expect(
result.entries.map(entry => entry.insertText),
message,
).toContain(insertText)
}
}
if (all) {
for (const entry of result.entries.filter(e => names?.includes(e.name))) {
expect(entry, entry.name + message).toContain(all)
}
}
}
}
if (excludes) {
for (const exclude of excludes) {
expect(result?.entryNames, message).not.toContain(exclude)
if (excludes) {
for (const exclude of excludes) {
expect(result?.entryNames, message).not.toContain(exclude)
}
}
}
} finally {
languageService.getSemanticDiagnostics = oldGetSemanticDiagnostics
}
},
codeAction: (marker: number | number[], matcher: CodeActionMatcher, meta?, { compareContent = false } = {}) => {
codeAction(marker: number | number[], matcher: CodeActionMatcher, meta?, { compareContent = false } = {}) {
for (const mark of Array.isArray(marker) ? marker : [marker]) {
if (!ranges[mark]) throw new Error(`No range with index ${mark} found, highest index is ${ranges.length - 1}`)
const start = ranges[mark]![0]!
Expand Down Expand Up @@ -192,10 +202,10 @@ export const fileContentsSpecialPositions = (contents: string, fileName = entryp
let mainMatch = currentMatch[1]!
if (addOnly) mainMatch = mainMatch.slice(0, -1)
const possiblyNum = +mainMatch
if (!Number.isNaN(possiblyNum)) {
addArr[2][possiblyNum] = offset
} else {
if (Number.isNaN(possiblyNum)) {
addArr[mainMatch === 't' ? '0' : '1'].push(offset)
} else {
addArr[2][possiblyNum] = offset
}
replacement.lastIndex -= matchLength
}
Expand Down

0 comments on commit adee249

Please sign in to comment.