Skip to content

Commit

Permalink
fix: improve repl/variable/completion performance for TS
Browse files Browse the repository at this point in the history
Fixes #1433

There were a couple problems:

- TS uses a `customDescriptionGenerator`. Previously this was being
  invoked on each individual property. I've merged this with the logic
  we added for custom `toString()` representations, so that we run it
  only once for an object and return a map of its properties.
- TS has thousands of variables in each scope. We call `Runtime.getProperties`
  to get scope variables to use in completions. It seems like getProperties,
  which does not have any 'limit' parameter, is blocking. We still need
  this to do sensible completions, but now each stacktrace caches its
  completions instead of getting new ones on every request.

With these changes, there may still be a little REPL stall on the first
evaluation (until the first completions call finishes, which takes 3-5s
on my macbook), but after that it should be appropriately snappy.
  • Loading branch information
connor4312 committed Dec 8, 2022
1 parent d812305 commit 647a933
Show file tree
Hide file tree
Showing 8 changed files with 126 additions and 93 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ This changelog records changes to stable releases since 1.50.2. "TBA" changes he
- fix: perScriptSourcemaps not reliably breaking ([vscode#166369](https://github.com/microsoft/vscode/issues/166369))
- fix: custom object `toString()` previews being too short ([vscode#155142](https://github.com/microsoft/vscode/issues/155142))
- fix: show warning if console output length is hit ([vscode#154479](https://github.com/microsoft/vscode/issues/154479))
- fix: improve variable and repl performance in large projects ([#1433](https://github.com/microsoft/vscode-js-debug/issues/1433))

## v1.74 (November 2022)

Expand Down
2 changes: 1 addition & 1 deletion src/adapter/completions.ts
Original file line number Diff line number Diff line change
Expand Up @@ -324,7 +324,7 @@ export class Completions {
...params,
returnByValue: true,
throwOnSideEffect: false,
expression: enumeratePropertiesTemplate(
expression: enumeratePropertiesTemplate.expr(
`(${expression})`,
JSON.stringify(prefix),
JSON.stringify(isInGlobalScope),
Expand Down
8 changes: 4 additions & 4 deletions src/adapter/stackTrace.ts
Original file line number Diff line number Diff line change
Expand Up @@ -468,7 +468,7 @@ export class StackFrame implements IFrameElement {
return (scope.variables[scopeNumber] = variable);
}

async completions(): Promise<Dap.CompletionItem[]> {
public readonly completions = once(async (): Promise<Dap.CompletionItem[]> => {
if (!this._scope) return [];
const variableStore = this._thread.pausedVariables();
if (!variableStore) {
Expand All @@ -479,14 +479,14 @@ export class StackFrame implements IFrameElement {
for (let scopeNumber = 0; scopeNumber < this._scope.chain.length; scopeNumber++) {
promises.push(
this._scopeVariable(scopeNumber, this._scope).then(async scopeVariable => {
const variables = await variableStore.getVariables({
const variables = await variableStore.getVariableNames({
variablesReference: scopeVariable.id,
});
return variables.map(variable => ({ label: variable.name, type: 'property' }));
return variables.map(label => ({ label: label, type: 'property' }));
}),
);
}
const completions = await Promise.all(promises);
return ([] as Dap.CompletionItem[]).concat(...completions);
}
});
}
41 changes: 38 additions & 3 deletions src/adapter/templates/getStringyProps.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,31 +2,66 @@
* Copyright (C) Microsoft Corporation. All rights reserved.
*--------------------------------------------------------*/

import { remoteFunction } from '.';
import { templateFunction } from '.';

/**
* Gets a mapping of property names with a custom `.toString()` method
* to their string representations.
*/
export const getStringyProps = remoteFunction(function (this: unknown, maxLength: number) {
export const getStringyProps = templateFunction(function (
this: unknown,
maxLength: number,
customToString: (defaultRepr: string) => unknown,
) {
const out: Record<string, string> = {};
const defaultPlaceholder = '<<default preview>>';
if (typeof this !== 'object' || !this) {
return out;
}

for (const [key, value] of Object.entries(this)) {
if (customToString) {
try {
const repr = customToString.call(value, defaultPlaceholder);
if (repr !== defaultPlaceholder) {
out[key] = String(repr);
continue;
}
} catch (e) {
out[key] = `${e} (couldn't describe ${key})`;
continue;
}
}

if (typeof value === 'object' && value && !String(value.toString).includes('[native code]')) {
const str = String(value);
if (!str.startsWith('[object ')) {
out[key] = str.length >= maxLength ? str.slice(0, maxLength) + '…' : str;
continue;
}
}
}

return out;
});

export const getToStringIfCustom = remoteFunction(function (this: unknown, maxLength: number) {
export const getToStringIfCustom = templateFunction(function (
this: unknown,
maxLength: number,
customToString: (defaultRepr: string) => unknown,
) {
if (customToString) {
try {
const defaultPlaceholder = '<<default preview>>';
const repr = customToString.call(this, defaultPlaceholder);
if (repr !== defaultPlaceholder) {
return String(repr);
}
} catch (e) {
return `${e} (couldn't describe object)`;
}
}

if (typeof this === 'object' && this && !String(this.toString).includes('[native code]')) {
const str = String(this);
if (!str.startsWith('[object ')) {
Expand Down
29 changes: 19 additions & 10 deletions src/adapter/templates/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,11 @@ import { SourceConstants } from '../sources';
export const getSourceSuffix = () =>
`\n//# sourceURL=eval-${randomBytes(4).toString('hex')}${SourceConstants.InternalExtension}\n`;

export type TemplateFunction<A extends unknown[]> = {
expr: (...args: A) => string;
decl: (...args: A) => string;
};

/**
* Creates a template for the given function that replaces its arguments
* and generates a string to be executed where it takes expressions to be
Expand Down Expand Up @@ -43,21 +48,21 @@ export const getSourceSuffix = () =>
* })();
* ```
*/
export function templateFunction<A>(fn: (a: A) => void): (a: string) => string;
export function templateFunction<A, B>(fn: (a: A, b: B) => void): (a: string, b: string) => string;
export function templateFunction<A>(fn: (a: A) => void): TemplateFunction<[string]>;
export function templateFunction<A, B>(
fn: (a: A, b: B) => void,
): TemplateFunction<[string, string]>;
export function templateFunction<A, B, C>(
fn: (a: A, b: B, c: C) => void,
): (a: string, b: string, c: string) => string;
export function templateFunction<Args extends unknown[]>(fn: string): (...args: Args) => string;
): TemplateFunction<[string, string, string]>;
export function templateFunction<Args extends unknown[]>(fn: string): TemplateFunction<Args>;
export function templateFunction<Args extends unknown[]>(
fn: string | ((...args: Args) => void),
): (...args: string[]) => string {
): TemplateFunction<string[]> {
return templateFunctionStr('' + fn);
}

export function templateFunctionStr<Args extends string[]>(
stringified: string,
): (...args: Args) => string {
function templateFunctionStr<Args extends string[]>(stringified: string): TemplateFunction<Args> {
const decl = parseExpressionAt(stringified, 0, {
ecmaVersion: 'latest',
locations: true,
Expand All @@ -76,10 +81,14 @@ export function templateFunctionStr<Args extends string[]>(
});

const { start, end } = decl.body as unknown as Node;
return (...args) => `(() => {
const inner = (args: string[]) => `
${args.map((a, i) => `let ${params[i]} = ${a}`).join('; ')};
${stringified.slice(start + 1, end - 1)}
})();${getSourceSuffix()}`;
`;
return {
expr: (...args: Args) => `(()=>{${inner(args)}})();\n${getSourceSuffix()}`,
decl: (...args: Args) => `function(){${inner(args)};\n${getSourceSuffix()}}`,
};
}

/**
Expand Down
2 changes: 1 addition & 1 deletion src/adapter/templates/serializeForClipboard.ts
Original file line number Diff line number Diff line change
Expand Up @@ -141,7 +141,7 @@ export const serializeForClipboardTmpl = templateFunction(function (

export const serializeForClipboard = remoteFunction<[number], string>(`
function(spaces2) {
const result = ${serializeForClipboardTmpl('this', 'spaces2')};
const result = ${serializeForClipboardTmpl.expr('this', 'spaces2')};
return result;
}
`);
2 changes: 1 addition & 1 deletion src/adapter/threads.ts
Original file line number Diff line number Diff line change
Expand Up @@ -471,7 +471,7 @@ export class Thread implements IVariableStoreLocationProvider {
const params: Cdp.Runtime.EvaluateParams =
args.context === 'clipboard'
? {
expression: serializeForClipboardTmpl(args.expression, '2'),
expression: serializeForClipboardTmpl.expr(args.expression, '2'),
includeCommandLineAPI: true,
returnByValue: true,
objectGroup: 'console',
Expand Down
Loading

0 comments on commit 647a933

Please sign in to comment.